You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by "dssysolyatin (via GitHub)" <gi...@apache.org> on 2026/04/01 08:46:30 UTC

[PR] [CALCITE-7457] Top-level VALUES and SELECT produce different validation results for the same expression [calcite]

dssysolyatin opened a new pull request, #4858:
URL: https://github.com/apache/calcite/pull/4858

   <!--
   Thanks for sending a pull request! Here are some critical tips for you:
   
   1. READ THE GUIDE FIRST: https://calcite.apache.org/develop/#contributing
      *For significant contributions, please discuss on the dev mailing list or Jira BEFORE coding.*
   
   2. JIRA IS USUALLY MANDATORY: Ensure you have created an issue on the Calcite Jira:
      https://issues.apache.org/jira/projects/CALCITE/issues
      *Check existing issues first to avoid duplicates.*
      *Note: A Jira is NOT required for typos and cosmetic changes (i.e., changes that are neither bugs nor features).*
   
   3. TIMING: Strongly recommended to create the Jira BEFORE you start writing code (e.g., a day or so before posting a PR).
      This gives others a chance to weigh in on your specification.
   
   4. CRITICAL CONSISTENCY RULE
      The following three items MUST match exactly in wording and meaning:
      (A) The Jira Issue Title
      (B) This Pull Request Title
      (C) Your Git Commit Message
   
      Format: [CALCITE-XXXX] <Description>
      Example: [CALCITE-0000] Add IF NOT EXISTS clause to CREATE TABLE
   
      Guidelines for a good Title:
      - Illustrate using SQL keywords (in ALL-CAPS) rather than Java method names.
      - Focus on the specification and user experience, not the implementation.
      - Make it clear whether it is a bug or a feature.
      - Use words like "should" to indicate desired behavior vs. current behavior (e.g., "Validator should not close model file").
   
   5. REPRODUCTION: If fixing a bug, please provide a concise SQL example or test case to reproduce the issue for a faster review.
   
   6. TESTING: Ensure `./gradlew build` passes and appropriate tests are added/updated.
   -->
   
   ## Jira Link
   
   [CALCITE-7457](https://issues.apache.org/jira/browse/CALCITE-7457)
   
   ## Changes Proposed
   - `VALUES` now routes through `validateQuery`, triggering `TableConstructorNamespace` validation (which calls `validateValues` and `inferUnknownTypes`). This matches the `SELECT` logic. The fix is a validateCall override in SqlValuesOperator.
     - `inferUnknownTypes` no longer descends into subqueries (node.isA(SqlKind.QUERY)). Each query handles its own `inferUnknownTypes` during its own validation.
    - Added additional test in existing tests which can be dropped. Just to show that now `SELECT` and `VALUES` behaves the same way


-- 
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: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-7457] VALUES and SELECT produce different validation results for the same expression [calcite]

Posted by "xiedeyantu (via GitHub)" <gi...@apache.org>.
xiedeyantu commented on PR #4858:
URL: https://github.com/apache/calcite/pull/4858#issuecomment-4229413088

   I have no other comments except for the above minor one, you can feel free to squash and merge.


-- 
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: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-7457] VALUES and SELECT produce different validation results for the same expression [calcite]

Posted by "dssysolyatin (via GitHub)" <gi...@apache.org>.
dssysolyatin merged PR #4858:
URL: https://github.com/apache/calcite/pull/4858


-- 
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: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-7457] VALUES and SELECT produce different validation results for the same expression [calcite]

Posted by "dssysolyatin (via GitHub)" <gi...@apache.org>.
dssysolyatin commented on PR #4858:
URL: https://github.com/apache/calcite/pull/4858#issuecomment-4378376024

   @mihaibudiu Yes, I plan to merge it. But after rereading the PR, I'm starting to think I'm not so confident about this change:
   
   - https://github.com/apache/calcite/pull/4858/changes#diff-c7f668762c07f4042575454e68c0774ea777477881601cafd78ddd038377c567R2309-R2311
   
    I'll run additional tests, probably on Friday, and if everything is OK, I'll merge it.


-- 
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: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-7457] VALUES and SELECT produce different validation results for the same expression [calcite]

Posted by "dssysolyatin (via GitHub)" <gi...@apache.org>.
dssysolyatin commented on code in PR #4858:
URL: https://github.com/apache/calcite/pull/4858#discussion_r3068126100


