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]

Reply via email to