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

[PR] [CALCITE-6770] Preserve column names when casts are inserted in projects [calcite]

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

   (no comment)


-- 
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-6770] Preserve column names when casts are inserted in projects [calcite]

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


##########
core/src/test/java/org/apache/calcite/test/JdbcAdapterTest.java:
##########
@@ -127,15 +127,15 @@ class JdbcAdapterTest {
             + "      JdbcFilter(condition=[<($0, 10)])\n"
             + "        JdbcTableScan(table=[[foodmart, store]])\n"
             + "  JdbcToEnumerableConverter\n"
-            + "    JdbcProject(EXPR$0=[CAST($1):VARCHAR(30)])\n"
+            + "    JdbcProject(ENAME=[CAST($1):VARCHAR(30)])\n"
             + "      JdbcFilter(condition=[>(CAST($0):INTEGER NOT NULL, 10)])\n"
             + "        JdbcTableScan(table=[[SCOTT, EMP]])")
         .runs()
         .enable(CalciteAssert.DB == CalciteAssert.DatabaseInstance.HSQLDB)
         .planHasSql("SELECT \"store_name\"\n"
                 + "FROM \"foodmart\".\"store\"\n"
                 + "WHERE \"store_id\" < 10")
-        .planHasSql("SELECT CAST(\"ENAME\" AS VARCHAR(30))\n"
+        .planHasSql("SELECT CAST(\"ENAME\" AS VARCHAR(30)) AS \"ENAME\"\n"

Review Comment:
   This preserves the names from the original select list that is being transformed. 
   If the original list had duplicates, this one should too.
   But I expect that this cannot happen at this point.



-- 
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-6770] Preserve column names when casts are inserted in projects [calcite]

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

   It's used in selectlist, case statement, in, and source row, all in `TypeCoercionImpl`.


-- 
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-6770] Preserve column names when casts are inserted in projects [calcite]

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

   Can you please file an issue about this and assign it to me?
   https://issues.apache.org/jira/projects/CALCITE/issues
   (If you can't assign it I will find it anyway)


-- 
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-6770] Preserve column names when casts are inserted in projects [calcite]

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

   This actually breaks the following SQL:
   ```
   SELECT
     CASE
       WHEN ID IS NOT NULL
       THEN foo
       ELSE CAST(ID AS VARCHAR)
     END AS ID
   FROM testTable;
   ```
   because it would rewrite the THEN clause to `THEN CAST(foo AS VARCHAR) AS foo` which is no longer valid SQL. Not all places accept identifiers accept alias.


-- 
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-6770] Preserve column names when casts are inserted in projects [calcite]

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

   I need to sign up for an account to create an issue. That's a lot of effort to be able to file a bug...  Generally speaking, if your goal is to make sure in SELECT the name would be preserved after adding CAST to an identifier, you should handle that in SELECT. Adding it to type coercion can cause a lot of issues, this is just one way of breaking it. Theoretically you can have situations where `a + b` is rewritten to `CAST(a AS xxx) as a + b`, which, is also invalid SQL. 


-- 
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-6770] Preserve column names when casts are inserted in projects [calcite]

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

   That's what I am saying, the function is called `coerceColumnType`, so I assumed it's part of a `SELECT` item.
   I will try to create a reproduction and if I succeed I will file an issue.


-- 
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-6770] Preserve column names when casts are inserted in projects [calcite]

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

   The change is in coerceColumnType, so I was assuming your case does not apply, since foo is not a column.


-- 
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-6770] Preserve column names when casts are inserted in projects [calcite]

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

   I have filed https://issues.apache.org/jira/browse/CALCITE-7072 and I will submit a fix shortly.
   Thank you for the bug report.
   But JIRA is really the place to file issues, people may not monitor closed PRs.
   


-- 
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-6770] Preserve column names when casts are inserted in projects [calcite]

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

   @rongrong can you validate the fix in https://github.com/apache/calcite/pull/4439?
   


-- 
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-6770] Preserve column names when casts are inserted in projects [calcite]

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


-- 
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-6770] Preserve column names when casts are inserted in projects [calcite]

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


##########
core/src/test/java/org/apache/calcite/test/JdbcAdapterTest.java:
##########
@@ -127,15 +127,15 @@ class JdbcAdapterTest {
             + "      JdbcFilter(condition=[<($0, 10)])\n"
             + "        JdbcTableScan(table=[[foodmart, store]])\n"
             + "  JdbcToEnumerableConverter\n"
-            + "    JdbcProject(EXPR$0=[CAST($1):VARCHAR(30)])\n"
+            + "    JdbcProject(ENAME=[CAST($1):VARCHAR(30)])\n"
             + "      JdbcFilter(condition=[>(CAST($0):INTEGER NOT NULL, 10)])\n"
             + "        JdbcTableScan(table=[[SCOTT, EMP]])")
         .runs()
         .enable(CalciteAssert.DB == CalciteAssert.DatabaseInstance.HSQLDB)
         .planHasSql("SELECT \"store_name\"\n"
                 + "FROM \"foodmart\".\"store\"\n"
                 + "WHERE \"store_id\" < 10")