##########
core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java:
##########
@@ -8104,15 +8144,28 @@ void testGroupExpressionEquivalenceParams() {
         + "   overlaps (date '1-2-3', date '1-2-3')^\n"
         + "or false")
         .fails("(?s).*Cannot apply 'OVERLAPS' to arguments of type .*");
-    // row with 3 arguments as right argument to overlaps
+    // row with 3 arguments as right argument to overlaps.
+    // validateValues checks ROW structure before OVERLAPS validates,
+    // producing "Unequal number of entries in ROW expressions".
+    // SELECT produces the same error.
+    sql("select true or"
+        + " (date '1-2-3', date '1-2-3')"
+        + " overlaps ^(date '1-2-3', date '1-2-3', date '1-2-3')^"
+        + " or false from (values (0)) as t(x)")
+        .fails("(?s).*Unequal number of entries in ROW expressions.*");
     expr("true\n"
-        + "or ^(date '1-2-3', date '1-2-3')\n"
-        + "  overlaps (date '1-2-3', date '1-2-3', date '1-2-3')^\n"
+        + "or (date '1-2-3', date '1-2-3')\n"
+        + "  overlaps ^(date '1-2-3', date '1-2-3', date '1-2-3')^\n"
         + "or false")
-        .fails("(?s).*Cannot apply 'OVERLAPS' to arguments of type .*");
-    expr("^period (date '1-2-3', date '1-2-3')\n"
-        + "   overlaps (date '1-2-3', date '1-2-3', date '1-2-3')^")
-        .fails("(?s).*Cannot apply 'OVERLAPS' to arguments of type .*");
+        .fails("(?s).*Unequal number of entries in ROW expressions.*");

Review Comment:
   You are correct that the original query does not contain a ROW. However, during parsing, the OVERLAPS arguments are wrapped inside a ROW prior to any validation
   
   Currently, VALUES follows the proper execution order:
       a. Operand inference
       b. Operand validation
   
   Operand inference throws an exception, but it probably should not. It  should probably simply leave the types unfilled. Operand validation would then fail with something like
   
   ```
   Cannot apply 'OVERLAPS' to arguments of type '<RECORDTYPE(DATE EXPR$0, DATE EXPR$1)> OVERLAPS <RECORDTYPE(DATE EXPR$0, DATE EXPR$1, DATE EXPR$2)>'. Supported form(s): '(<DT>, <DT>) OVERLAPS (<DT>, <DT>)'
   '(<DT>, <DT>) OVERLAPS (<DT>, <INTERVAL>)'
   '(<DT>, <INTERVAL>) OVERLAPS (<DT>, <DT>)'
   '(<DT>, <INTERVAL>) OVERLAPS (<DT>, <INTERVAL>)'
   ```
   
   Regarding the regression, I'm not sure it is regression, since SELECT already had the same behavior. Also `SELECT * FROM VALUES()/INSERT/UPDATE/DELETE` behave the same as they call `validateValues` which first:
     1. Does operand inference
     2. And then does operand validation
   
   Ironically, `VALUES` expression (when `VALUES` is top level node) were the exception.



-- 
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: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-7457] VALUES and SELECT produce different validation results for the same expression [calcite]

Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #4858:
URL: https://github.com/apache/calcite/pull/4858#issuecomment-4467802725

   ## [![Quality Gate Passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-passed-20px.png 'Quality Gate Passed')](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4858) **Quality Gate passed**  
   Issues  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [1 New issue](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4858&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/accepted-16px.png '') [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4858&issueStatuses=ACCEPTED)
   
   Measures  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=4858&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [100.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4858&metric=new_coverage&view=list)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4858&metric=new_duplicated_lines_density&view=list)  
     
   <!-- sqra-placement-anchor -->
   [See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4858)
   
   


-- 
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: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-7457] VALUES and SELECT produce different validation results for the same expression [calcite]

Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #4858:
URL: https://github.com/apache/calcite/pull/4858#issuecomment-4229671675

   ## [![Quality Gate Passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-passed-20px.png 'Quality Gate Passed')](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4858) **Quality Gate passed**  
   Issues  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0 New issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4858&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/accepted-16px.png '') [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4858&issueStatuses=ACCEPTED)
   
   Measures  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=4858&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [100.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4858&metric=new_coverage&view=list)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4858&metric=new_duplicated_lines_density&view=list)  
     
   [See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4858)
   
   


