Skip to content

Commit e810d8b

Browse files
committed
[CALCITE-7418] SqlOverlapsOperator does not reject some illegal comparisons (e.g., TIME vs DATE)
Signed-off-by: Mihai Budiu <mbudiu@feldera.com>
1 parent a443737 commit e810d8b

2 files changed

Lines changed: 119 additions & 2 deletions

File tree

core/src/main/java/org/apache/calcite/sql/fun/SqlOverlapsOperator.java

Lines changed: 56 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,8 @@
3333

3434
import com.google.common.collect.ImmutableList;
3535

36+
import org.checkerframework.checker.nullness.qual.Nullable;
37+
3638
/**
3739
* SqlOverlapsOperator represents the SQL:1999 standard {@code OVERLAPS}
3840
* function. Determines whether two anchored time intervals overlap.
@@ -77,8 +79,30 @@ void arg(SqlWriter writer, SqlCall call, int leftPrec, int rightPrec, int i) {
7779
return SqlOperandCountRanges.of(2);
7880
}
7981

82+
/**
83+
* Returns a template describing how the operator signature is to be built.
84+
*
85+
* @param operandsCount is used with functions that can take a variable
86+
* number of operands
87+
* @return signature template, where {0} is the operator name and {1}, {2}, etc are operands
88+
*/
89+
@Override public @Nullable String getSignatureTemplate(final int operandsCount) {
90+
// This function can be called in 3 ways:
91+
// - as a binary operator; format like a binary operator left OP right
92+
// - as a ternary operator, for (a, b) CONTAINS c
93+
// - as a quaternary operator, for (a, b) OVERLAPS (c, d)
94+
if (operandsCount == 2) {
95+
return "{1} {0} {2}";
96+
} else if (operandsCount == 3) {
97+
return "({1}, {2}) {0} {3}";
98+
} else if (operandsCount == 4) {
99+
return "({1}, {2}) {0} ({3}, {4})";
100+
}
101+
throw new IllegalArgumentException("Unexpected operand count " + operandsCount);
102+
}
103+
80104
@Override public String getAllowedSignatures(String opName) {
81-
final String d = "DATETIME";
105+
final String d = "DT";
82106
final String i = "INTERVAL";
83107
String[] typeNames = {
84108
d, d,
@@ -96,6 +120,16 @@ void arg(SqlWriter writer, SqlCall call, int leftPrec, int rightPrec, int i) {
96120
SqlUtil.getAliasedSignature(this, opName,
97121
ImmutableList.of(d, typeNames[y], d, typeNames[y + 1])));
98122
}
123+
if (opName.equalsIgnoreCase("contains")) {
124+
// Two more forms supported: (DT, DT) CONTAINS DT and (DT, INTERVAL) CONTAINS DT
125+
ret.append(NL);
126+
ret.append(SqlUtil.getAliasedSignature(this, opName, ImmutableList.of(d, d, d)));
127+
ret.append(NL);
128+
ret.append(SqlUtil.getAliasedSignature(this, opName, ImmutableList.of(d, i, d)));
129+
}
130+
ret.append(NL);
131+
ret.append("Where 'DT' is one of 'DATE', 'TIME', or 'TIMESTAMP', "
132+
+ "the same for all arguments.");
99133
return ret.toString();
100134
}
101135