-        .planHasSql("SELECT CAST(\"ENAME\" AS VARCHAR(30))\n"
+        .planHasSql("SELECT CAST(\"ENAME\" AS VARCHAR(30)) AS \"ENAME\"\n"

Review Comment:
   I mean, when the names are inserted, the program has been validated.



-- 
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-6770] Preserve column names when casts are inserted in projects [calcite]

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


##########
core/src/test/java/org/apache/calcite/test/JdbcAdapterTest.java:
##########
@@ -127,15 +127,15 @@ class JdbcAdapterTest {
             + "      JdbcFilter(condition=[<($0, 10)])\n"
             + "        JdbcTableScan(table=[[foodmart, store]])\n"
             + "  JdbcToEnumerableConverter\n"
-            + "    JdbcProject(EXPR$0=[CAST($1):VARCHAR(30)])\n"
+            + "    JdbcProject(ENAME=[CAST($1):VARCHAR(30)])\n"
             + "      JdbcFilter(condition=[>(CAST($0):INTEGER NOT NULL, 10)])\n"
             + "        JdbcTableScan(table=[[SCOTT, EMP]])")
         .runs()
         .enable(CalciteAssert.DB == CalciteAssert.DatabaseInstance.HSQLDB)
         .planHasSql("SELECT \"store_name\"\n"
                 + "FROM \"foodmart\".\"store\"\n"
                 + "WHERE \"store_id\" < 10")
-        .planHasSql("SELECT CAST(\"ENAME\" AS VARCHAR(30))\n"
+        .planHasSql("SELECT CAST(\"ENAME\" AS VARCHAR(30)) AS \"ENAME\"\n"

Review Comment:
   What if there are duplicate `ENAME` in this case? Will the 2nd `ENAME` be `ENAME0`?



##########
core/src/test/java/org/apache/calcite/test/SqlToRelConverterTest.java:
##########
@@ -257,6 +257,15 @@ public static void checkActualAndReferenceFiles() {
     sql(sql).ok();
   }
 
+  /** Test case for
+   * <a href="https://issues.apache.org/jira/browse/CALCITE-6770">[CALCITE-6770]
+   * Preserve column names when casts are inserted in projects</a>. */
+  @Test void testCastNames() {
+    final String sql = "SELECT * FROM (SELECT empno, 'x' AS X FROM emp) "
+        + "UNION ALL (SELECT empno, 'xx' AS X from EMP)";

Review Comment:
   from `emp`?



-- 
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-6770] Preserve column names when casts are inserted in projects [calcite]

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

   ## [![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=4130) **Quality Gate passed**  
   Issues  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [2 New issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4130&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=4130&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=4130&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=4130&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=4130&metric=new_duplicated_lines_density&view=list)  
     
   [See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4130)
   
   


-- 
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-6770] Preserve column names when casts are inserted in projects [calcite]

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

   I see that the AS is inserted in the CASE statement, but I have trouble creating a test that fails. How did you observe 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-6770] Preserve column names when casts are inserted in projects [calcite]

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

   I have added a tiny commit to fix a bug I have introduced in https://github.com/apache/calcite/pull/4116.
   No tests are affected, since the calcite simplification code removes the casts in question before they reach the bug.


-- 
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-6770] Preserve column names when casts are inserted in projects [calcite]

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

   > This actually breaks the following SQL:
   > 
   > ```
   > 
   > SELECT
   > 
   >   CASE
   > 
   >     WHEN ID IS NOT NULL
   > 
   >     THEN foo
   > 
   >     ELSE CAST(ID AS VARCHAR)
   > 
   >   END AS ID
   > 
   > FROM testTable;
   > 
   > ```
   > 
   > because it would rewrite the THEN clause to `THEN CAST(foo AS VARCHAR) AS foo` which is no longer valid SQL. Not all places accept identifiers accept alias.
   
   This sql will fail. 


-- 
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-6770] Preserve column names when casts are inserted in projects [calcite]

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

   I figured out how to write a test, one way to do it is to configure the validator withIdentifierExpansion(true).
   The rest should be easy.


-- 
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-6770] Preserve column names when casts are inserted in projects [calcite]

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

   I understand that, but i could not write a test that fails. Can you give more details about the configuration of your compiler and how the error manifests itself?


-- 
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