-- 
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: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-7457] VALUES and SELECT produce different validation results for the same expression [calcite]

Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu commented on code in PR #4858:
URL: https://github.com/apache/calcite/pull/4858#discussion_r3068128680


##########
core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java:
##########
@@ -8104,15 +8144,28 @@ void testGroupExpressionEquivalenceParams() {
         + "   overlaps (date '1-2-3', date '1-2-3')^\n"
         + "or false")
         .fails("(?s).*Cannot apply 'OVERLAPS' to arguments of type .*");
-    // row with 3 arguments as right argument to overlaps
+    // row with 3 arguments as right argument to overlaps.
+    // validateValues checks ROW structure before OVERLAPS validates,
+    // producing "Unequal number of entries in ROW expressions".
+    // SELECT produces the same error.
+    sql("select true or"
+        + " (date '1-2-3', date '1-2-3')"
+        + " overlaps ^(date '1-2-3', date '1-2-3', date '1-2-3')^"
+        + " or false from (values (0)) as t(x)")
+        .fails("(?s).*Unequal number of entries in ROW expressions.*");
     expr("true\n"
-        + "or ^(date '1-2-3', date '1-2-3')\n"
-        + "  overlaps (date '1-2-3', date '1-2-3', date '1-2-3')^\n"
+        + "or (date '1-2-3', date '1-2-3')\n"
+        + "  overlaps ^(date '1-2-3', date '1-2-3', date '1-2-3')^\n"
         + "or false")
-        .fails("(?s).*Cannot apply 'OVERLAPS' to arguments of type .*");
-    expr("^period (date '1-2-3', date '1-2-3')\n"
-        + "   overlaps (date '1-2-3', date '1-2-3', date '1-2-3')^")
-        .fails("(?s).*Cannot apply 'OVERLAPS' to arguments of type .*");
+        .fails("(?s).*Unequal number of entries in ROW expressions.*");

Review Comment:
   Do you know why no checks were run in CI? 
   I would expect this error using ROW to appear in many more cases.
   



-- 
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: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-7457] VALUES and SELECT produce different validation results for the same expression [calcite]

Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu commented on code in PR #4858:
URL: https://github.com/apache/calcite/pull/4858#discussion_r3068147938


##########
core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java:
##########
@@ -8104,15 +8144,28 @@ void testGroupExpressionEquivalenceParams() {
         + "   overlaps (date '1-2-3', date '1-2-3')^\n"
         + "or false")
         .fails("(?s).*Cannot apply 'OVERLAPS' to arguments of type .*");
-    // row with 3 arguments as right argument to overlaps
+    // row with 3 arguments as right argument to overlaps.
+    // validateValues checks ROW structure before OVERLAPS validates,
+    // producing "Unequal number of entries in ROW expressions".
+    // SELECT produces the same error.
+    sql("select true or"
+        + " (date '1-2-3', date '1-2-3')"
+        + " overlaps ^(date '1-2-3', date '1-2-3', date '1-2-3')^"
+        + " or false from (values (0)) as t(x)")
+        .fails("(?s).*Unequal number of entries in ROW expressions.*");
     expr("true\n"
-        + "or ^(date '1-2-3', date '1-2-3')\n"
-        + "  overlaps (date '1-2-3', date '1-2-3', date '1-2-3')^\n"
+        + "or (date '1-2-3', date '1-2-3')\n"
+        + "  overlaps ^(date '1-2-3', date '1-2-3', date '1-2-3')^\n"
         + "or false")
-        .fails("(?s).*Cannot apply 'OVERLAPS' to arguments of type .*");
-    expr("^period (date '1-2-3', date '1-2-3')\n"
-        + "   overlaps (date '1-2-3', date '1-2-3', date '1-2-3')^")
-        .fails("(?s).*Cannot apply 'OVERLAPS' to arguments of type .*");
+        .fails("(?s).*Unequal number of entries in ROW expressions.*");

Review Comment:
   Can you please run all the tests to check that there aren't other tests affected by the ROW message? 