@@ -108,11 +142,21 @@ void arg(SqlWriter writer, SqlCall call, int leftPrec, int rightPrec, int i) {
108142
final SqlSingleOperandTypeChecker rightChecker;
109143
switch (kind) {
110144
case CONTAINS:
145+
// A ternary call of the form (a, b) CONTAINS c
146+
// OR a quaternary call of the form (a, b) CONTAINS (c, d)
111147
rightChecker = OperandTypes.PERIOD_OR_DATETIME;
112148
break;
113-
default:
149+
case OVERLAPS:
150+
case PRECEDES:
151+
case IMMEDIATELY_PRECEDES:
152+
case SUCCEEDS:
153+
case IMMEDIATELY_SUCCEEDS:
154+
case PERIOD_EQUALS:
155+
// Always a quaternary call of the form (a, b) OVERLAPS (c, d)
114156
rightChecker = OperandTypes.PERIOD;
115157
break;
158+
default:
159+
throw new IllegalArgumentException("Unexpected operation " + kind);
116160
}
117161
if (!rightChecker.checkSingleOperandType(callBinding,
118162
callBinding.operand(1), 0, throwOnFailure)) {
@@ -121,6 +165,7 @@ void arg(SqlWriter writer, SqlCall call, int leftPrec, int rightPrec, int i) {
121165
final RelDataType t0 = callBinding.getOperandType(0);
122166
final RelDataType t1 = callBinding.getOperandType(1);
123167
if (!SqlTypeUtil.isDatetime(t1)) {
168+
// "quaternary" call, of the form (a, b) OVERLAPS (c, d)
124169
final RelDataType t00 = t0.getFieldList().get(0).getType();
125170
final RelDataType t10 = t1.getFieldList().get(0).getType();
126171
if (!SqlTypeUtil.sameNamedType(t00, t10)) {
@@ -129,6 +174,15 @@ void arg(SqlWriter writer, SqlCall call, int leftPrec, int rightPrec, int i) {
129174
}
130175
return false;
131176
}
177+
} else {
178+
// "ternary" call, of the form (a, b) CONTAINS c
179+
final RelDataType t00 = t0.getFieldList().get(0).getType();
180+
if (!SqlTypeUtil.sameNamedType(t00, t1)) {
181+
if (throwOnFailure) {
182+
throw callBinding.newValidationSignatureError();
183+
}
184+
return false;
185+
}
132186
}
133187
return true;
134188
}

testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3307,6 +3307,69 @@ static void checkOverlaps(OverlapChecker c) {
33073307
c.isTrue("($3,$0) IMMEDIATELY SUCCEEDS ($0,$0)");
33083308
}
33093309

3310+
/** Test cases for <a href="https://issues.apache.org/jira/browse/CALCITE-7418">[CALCITE-7418]
3311+
* SqlOverlapsOperator does not reject some illegal comparisons (e.g., TIME vs DATE)</a>. */
3312+
@Test void testNegativePeriodOperators() {
3313+
final String containsError = "Supported form\\(s\\): "
3314+
+ "'\\(<DT>, <DT>\\) CONTAINS \\(<DT>, <DT>\\)'\\n"
3315+
+ "'\\(<DT>, <DT>\\) CONTAINS \\(<DT>, <INTERVAL>\\)'\\n"
3316+
+ "'\\(<DT>, <INTERVAL>\\) CONTAINS \\(<DT>, <DT>\\)'\\n"
3317+
+ "'\\(<DT>, <INTERVAL>\\) CONTAINS \\(<DT>, <INTERVAL>\\)'\\n"
3318+
+ "'\\(<DT>, <DT>\\) CONTAINS <DT>'\\n"
3319+
+ "'\\(<DT>, <INTERVAL>\\) CONTAINS <DT>'\\n"
3320+
+ "Where 'DT' is one of 'DATE', 'TIME', or 'TIMESTAMP', the same for all arguments.";
3321+
final SqlOperatorFixture f = fixture();
3322+
f.checkFails("^(DATE '2020-10-10', DATE '2021-10-10') CONTAINS TIME '10:00:00'^",
3323+
"Cannot apply 'CONTAINS' to arguments of type "
3324+
+ "'<RECORDTYPE\\(DATE EXPR\\$0, DATE EXPR\\$1\\)> CONTAINS <TIME\\(0\\)>'\\. "
3325+
+ containsError, false);
3326+
f.checkFails("^(DATE '2020-10-10', DATE '2021-10-10') CONTAINS "
3327+
+ "TIMESTAMP '2010-01-01 10:00:00'^",
3328+
"Cannot apply 'CONTAINS' to arguments of type "
3329+
+ "'<RECORDTYPE\\(DATE EXPR\\$0, DATE EXPR\\$1\\)> CONTAINS <TIMESTAMP\\(0\\)>'\\. "
3330+
+ containsError, false);
3331+
f.checkFails("^(DATE '2020-10-10', TIMESTAMP '2021-10-10 00:00:00') "
3332+
+ "CONTAINS TIMESTAMP '2010-01-01 10:00:00'^",
3333+
"Cannot apply 'CONTAINS' to arguments of type "
3334+
+ "'<RECORDTYPE\\(DATE EXPR\\$0, TIMESTAMP\\(0\\) EXPR\\$1\\)> "
3335+
+ "CONTAINS <TIMESTAMP\\(0\\)>'\\. "
3336+
+ containsError, false);
3337+
f.checkFails("^(TIME '10:10:10', DATE '2021-10-10') CONTAINS TIME '10:00:00'^",
3338+
"Cannot apply 'CONTAINS' to arguments of type "
3339+
+ "'<RECORDTYPE\\(TIME\\(0\\) EXPR\\$0, DATE EXPR\\$1\\)> CONTAINS <TIME\\(0\\)>'\\. "
3340+
+ containsError, false);
3341+
f.checkFails("^(TIME '10:10:10', DATE '2021-10-10') CONTAINS TIMESTAMP '2010-02-02 10:00:00'^",
3342+
"Cannot apply 'CONTAINS' to arguments of type "
3343+
+ "'<RECORDTYPE\\(TIME\\(0\\) EXPR\\$0, DATE EXPR\\$1\\)> "
3344+
+ "CONTAINS <TIMESTAMP\\(0\\)>'\\. "
3345+
+ containsError, false);
3346+
final String overlapsError = "Supported form\\(s\\): "
3347+
+ "'\\(<DT>, <DT>\\) OVERLAPS \\(<DT>, <DT>\\)'\\n"
3348+
+ "'\\(<DT>, <DT>\\) OVERLAPS \\(<DT>, <INTERVAL>\\)'\\n"
3349+
+ "'\\(<DT>, <INTERVAL>\\) OVERLAPS \\(<DT>, <DT>\\)'\\n"
3350+
+ "'\\(<DT>, <INTERVAL>\\) OVERLAPS \\(<DT>, <INTERVAL>\\)'\\n"
3351+
+ "Where 'DT' is one of 'DATE', 'TIME', or 'TIMESTAMP', the same for all arguments.";
3352+
f.checkFails("^(TIME '10:10:10', DATE '2021-10-10') OVERLAPS "
3353+
+ "(TIMESTAMP '2010-02-02 10:00:00', TIME '10:00:00')^",
3354+
"Cannot apply 'OVERLAPS' to arguments of type "
3355+
+ "'<RECORDTYPE\\(TIME\\(0\\) EXPR\\$0, DATE EXPR\\$1\\)> "
3356+
+ "OVERLAPS <RECORDTYPE\\(TIMESTAMP\\(0\\) EXPR\\$0, TIME\\(0\\) EXPR\\$1\\)>'\\. "
3357+
+ overlapsError, false);
3358+
f.checkFails("^(TIME '10:10:10', DATE '2021-10-10') "
3359+
+ "OVERLAPS (TIME '10:00:00', DATE '2020-01-01')^",
3360+
"Cannot apply 'OVERLAPS' to arguments of type "
3361+
+ "'<RECORDTYPE\\(TIME\\(0\\) EXPR\\$0, DATE EXPR\\$1\\)> "
3362+
+ "OVERLAPS <RECORDTYPE\\(TIME\\(0\\) EXPR\\$0, DATE EXPR\\$1\\)>'\\. "
3363+
+ overlapsError, false);
3364+
final String precedesError = overlapsError.replace("OVERLAPS", "PRECEDES");
3365+
f.checkFails("^(TIME '10:10:10', DATE '2021-10-10') "
3366+
+ "PRECEDES (TIME '10:00:00', TIME '10:10:10')^",
3367+
"Cannot apply 'PRECEDES' to arguments of type "
3368+
+ "'<RECORDTYPE\\(TIME\\(0\\) EXPR\\$0, DATE EXPR\\$1\\)> "
3369+
+ "PRECEDES <RECORDTYPE\\(TIME\\(0\\) EXPR\\$0, TIME\\(0\\) EXPR\\$1\\)>'\\. "
3370+
+ precedesError, false);
3371+
}
3372+
33103373
@Test void testLessThanOperator() {
33113374
final SqlOperatorFixture f = fixture();
33123375
f.setFor(SqlStdOperatorTable.LESS_THAN, VmName.EXPAND);

0 commit comments

Comments
 (0)