alex-plekhanov commented on code in PR #13324:
URL: https://github.com/apache/ignite/pull/13324#discussion_r3539056443
##########
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/prepare/IgniteSqlValidator.java:
##########
@@ -152,6 +152,13 @@ public IgniteSqlValidator(
validateUpdateFields(call);
super.validateUpdate(call);
+
+ SqlSelect srcSelect = call.getSourceSelect();
+ SqlValidatorScope scope = getWhereScope(srcSelect);
+
+ for (SqlNode expr : call.getSourceExpressionList()) {
+ deriveType(scope, expr);
Review Comment:
Redundant braces
##########
modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/integration/OperatorsExtensionIntegrationTest.java:
##########
@@ -158,6 +167,56 @@ public void testCustomAggregateFunction() {
.check();
}
+ /** */
+ @Test
Review Comment:
Too complicated test. We can reproduce this problem much simplier without
any extensions with test like:
```
@Test
public void testDmlIntervalArithmetic() {
sql("CREATE TABLE test(ts TIMESTAMP)");
sql("INSERT INTO test VALUES (?)", Timestamp.valueOf("2021-01-01
00:00:01"));
sql("UPDATE test SET ts = ts - INTERVAL 1 SECOND");
assertQuery("SELECT * FROM
test").returns(Timestamp.valueOf("2021-01-01 00:00:00")).check();
}
```
WIth this code we can prove that standard Calcite date arithmetic has the
same problem (at least we can easily check it with pure Apache Calcite), report
bug to Calcite, and remove our patch later.
##########
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/prepare/IgniteSqlValidator.java:
##########
@@ -152,6 +152,13 @@ public IgniteSqlValidator(
validateUpdateFields(call);
super.validateUpdate(call);
+
+ SqlSelect srcSelect = call.getSourceSelect();
+ SqlValidatorScope scope = getWhereScope(srcSelect);
Review Comment:
Why `where` scope? Looks like `selectItems` are validated within `select`
scope and `selectItems` derived from `sourceExpressionList`. Maybe
`sourceExpressionList` also should be validated within `select` scope? (I'm not
quite sure about this. Maybe there is a reason for `where` scope?)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]