-- 
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: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-7457] VALUES and SELECT produce different validation results for the same expression [calcite]

Posted by "dssysolyatin (via GitHub)" <gi...@apache.org>.
dssysolyatin commented on code in PR #4858:
URL: https://github.com/apache/calcite/pull/4858#discussion_r3068180131


##########
core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java:
##########
@@ -8104,15 +8144,28 @@ void testGroupExpressionEquivalenceParams() {
         + "   overlaps (date '1-2-3', date '1-2-3')^\n"
         + "or false")
         .fails("(?s).*Cannot apply 'OVERLAPS' to arguments of type .*");
-    // row with 3 arguments as right argument to overlaps
+    // row with 3 arguments as right argument to overlaps.
+    // validateValues checks ROW structure before OVERLAPS validates,
+    // producing "Unequal number of entries in ROW expressions".
+    // SELECT produces the same error.
+    sql("select true or"
+        + " (date '1-2-3', date '1-2-3')"
+        + " overlaps ^(date '1-2-3', date '1-2-3', date '1-2-3')^"
+        + " or false from (values (0)) as t(x)")
+        .fails("(?s).*Unequal number of entries in ROW expressions.*");
     expr("true\n"
-        + "or ^(date '1-2-3', date '1-2-3')\n"
-        + "  overlaps (date '1-2-3', date '1-2-3', date '1-2-3')^\n"
+        + "or (date '1-2-3', date '1-2-3')\n"
+        + "  overlaps ^(date '1-2-3', date '1-2-3', date '1-2-3')^\n"
         + "or false")
-        .fails("(?s).*Cannot apply 'OVERLAPS' to arguments of type .*");
-    expr("^period (date '1-2-3', date '1-2-3')\n"
-        + "   overlaps (date '1-2-3', date '1-2-3', date '1-2-3')^")
-        .fails("(?s).*Cannot apply 'OVERLAPS' to arguments of type .*");
+        .fails("(?s).*Unequal number of entries in ROW expressions.*");

Review Comment:
   Those all ran before I added the LGTM-will-merge-soon label. Anyway, I made a small tweak based on https://github.com/apache/calcite/pull/4858#discussion_r3030989104. Re-running now



-- 
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: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-7457] VALUES and SELECT produce different validation results for the same expression [calcite]

Posted by "dssysolyatin (via GitHub)" <gi...@apache.org>.
dssysolyatin commented on code in PR #4858:
URL: https://github.com/apache/calcite/pull/4858#discussion_r3068140074


##########
core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java:
##########
@@ -1322,13 +1322,20 @@ void testLikeAndSimilarFails() {
     expr("values 1.0 + ^NULL^")
         .withTypeCoercion(false)
         .fails("(?s).*Illegal use of .NULL.*");
+    // SELECT produces DECIMAL(3, 1) because inferUnknownTypes infers
+    // NULL as DECIMAL(2, 1) via FIRST_KNOWN, and DECIMAL(2,1) + DECIMAL(2,1)
+    // = DECIMAL(3, 1)
+    sql("select 1.0 + NULL from (values (0)) as t(x)")
+        .columnType("DECIMAL(3, 1)");
     expr("values 1.0 + NULL")
-        .columnType("DECIMAL(2, 1)");
+        .columnType("DECIMAL(3, 1)");

Review Comment:
   I explained above, in the `SELECT` section, why this happens. I'm not saying it's the correct behavior and I would expect that `1.0 + NULL = NULL` with type `DECIMAL(2,1)` (common not null values type) in this case, but currently it works like that 



-- 
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: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-7457] VALUES and SELECT produce different validation results for the same expression [calcite]

Posted by "dssysolyatin (via GitHub)" <gi...@apache.org>.
dssysolyatin commented on code in PR #4858:
URL: https://github.com/apache/calcite/pull/4858#discussion_r3068137865


##########
core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java:
##########
@@ -8104,15 +8144,28 @@ void testGroupExpressionEquivalenceParams() {
         + "   overlaps (date '1-2-3', date '1-2-3')^\n"
         + "or false")
         .fails("(?s).*Cannot apply 'OVERLAPS' to arguments of type .*");
