github/jooq
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

[#461] [#473] [#2597] [#8234] Unnecessary casts should no longer be ignored

Browse Source 
Historically, SQL casts were ignored if jOOQ's Field<T> type was of the same type T as the cast type. This makes sense for some internals, but not in the public API. When a user wants to cast an expression, this should always produce a SQL cast.
This commit is contained in:
lukaseder 2019-04-10 16:16:20 +02:00
parent 5832e45eaf
commit cd057d34d4
19 changed files with 105 additions and 65 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

33
jOOQ/src/main/java/org/jooq/impl/AbstractField.java
View File

@ -63,6 +63,7 @@ import static org.jooq.impl.ExpressionOperator.MULTIPLY;
import static org.jooq.impl.ExpressionOperator.SUBTRACT;
import static org.jooq.impl.Tools.EMPTY_FIELD;
import static org.jooq.impl.Tools.EMPTY_STRING;
import static org.jooq.impl.Tools.castIfNeeded;
import static org.jooq.tools.Convert.FALSE_VALUES;
import static org.jooq.tools.Convert.TRUE_VALUES;
@ -245,26 +246,14 @@ abstract class AbstractField<T> extends AbstractNamed implements Field<T> {
return cast(field.getDataType());
}
@SuppressWarnings("unchecked")
@Override
public final <Z> Field<Z> cast(DataType<Z> type) {
// [#473] Prevent unnecessary casts
if (getDataType().equals(type))
return (Field<Z>) this;
else
return new Cast<Z>(this, type);
return new Cast<Z>(this, type);
}
@SuppressWarnings("unchecked")
@Override
public final <Z> Field<Z> cast(Class<Z> type) {
// [#2597] Prevent unnecessary casts
if (getType() == type)
return (Field<Z>) this;
else
return cast(DefaultDataType.getDataType(null, type));
return cast(DefaultDataType.getDataType(null, type));
}
// ------------------------------------------------------------------------
@ -276,15 +265,9 @@ abstract class AbstractField<T> extends AbstractNamed implements Field<T> {
return coerce(field.getDataType());
}
@SuppressWarnings("unchecked")
@Override
public final <Z> Field<Z> coerce(DataType<Z> type) {
// [#473] Prevent unnecessary coercions
if (getDataType().equals(type))
return (Field<Z>) this;
else
return new Coerce<Z>(this, type);
return new Coerce<Z>(this, type);
}
@Override
@ -721,7 +704,7 @@ abstract class AbstractField<T> extends AbstractNamed implements Field<T> {
else if (Boolean.class.isAssignableFrom(type))
return ((Field<Boolean>) this).equal(inline(true));
else
return cast(String.class).in(TRUE_VALUES);
return castIfNeeded(this, String.class).in(TRUE_VALUES);
}
@SuppressWarnings({ "unchecked" })
@ -736,7 +719,7 @@ abstract class AbstractField<T> extends AbstractNamed implements Field<T> {
else if (Boolean.class.isAssignableFrom(type))
return ((Field<Boolean>) this).equal(inline(false));
else
return cast(String.class).in(Tools.inline(FALSE_VALUES.toArray(EMPTY_STRING)));
return castIfNeeded(this, String.class).in(Tools.inline(FALSE_VALUES.toArray(EMPTY_STRING)));
}
@Override
@ -1272,7 +1255,7 @@ abstract class AbstractField<T> extends AbstractNamed implements Field<T> {
@Override
public final Condition equalIgnoreCase(Field<String> value) {
return DSL.lower(cast(String.class)).equal(DSL.lower(value));
return DSL.lower(castIfNeeded(this, String.class)).equal(DSL.lower(value));
}
@Override
@ -1302,7 +1285,7 @@ abstract class AbstractField<T> extends AbstractNamed implements Field<T> {
@Override
public final Condition notEqualIgnoreCase(Field<String> value) {
return DSL.lower(cast(String.class)).notEqual(DSL.lower(value));
return DSL.lower(castIfNeeded(this, String.class)).notEqual(DSL.lower(value));
}
@Override

3
jOOQ/src/main/java/org/jooq/impl/CompareCondition.java
View File

@ -60,6 +60,7 @@ import static org.jooq.impl.Keywords.K_AS;
import static org.jooq.impl.Keywords.K_CAST;
import static org.jooq.impl.Keywords.K_ESCAPE;
import static org.jooq.impl.Keywords.K_VARCHAR;
import static org.jooq.impl.Tools.castIfNeeded;
import static org.jooq.impl.Tools.embeddedFields;
import static org.jooq.impl.Tools.isEmbeddable;
@ -124,7 +125,7 @@ final class CompareCondition extends AbstractCondition implements LikeEscapeStep
&& field1.getType() != String.class
&& REQUIRES_CAST_ON_LIKE.contains(family)) {
lhs = lhs.cast(String.class);
lhs = castIfNeeded(lhs, String.class);
}
// [#1423] Only Postgres knows a true ILIKE operator. Other dialects

7
jOOQ/src/main/java/org/jooq/impl/Concat.java
View File

@ -37,11 +37,11 @@
*/
package org.jooq.impl;
import static org.jooq.impl.DSL.castAll;
import static org.jooq.impl.DSL.function;
import static org.jooq.impl.ExpressionOperator.ADD;
import static org.jooq.impl.ExpressionOperator.BIT_AND;
import static org.jooq.impl.ExpressionOperator.CONCAT;
import static org.jooq.impl.Tools.castAllIfNeeded;
import org.jooq.Configuration;
import org.jooq.Field;
@ -65,12 +65,11 @@ final class Concat extends AbstractFunction<String> {
final Field<String> getFunction0(Configuration configuration) {
// [#461] Type cast the concat expression, if this isn't a VARCHAR field
Field<String>[] cast = castAll(String.class, getArguments());
Field<String>[] cast = castAllIfNeeded(getArguments(), String.class);
// If there is only one argument, return it immediately
if (cast.length == 1) {
if (cast.length == 1)
return cast[0];
}
Field<String> first = cast[0];
Field<String>[] others = new Field[cast.length - 1];

18
jOOQ/src/main/java/org/jooq/impl/DSL.java
View File

@ -12443,24 +12443,6 @@ public class DSL {
return NULL().cast(type);
}
/**
* Cast all fields that need casting.
*
* @param <T> The generic field type
* @param type The type to cast to
* @param fields The fields to be cast to a uniform type
* @return The cast fields
*/
static <T> Field<T>[] castAll(Class<T> type, Field<?>... fields) {
Field<?>[] castFields = new Field<?>[fields.length];
for (int i = 0; i < fields.length; i++) {
castFields[i] = fields[i].cast(type);
}
return (Field<T>[]) castFields;
}
/**
* The <code>COALESCE(value1, value2, ... , value n)</code> function.
*

1
jOOQ/src/main/java/org/jooq/impl/DateAdd.java
View File

@ -41,6 +41,7 @@ import static org.jooq.impl.DSL.inline;
import static org.jooq.impl.DSL.keyword;
import static org.jooq.impl.DSL.sql;
import static org.jooq.impl.SQLDataType.VARCHAR;
import static org.jooq.impl.Tools.castIfNeeded;
import org.jooq.Configuration;
import org.jooq.DatePart;

3
jOOQ/src/main/java/org/jooq/impl/DateDiff.java
View File

@ -38,6 +38,7 @@
package org.jooq.impl;
import static org.jooq.impl.DSL.function;
import static org.jooq.impl.Tools.castIfNeeded;
import org.jooq.Configuration;
import org.jooq.Field;
@ -123,6 +124,6 @@ final class DateDiff<T> extends AbstractFunction<Integer> {
}
// Default implementation for equals() and hashCode()
return date1.sub(date2).cast(Integer.class);
return castIfNeeded(date1.sub(date2), Integer.class);
}
}

3
jOOQ/src/main/java/org/jooq/impl/DateOrTime.java
View File

@ -38,6 +38,7 @@
package org.jooq.impl;
import static org.jooq.impl.DSL.keyword;
import static org.jooq.impl.Tools.castIfNeeded;
import org.jooq.Configuration;
import org.jooq.DataType;
@ -93,7 +94,7 @@ final class DateOrTime<T> extends AbstractFunction<T> {
}
default:
return field.cast(getDataType());
return castIfNeeded(field, getDataType());
}
}
}

3
jOOQ/src/main/java/org/jooq/impl/Degrees.java
View File

@ -39,6 +39,7 @@ package org.jooq.impl;
import static org.jooq.impl.DSL.inline;
import static org.jooq.impl.DSL.pi;
import static org.jooq.impl.Tools.castIfNeeded;
import java.math.BigDecimal;
@ -76,7 +77,7 @@ final class Degrees extends AbstractFunction<BigDecimal> {
case FIREBIRD:
case SQLITE:
return argument.cast(BigDecimal.class).mul(inline(180)).div(pi());
return castIfNeeded(argument, BigDecimal.class).mul(inline(180)).div(pi());
default:
return DSL.field("{degrees}({0})", SQLDataType.NUMERIC, argument);

11
jOOQ/src/main/java/org/jooq/impl/Expression.java
View File

@ -71,6 +71,7 @@ import static org.jooq.impl.ExpressionOperator.BIT_XOR;
import static org.jooq.impl.ExpressionOperator.SHL;
import static org.jooq.impl.ExpressionOperator.SHR;
import static org.jooq.impl.ExpressionOperator.SUBTRACT;
import static org.jooq.impl.Tools.castIfNeeded;
import java.sql.Timestamp;
import java.util.Arrays;
@ -179,12 +180,12 @@ final class Expression<T> extends AbstractFunction<T> {
// Many dialects don't support shifts. Use multiplication/division instead
else if (SHL == operator && EMULATE_SHR_SHL.contains(family))
return lhs.mul((Field<? extends Number>) DSL.power(two(), rhsAsNumber()).cast(lhs));
return lhs.mul((Field<? extends Number>) castIfNeeded(DSL.power(two(), rhsAsNumber()), lhs));
// [#3962] This emulation is expensive. If this is emulated, BitCount should
// use division instead of SHR directly
else if (SHR == operator && EMULATE_SHR_SHL.contains(family))
return lhs.div((Field<? extends Number>) DSL.power(two(), rhsAsNumber()).cast(lhs));
return lhs.div((Field<? extends Number>) castIfNeeded(DSL.power(two(), rhsAsNumber()), lhs));
// Some dialects support shifts as functions
else if (SHL == operator && FIREBIRD == family)
@ -518,6 +519,12 @@ final class Expression<T> extends AbstractFunction<T> {

7
jOOQ/src/main/java/org/jooq/impl/Extract.java
View File

@ -43,6 +43,7 @@ import static org.jooq.impl.DSL.function;
import static org.jooq.impl.DSL.inline;
import static org.jooq.impl.DSL.one;
import static org.jooq.impl.SQLDataType.INTEGER;
import static org.jooq.impl.Tools.castIfNeeded;
import java.sql.Date;
import java.sql.Timestamp;
@ -365,14 +366,14 @@ final class Extract extends AbstractFunction<Integer> {
private final Field<Integer> getDefaultEmulation() {
switch (datePart) {
case DECADE:
return DSL.cast(DSL.year(field).div(inline(10)), INTEGER);
return castIfNeeded(DSL.year(field).div(inline(10)), INTEGER);
case CENTURY:
return DSL.cast(
return castIfNeeded(
DSL.sign(DSL.year(field))
.mul(DSL.abs(DSL.year(field)).add(inline(99)))
.div(inline(100)), INTEGER);
case MILLENNIUM:
return DSL.cast(
return castIfNeeded(
DSL.sign(DSL.year(field))
.mul(DSL.abs(DSL.year(field)).add(inline(999)))
.div(inline(1000)), INTEGER);

3
jOOQ/src/main/java/org/jooq/impl/Function.java
View File

@ -88,6 +88,7 @@ import static org.jooq.impl.Term.MEDIAN;
import static org.jooq.impl.Term.MODE;
import static org.jooq.impl.Term.PRODUCT;
import static org.jooq.impl.Term.ROW_NUMBER;
import static org.jooq.impl.Tools.castIfNeeded;
import static org.jooq.impl.Tools.BooleanDataKey.DATA_RANKING_FUNCTION;
import static org.jooq.impl.Tools.BooleanDataKey.DATA_ROWNUMBER_FUNCTION;
import static org.jooq.impl.Tools.DataKey.DATA_WINDOW_DEFINITIONS;
@ -366,7 +367,7 @@ class Function<T> extends AbstractField<T> implements
ctx.visit(K_DISTINCT).sql(' ');
// The explicit cast is needed in Postgres
ctx.visit(((Field<?>) arguments.get(0)).cast(String.class));
ctx.visit(castIfNeeded((Field<?>) arguments.get(0), String.class));
if (arguments.size() > 1)
ctx.sql(", ").visit(arguments.get(1));

3
jOOQ/src/main/java/org/jooq/impl/Radians.java
View File

@ -40,6 +40,7 @@ package org.jooq.impl;
import static org.jooq.impl.DSL.function;
import static org.jooq.impl.DSL.inline;
import static org.jooq.impl.DSL.pi;
import static org.jooq.impl.Tools.castIfNeeded;
import java.math.BigDecimal;
@ -75,7 +76,7 @@ final class Radians extends AbstractFunction<BigDecimal> {
case FIREBIRD:
case SQLITE:
return argument.cast(BigDecimal.class).mul(pi()).div(inline(180));
return castIfNeeded(argument, BigDecimal.class).mul(pi()).div(inline(180));
default:
return function("radians", SQLDataType.NUMERIC, argument);

3
jOOQ/src/main/java/org/jooq/impl/RatioToReport.java
View File

@ -40,6 +40,7 @@ package org.jooq.impl;
import static org.jooq.SQLDialect.SQLITE;
import static org.jooq.impl.SQLDataType.DECIMAL;
import static org.jooq.impl.SQLDataType.DOUBLE;
import static org.jooq.impl.Tools.castIfNeeded;
import java.math.BigDecimal;
@ -82,7 +83,7 @@ final class RatioToReport extends Function<BigDecimal> {
default:
ctx.visit(field.cast((DataType<?>) (ctx.family() == SQLITE ? DOUBLE : DECIMAL)))
ctx.visit(castIfNeeded(field, (DataType<?>) (ctx.family() == SQLITE ? DOUBLE : DECIMAL)))
.sql(" / ")
.visit(DSL.sum(field));
toSQLOverClause(ctx);

3
jOOQ/src/main/java/org/jooq/impl/Round.java
View File

@ -39,6 +39,7 @@ package org.jooq.impl;
import static org.jooq.impl.DSL.function;
import static org.jooq.impl.DSL.val;
import static org.jooq.impl.Tools.castIfNeeded;
import java.math.BigDecimal;
@ -110,7 +111,7 @@ final class Round<T extends Number> extends AbstractFunction<T> {
if (decimals == 0)
return function("round", getDataType(), argument);
else
return function("round", getDataType(), argument.cast(BigDecimal.class), val(decimals));
return function("round", getDataType(), castIfNeeded(argument, BigDecimal.class), val(decimals));
}
// This is the optimal implementation by most RDBMS

5
jOOQ/src/main/java/org/jooq/impl/RowOverlapsCondition.java
View File

@ -58,6 +58,7 @@ import static org.jooq.SQLDialect.SQLITE;
// ...
// ...
import static org.jooq.impl.Keywords.K_OVERLAPS;
import static org.jooq.impl.Tools.castIfNeeded;
import java.util.EnumSet;
@ -129,8 +130,8 @@ final class RowOverlapsCondition<T1, T2> extends AbstractCondition {
// All other OVERLAPS predicates can be emulated simply
else {
return (QueryPartInternal)
right1.le(left2.cast(right1)).and(
left1.le(right2.cast(left1)));
right1.le(castIfNeeded(left2, right1)).and(
left1.le(castIfNeeded(right2, left1)));
}
}

3
jOOQ/src/main/java/org/jooq/impl/TimestampDiff.java
View File

@ -39,6 +39,7 @@ package org.jooq.impl;
import static org.jooq.impl.DSL.function;
import static org.jooq.impl.SQLDataType.INTEGER;
import static org.jooq.impl.Tools.castIfNeeded;
import org.jooq.Configuration;
import org.jooq.Field;
@ -144,6 +145,6 @@ final class TimestampDiff extends AbstractFunction<DayToSecond> {
}
// Default implementation for equals() and hashCode()
return timestamp1.sub(timestamp2).cast(DayToSecond.class);
return castIfNeeded(timestamp1.sub(timestamp2), DayToSecond.class);
}
}

48
jOOQ/src/main/java/org/jooq/impl/Tools.java
View File

@ -1254,6 +1254,52 @@ final class Tools {
return new IllegalArgumentException("Cannot interpret argument of type " + value.getClass() + " as a Field: " + value);
}
/**
* [#461] [#473] [#2597] [#8234] Some internals need a cast only if necessary.
*/
@SuppressWarnings("unchecked")
static <T> Field<T>[] castAllIfNeeded(Field<?>[] fields, Class<T> type) {
Field<T>[] castFields = new Field[fields.length];
for (int i = 0; i < fields.length; i++)
castFields[i] = castIfNeeded(fields[i], type);
return castFields;
}
/**
* [#461] [#473] [#2597] [#8234] Some internals need a cast only if necessary.
*/
@SuppressWarnings("unchecked")
static final <T> Field<T> castIfNeeded(Field<?> field, Class<T> type) {
if (field.getType().equals(type))
return (Field<T>) field;
else
return field.cast(type);
}
/**
* [#461] [#473] [#2597] [#8234] Some internals need a cast only if necessary.
*/
@SuppressWarnings("unchecked")
static final <T> Field<T> castIfNeeded(Field<?> field, DataType<T> type) {
if (field.getDataType().equals(type))
return (Field<T>) field;
else
return field.cast(type);
}
/**
* [#461] [#473] [#2597] [#8234] Some internals need a cast only if necessary.
*/
@SuppressWarnings("unchecked")
static final <T> Field<T> castIfNeeded(Field<?> field, Field<T> type) {
if (field.getDataType().equals(type.getDataType()))
return (Field<T>) field;
else
return field.cast(type);
}
/**
* Be sure that a given object is a field.
*
@ -2794,7 +2840,7 @@ final class Tools {
}
}
else {
return field.cast(String.class);
return castIfNeeded(field, String.class);
}
}

9
jOOQ/src/main/java/org/jooq/impl/Trunc.java
View File

@ -42,6 +42,7 @@ import static org.jooq.impl.DSL.inline;
import static org.jooq.impl.DSL.keyword;
import static org.jooq.impl.DSL.one;
import static org.jooq.impl.DSL.zero;
import static org.jooq.impl.Tools.castIfNeeded;
import static org.jooq.impl.Tools.extractVal;
import java.math.BigDecimal;
@ -111,7 +112,13 @@ final class Trunc<T> extends AbstractFunction<T> {
case POSTGRES:
return DSL.field("{trunc}({0}, {1})", SQLDataType.NUMERIC, field.cast(BigDecimal.class), decimals).cast(field.getDataType());
return castIfNeeded(
DSL.field("{trunc}({0}, {1})", SQLDataType.NUMERIC,
castIfNeeded(field, BigDecimal.class),
decimals
),
field.getDataType()
);

4
jOOQ/src/main/java/org/jooq/impl/TruncDate.java
View File

@ -38,6 +38,7 @@
package org.jooq.impl;
import static org.jooq.impl.DSL.inline;
import static org.jooq.impl.Tools.castIfNeeded;
import java.sql.Date;
@ -219,6 +220,9 @@ final class TruncDate<T> extends AbstractFunction<T> {