Skip to content

Commit

Permalink
[CALCITE-6015] AssertionError during optimization of EXTRACT expression
Browse files Browse the repository at this point in the history
Signed-off-by: Mihai Budiu <[email protected]>
  • Loading branch information
mihaibudiu committed Mar 20, 2024
1 parent 9c0d690 commit 048fe95
Show file tree
Hide file tree
Showing 9 changed files with 213 additions and 110 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -3128,7 +3128,8 @@ private static class ExtractImplementor extends AbstractRexCallImplementor {
case MILLISECOND:
case MICROSECOND:
case NANOSECOND:
if (sqlTypeName == SqlTypeName.DATE) {
if (sqlTypeName == SqlTypeName.DATE
|| SqlTypeName.YEAR_INTERVAL_TYPES.contains(sqlTypeName)) {
return Expressions.constant(0L);
}
operand = mod(operand, TimeUnit.MINUTE.multiplier.longValue(), !isIntervalType);
Expand All @@ -3155,9 +3156,6 @@ private static class ExtractImplementor extends AbstractRexCallImplementor {
translator.getRoot()));
return Expressions.divide(operand,
Expressions.constant(TimeUnit.SECOND.multiplier.longValue()));
case INTERVAL_YEAR:
case INTERVAL_YEAR_MONTH:
case INTERVAL_MONTH:
case INTERVAL_DAY:
case INTERVAL_DAY_HOUR:
case INTERVAL_DAY_MINUTE:
Expand All @@ -3168,6 +3166,11 @@ private static class ExtractImplementor extends AbstractRexCallImplementor {
case INTERVAL_MINUTE:
case INTERVAL_MINUTE_SECOND:
case INTERVAL_SECOND:
return Expressions.divide(operand,
Expressions.constant(TimeUnit.SECOND.multiplier.longValue()));
case INTERVAL_YEAR:
case INTERVAL_YEAR_MONTH:
case INTERVAL_MONTH:
// no convertlet conversion, pass it as extract
throw new AssertionError("unexpected " + sqlTypeName);
default:
Expand Down
2 changes: 1 addition & 1 deletion core/src/main/java/org/apache/calcite/sql/SqlCall.java
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ public int operandCount() {
* Returns a string describing the actual argument types of a call, e.g.
* "SUBSTR(VARCHAR(12), NUMBER(3,2), INTEGER)".
*/
protected String getCallSignature(
public String getCallSignature(
SqlValidator validator,
@Nullable SqlValidatorScope scope) {
List<String> signatureList = new ArrayList<>();
Expand Down
119 changes: 117 additions & 2 deletions core/src/main/java/org/apache/calcite/sql/fun/SqlExtractFunction.java
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package org.apache.calcite.sql.fun;

import org.apache.calcite.avatica.util.TimeUnitRange;
import org.apache.calcite.rel.type.RelDataType;
import org.apache.calcite.sql.SqlCall;
import org.apache.calcite.sql.SqlFunction;
import org.apache.calcite.sql.SqlFunctionCategory;
Expand All @@ -26,12 +27,16 @@
import org.apache.calcite.sql.SqlWriter;
import org.apache.calcite.sql.type.OperandTypes;
import org.apache.calcite.sql.type.ReturnTypes;
import org.apache.calcite.sql.type.SqlTypeName;
import org.apache.calcite.sql.validate.SqlMonotonicity;
import org.apache.calcite.sql.validate.SqlValidator;
import org.apache.calcite.sql.validate.SqlValidatorScope;
import org.apache.calcite.util.Util;

import com.google.common.collect.ImmutableSet;

import static org.apache.calcite.sql.validate.SqlNonNullableAccessors.getOperandLiteralValueOrThrow;
import static org.apache.calcite.util.Static.RESOURCE;

/**
* The SQL <code>EXTRACT</code> operator. Extracts a specified field value from
Expand Down Expand Up @@ -70,6 +75,59 @@ public SqlExtractFunction(String name) {
writer.endFunCall(frame);
}

// List of types that support EXTRACT(X, ...) where X is MONTH or larger
private static final ImmutableSet<SqlTypeName> MONTH_AND_ABOVE_TYPES =
new ImmutableSet.Builder<SqlTypeName>()
.add(SqlTypeName.DATE)
.add(SqlTypeName.TIMESTAMP)
.add(SqlTypeName.TIMESTAMP_WITH_LOCAL_TIME_ZONE)
.addAll(SqlTypeName.YEAR_INTERVAL_TYPES)
.build();

// List of types that support EXTRACT(X, ...) where X is between DAY and WEEK
private static final ImmutableSet<SqlTypeName> DAY_TO_WEEK_TYPES =
new ImmutableSet.Builder<SqlTypeName>()
.add(SqlTypeName.DATE)
.add(SqlTypeName.TIMESTAMP)
.add(SqlTypeName.TIMESTAMP_WITH_LOCAL_TIME_ZONE)
.build();

// List of types that support EXTRACT(EPOCH, ...)
private static final ImmutableSet<SqlTypeName> EPOCH_TYPES =
new ImmutableSet.Builder<SqlTypeName>()
.add(SqlTypeName.DATE)
.add(SqlTypeName.TIMESTAMP)
.add(SqlTypeName.TIMESTAMP_WITH_LOCAL_TIME_ZONE)
.addAll(SqlTypeName.YEAR_INTERVAL_TYPES)
.addAll(SqlTypeName.DAY_INTERVAL_TYPES)
.build();

// List of types that support EXTRACT(DAY, ...)
private static final ImmutableSet<SqlTypeName> DAY_TYPES =
new ImmutableSet.Builder<SqlTypeName>()
.add(SqlTypeName.DATE)
.add(SqlTypeName.TIMESTAMP)
.add(SqlTypeName.TIMESTAMP_WITH_LOCAL_TIME_ZONE)
.add(SqlTypeName.INTERVAL_DAY)
.add(SqlTypeName.INTERVAL_DAY_HOUR)
.add(SqlTypeName.INTERVAL_DAY_MINUTE)
.add(SqlTypeName.INTERVAL_DAY_SECOND)
.addAll(SqlTypeName.YEAR_INTERVAL_TYPES)
.build();

// List of types that support EXTRACT(X, ...) where X is
// between HOUR and NANOSECOND
private static final ImmutableSet<SqlTypeName> HOUR_TO_NANOSECOND_TYPES =
new ImmutableSet.Builder<SqlTypeName>()
.add(SqlTypeName.DATE)
.add(SqlTypeName.TIMESTAMP)
.add(SqlTypeName.TIMESTAMP_WITH_LOCAL_TIME_ZONE)
.add(SqlTypeName.TIME)
.add(SqlTypeName.TIME_WITH_LOCAL_TIME_ZONE)
.addAll(SqlTypeName.YEAR_INTERVAL_TYPES)
.addAll(SqlTypeName.DAY_INTERVAL_TYPES)
.build();

@Override public void validateCall(SqlCall call, SqlValidator validator,
SqlValidatorScope scope, SqlValidatorScope operandScope) {
super.validateCall(call, validator, scope, operandScope);
Expand All @@ -83,8 +141,65 @@ public SqlExtractFunction(String name) {
// startUnit = EPOCH and timeFrameName = 'MINUTE15'.
//
// If the latter, check that timeFrameName is valid.
validator.validateTimeFrame(
(SqlIntervalQualifier) call.getOperandList().get(0));
SqlIntervalQualifier qualifier = call.operand(0);
validator.validateTimeFrame(qualifier);
TimeUnitRange range = qualifier.timeUnitRange;

RelDataType type = validator.getValidatedNodeTypeIfKnown(call.operand(1));
if (type == null) {
return;
}

SqlTypeName typeName = type.getSqlTypeName();
boolean legal;
switch (range) {
case YEAR:
case MONTH:
case ISOYEAR:
case QUARTER:
case DECADE:
case CENTURY:
case MILLENNIUM:
legal = MONTH_AND_ABOVE_TYPES.contains(typeName);
break;
case WEEK:
case DOW:
case ISODOW:
case DOY:
legal = DAY_TO_WEEK_TYPES.contains(typeName);
break;
case EPOCH:
legal = EPOCH_TYPES.contains(typeName);
break;
case DAY:
legal = DAY_TYPES.contains(typeName);
break;
case HOUR:
case MINUTE:
case SECOND:
case MILLISECOND:
case MICROSECOND:
case NANOSECOND:
legal = HOUR_TO_NANOSECOND_TYPES.contains(typeName);
break;
case YEAR_TO_MONTH:
case DAY_TO_HOUR:
case DAY_TO_MINUTE:
case DAY_TO_SECOND:
case HOUR_TO_MINUTE:
case HOUR_TO_SECOND:
case MINUTE_TO_SECOND:
default:
legal = false;
break;
}

if (!legal) {
throw validator.newValidationError(call,
RESOURCE.canNotApplyOp2Type(call.getOperator().getName(),
call.getCallSignature(validator, scope),
call.getOperator().getAllowedSignatures()));
}
}

@Override public SqlMonotonicity getMonotonicity(SqlOperatorBinding call) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1100,7 +1100,8 @@ public static SqlSingleOperandTypeChecker same(int operandCount,
family(SqlTypeFamily.DATETIME_INTERVAL, SqlTypeFamily.DATETIME);

public static final SqlSingleOperandTypeChecker INTERVALINTERVAL_INTERVALDATETIME =
INTERVAL_SAME_SAME.or(INTERVAL_DATETIME);
INTERVAL_SAME_SAME.or(INTERVAL_DATETIME)
.or(family(SqlTypeFamily.INTERVAL_DAY_TIME, SqlTypeFamily.INTERVAL_YEAR_MONTH));

// TODO: datetime+interval checking missing
// TODO: interval+datetime checking missing
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1869,7 +1869,7 @@ protected SqlSelect createSourceSelectForDelete(SqlDelete call) {
}
final SqlNode original = originalExprs.get(node);
if (original != null && original != node) {
return getValidatedNodeType(original);
return getValidatedNodeTypeIfKnown(original);
}
if (node instanceof SqlIdentifier) {
return getCatalogReader().getNamedType((SqlIdentifier) node);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7067,9 +7067,8 @@ void testGroupExpressionEquivalenceParams() {
.columnType("BIGINT NOT NULL");
expr("extract(minute from interval '1.1' second)").ok();
expr("extract(year from DATE '2008-2-2')").ok();
expr("extract(minute from interval '11' month)").ok();

wholeExpr("extract(minute from interval '11' month)")
.fails("(?s).*Cannot apply.*");
wholeExpr("extract(year from interval '11' second)")
.fails("(?s).*Cannot apply.*");
}
Expand Down
4 changes: 4 additions & 0 deletions site/_docs/history.md
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,10 @@ z.
#### Breaking Changes
{: #breaking-1-37-0}

* In the context of [CALCITE-6015] the visibility of the method
`SqlCall.getCallSignature` has been converted from `protected` to `public`.
Any subclass overriding it will need to be adjusted accordingly.

Compatibility: This release is tested on Linux, macOS, Microsoft Windows;
using JDK/OpenJDK versions 8 to 19;
Guava versions 21.0 to 32.1.3-jre;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,14 +84,6 @@ public interface SqlOperatorFixture extends AutoCloseable {
// TODO: Change message
String BAD_DATETIME_MESSAGE = "(?s).*";

// Error messages when an invalid time unit is given as
// input to extract for a particular input type.
String INVALID_EXTRACT_UNIT_CONVERTLET_ERROR =
"Was not expecting value '.*' for enumeration.*";

String INVALID_EXTRACT_UNIT_VALIDATION_ERROR =
"Cannot apply 'EXTRACT' to arguments of type .*'\n.*";

String LITERAL_OUT_OF_RANGE_MESSAGE =
"(?s).*Numeric literal.*out of range.*";

Expand Down
Loading

0 comments on commit 048fe95

Please sign in to comment.