-    // row with 3 arguments as right argument to overlaps
+    // row with 3 arguments as right argument to overlaps.
+    // validateValues checks ROW structure before OVERLAPS validates,
+    // producing "Unequal number of entries in ROW expressions".
+    // SELECT produces the same error.
+    sql("select true or"
+        + " (date '1-2-3', date '1-2-3')"
+        + " overlaps ^(date '1-2-3', date '1-2-3', date '1-2-3')^"
+        + " or false from (values (0)) as t(x)")
+        .fails("(?s).*Unequal number of entries in ROW expressions.*");
     expr("true\n"
-        + "or ^(date '1-2-3', date '1-2-3')\n"
-        + "  overlaps (date '1-2-3', date '1-2-3', date '1-2-3')^\n"
+        + "or (date '1-2-3', date '1-2-3')\n"
+        + "  overlaps ^(date '1-2-3', date '1-2-3', date '1-2-3')^\n"
         + "or false")
-        .fails("(?s).*Cannot apply 'OVERLAPS' to arguments of type .*");
-    expr("^period (date '1-2-3', date '1-2-3')\n"
-        + "   overlaps (date '1-2-3', date '1-2-3', date '1-2-3')^")
-        .fails("(?s).*Cannot apply 'OVERLAPS' to arguments of type .*");
+        .fails("(?s).*Unequal number of entries in ROW expressions.*");

Review Comment:
   Ah, this is because I add `LGMT-will-merge-soon` after first approve, it run CI one more time but with specific CI tests



-- 
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: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-7457] VALUES and SELECT produce different validation results for the same expression [calcite]

Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu commented on code in PR #4858:
URL: https://github.com/apache/calcite/pull/4858#discussion_r3068069451


##########
core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java:
##########
@@ -1322,13 +1322,20 @@ void testLikeAndSimilarFails() {
     expr("values 1.0 + ^NULL^")
         .withTypeCoercion(false)
         .fails("(?s).*Illegal use of .NULL.*");
+    // SELECT produces DECIMAL(3, 1) because inferUnknownTypes infers
+    // NULL as DECIMAL(2, 1) via FIRST_KNOWN, and DECIMAL(2,1) + DECIMAL(2,1)
+    // = DECIMAL(3, 1)
+    sql("select 1.0 + NULL from (values (0)) as t(x)")
+        .columnType("DECIMAL(3, 1)");
     expr("values 1.0 + NULL")
-        .columnType("DECIMAL(2, 1)");
+        .columnType("DECIMAL(3, 1)");

Review Comment:
   Why should this inferred type be different?
   Something is off.



-- 
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: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-7457] VALUES and SELECT produce different validation results for the same expression [calcite]

Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu commented on PR #4858:
URL: https://github.com/apache/calcite/pull/4858#issuecomment-4367697995

   @dssysolyatin do you plan to merge this?


-- 
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: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-7457] VALUES and SELECT produce different validation results for the same expression [calcite]

Posted by "dssysolyatin (via GitHub)" <gi...@apache.org>.
dssysolyatin commented on code in PR #4858:
URL: https://github.com/apache/calcite/pull/4858#discussion_r3068140074


##########
core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java:
##########
@@ -1322,13 +1322,20 @@ void testLikeAndSimilarFails() {
     expr("values 1.0 + ^NULL^")
         .withTypeCoercion(false)
         .fails("(?s).*Illegal use of .NULL.*");
+    // SELECT produces DECIMAL(3, 1) because inferUnknownTypes infers
+    // NULL as DECIMAL(2, 1) via FIRST_KNOWN, and DECIMAL(2,1) + DECIMAL(2,1)
+    // = DECIMAL(3, 1)
+    sql("select 1.0 + NULL from (values (0)) as t(x)")
+        .columnType("DECIMAL(3, 1)");
     expr("values 1.0 + NULL")
-        .columnType("DECIMAL(2, 1)");
+        .columnType("DECIMAL(3, 1)");

Review Comment:
   I explained above, in the `SELECT` section, why this happens. I'm not saying it's the correct behavior and I would expect that `1.0 + NULL = NULL` of type `1.0` in this case, but currently it works like that 



-- 
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: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-7457] VALUES and SELECT produce different validation results for the same expression [calcite]

Posted by "dssysolyatin (via GitHub)" <gi...@apache.org>.
dssysolyatin commented on code in PR #4858:
URL: https://github.com/apache/calcite/pull/4858#discussion_r3068145496


##########
core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java:
##########
@@ -1322,13 +1322,20 @@ void testLikeAndSimilarFails() {
     expr("values 1.0 + ^NULL^")
         .withTypeCoercion(false)
         .fails("(?s).*Illegal use of .NULL.*");
+    // SELECT produces DECIMAL(3, 1) because inferUnknownTypes infers
+    // NULL as DECIMAL(2, 1) via FIRST_KNOWN, and DECIMAL(2,1) + DECIMAL(2,1)
+    // = DECIMAL(3, 1)
+    sql("select 1.0 + NULL from (values (0)) as t(x)")
+        .columnType("DECIMAL(3, 1)");
     expr("values 1.0 + NULL")
-        .columnType("DECIMAL(2, 1)");
+        .columnType("DECIMAL(3, 1)");

Review Comment:
   Right, if you open task description - https://issues.apache.org/jira/browse/CALCITE-7457. It will contain test case for main branch



-- 
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: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-7457] VALUES and SELECT produce different validation results for the same expression [calcite]

Posted by "dssysolyatin (via GitHub)" <gi...@apache.org>.
dssysolyatin commented on code in PR #4858:
URL: https://github.com/apache/calcite/pull/4858#discussion_r3068145496


##########
core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java:
##########
@@ -1322,13 +1322,20 @@ void testLikeAndSimilarFails() {
     expr("values 1.0 + ^NULL^")
         .withTypeCoercion(false)
         .fails("(?s).*Illegal use of .NULL.*");
+    // SELECT produces DECIMAL(3, 1) because inferUnknownTypes infers
+    // NULL as DECIMAL(2, 1) via FIRST_KNOWN, and DECIMAL(2,1) + DECIMAL(2,1)
+    // = DECIMAL(3, 1)
+    sql("select 1.0 + NULL from (values (0)) as t(x)")
+        .columnType("DECIMAL(3, 1)");
     expr("values 1.0 + NULL")
-        .columnType("DECIMAL(2, 1)");
+        .columnType("DECIMAL(3, 1)");

Review Comment:
   Right, if you open task description - https://issues.apache.org/jira/browse/CALCITE-7457. You can see test case for main branch



-- 
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: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-7457] VALUES and SELECT produce different validation results for the same expression [calcite]

Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #4858:
URL: https://github.com/apache/calcite/pull/4858#issuecomment-4484281613

   ## [![Quality Gate Passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-passed-20px.png 'Quality Gate Passed')](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4858) **Quality Gate passed**  
   Issues  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0 New issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4858&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/accepted-16px.png '') [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4858&issueStatuses=ACCEPTED)
   
   Measures  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=4858&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [100.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4858&metric=new_coverage&view=list)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4858&metric=new_duplicated_lines_density&view=list)  
     
   <!-- sqra-placement-anchor -->
   [See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4858)
   
   


-- 
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: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-7457] VALUES and SELECT produce different validation results for the same expression [calcite]

Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #4858:
URL: https://github.com/apache/calcite/pull/4858#issuecomment-4484240749

   ## [![Quality Gate Passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-passed-20px.png 'Quality Gate Passed')](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4858) **Quality Gate passed**  
   Issues  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [1 New issue](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4858&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/accepted-16px.png '') [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4858&issueStatuses=ACCEPTED)
   
   Measures  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=4858&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [100.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4858&metric=new_coverage&view=list)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4858&metric=new_duplicated_lines_density&view=list)  
     
   <!-- sqra-placement-anchor -->
   [See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4858)
   
   


-- 
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: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-7457] VALUES and SELECT produce different validation results for the same expression [calcite]

Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu commented on code in PR #4858:
URL: https://github.com/apache/calcite/pull/4858#discussion_r3068034237


##########
core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java:
##########
@@ -8104,15 +8144,28 @@ void testGroupExpressionEquivalenceParams() {
         + "   overlaps (date '1-2-3', date '1-2-3')^\n"
         + "or false")
         .fails("(?s).*Cannot apply 'OVERLAPS' to arguments of type .*");
-    // row with 3 arguments as right argument to overlaps
+    // row with 3 arguments as right argument to overlaps.
+    // validateValues checks ROW structure before OVERLAPS validates,
+    // producing "Unequal number of entries in ROW expressions".
+    // SELECT produces the same error.
+    sql("select true or"
+        + " (date '1-2-3', date '1-2-3')"
+        + " overlaps ^(date '1-2-3', date '1-2-3', date '1-2-3')^"
+        + " or false from (values (0)) as t(x)")
+        .fails("(?s).*Unequal number of entries in ROW expressions.*");
     expr("true\n"
-        + "or ^(date '1-2-3', date '1-2-3')\n"
-        + "  overlaps (date '1-2-3', date '1-2-3', date '1-2-3')^\n"
+        + "or (date '1-2-3', date '1-2-3')\n"
+        + "  overlaps ^(date '1-2-3', date '1-2-3', date '1-2-3')^\n"
         + "or false")
-        .fails("(?s).*Cannot apply 'OVERLAPS' to arguments of type .*");
-    expr("^period (date '1-2-3', date '1-2-3')\n"
-        + "   overlaps (date '1-2-3', date '1-2-3', date '1-2-3')^")
-        .fails("(?s).*Cannot apply 'OVERLAPS' to arguments of type .*");
+        .fails("(?s).*Unequal number of entries in ROW expressions.*");

Review Comment:
   This is a regression, there is not ROW expression in the original program. The user will have difficulties finding the location of this error.



-- 
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: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-7457] VALUES and SELECT produce different validation results for the same expression [calcite]

Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu commented on code in PR #4858:
URL: https://github.com/apache/calcite/pull/4858#discussion_r3068144066


##########
core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java:
##########
@@ -1322,13 +1322,20 @@ void testLikeAndSimilarFails() {
     expr("values 1.0 + ^NULL^")
         .withTypeCoercion(false)
         .fails("(?s).*Illegal use of .NULL.*");
+    // SELECT produces DECIMAL(3, 1) because inferUnknownTypes infers
+    // NULL as DECIMAL(2, 1) via FIRST_KNOWN, and DECIMAL(2,1) + DECIMAL(2,1)
+    // = DECIMAL(3, 1)
+    sql("select 1.0 + NULL from (values (0)) as t(x)")
+        .columnType("DECIMAL(3, 1)");
     expr("values 1.0 + NULL")
-        .columnType("DECIMAL(2, 1)");
+        .columnType("DECIMAL(3, 1)");

Review Comment:
   So you say SELECT 1.0 + NULL already was DECIMAL(3,1), but when used in VALUES the result was different?



-- 
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: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-7457] VALUES and SELECT produce different validation results for the same expression [calcite]

Posted by "dssysolyatin (via GitHub)" <gi...@apache.org>.
dssysolyatin commented on PR #4858:
URL: https://github.com/apache/calcite/pull/4858#issuecomment-4467757398

   Added a few more tests and ran some external ones to make sure nothing's broken. If no one has concerns, I'll squash and merge on Tuesday


-- 
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: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-7457] VALUES and SELECT produce different validation results for the same expression [calcite]

Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #4858:
URL: https://github.com/apache/calcite/pull/4858#issuecomment-4169559071

   ## [![Quality Gate Passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-passed-20px.png 'Quality Gate Passed')](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4858) **Quality Gate passed**  
   Issues  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0 New issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4858&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/accepted-16px.png '') [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4858&issueStatuses=ACCEPTED)
   
   Measures  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=4858&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [100.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4858&metric=new_coverage&view=list)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4858&metric=new_duplicated_lines_density&view=list)  
     
   [See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4858)
   
   


-- 
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: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-7457] VALUES and SELECT produce different validation results for the same expression [calcite]

Posted by "xiedeyantu (via GitHub)" <gi...@apache.org>.
xiedeyantu commented on code in PR #4858:
URL: https://github.com/apache/calcite/pull/4858#discussion_r3030989104


##########
core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java:
##########
@@ -7924,6 +7936,34 @@ void testGroupExpressionEquivalenceParams() {
         .ok();
   }
 
+  /** Test case for
+   * <a href="https://issues.apache.org/jira/browse/CALCITE-7457">[CALCITE-7457]
+   * Top-level VALUES and SELECT produce different validation results

Review Comment:
   The javadoc should be removed "Top-level"?



-- 
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: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org