You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by "guestli (via GitHub)" <gi...@apache.org> on 2023/08/18 11:49:07 UTC

[GitHub] [calcite] guestli opened a new pull request, #3375: [CALCITE-5583] JDBC adapter does not generate 'SELECT *' when duplicate field names are possible

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

   (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


[GitHub] [calcite] guestli commented on a diff in pull request #3375: [CALCITE-5583] JDBC adapter does not generate 'SELECT *' when duplicate field names are possible

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


##########
core/src/main/java/org/apache/calcite/rel/rel2sql/SqlImplementor.java:
##########
@@ -261,9 +263,24 @@ protected SqlCall as(SqlNode e, String alias, String... fieldNames) {
 
   /** Returns whether a list of expressions projects all fields, in order,

Review Comment:
   According to the code description, I don't think it's enough to determine Star, so I think it's more appropriate to change the method name to 'projectAllField'.
   



-- 
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-5583] JDBC adapter does not generate 'SELECT *' when duplicate field names [calcite]

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

   Any updates on this 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


[GitHub] [calcite] sonarcloud[bot] commented on pull request #3375: [CALCITE-5583] JDBC adapter does not generate 'SELECT *' when duplicate field names

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

   Kudos, SonarCloud Quality Gate passed!&nbsp; &nbsp; [![Quality Gate passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/passed-16px.png 'Quality Gate passed')](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3375)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3375&resolved=false&types=BUG) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3375&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3375&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3375&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3375&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3375&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3375&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3375&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3375&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3375&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3375&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3375&resolved=false&types=CODE_SMELL)
   
   [![97.4%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/90-16px.png '97.4%')](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3375&metric=new_coverage&view=list) [97.4% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3375&metric=new_coverage&view=list)  
   [![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3375&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3375&metric=new_duplicated_lines_density&view=list)
   
   


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


[GitHub] [calcite] sonarcloud[bot] commented on pull request #3375: [CALCITE-5583] JDBC adapter does not generate 'SELECT *' when duplicate field names are possible

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

   Kudos, SonarCloud Quality Gate passed!&nbsp; &nbsp; [![Quality Gate passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/passed-16px.png 'Quality Gate passed')](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3375)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3375&resolved=false&types=BUG) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3375&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3375&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3375&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3375&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3375&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3375&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3375&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3375&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3375&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3375&resolved=false&types=CODE_SMELL) [3 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3375&resolved=false&types=CODE_SMELL)
   
   [![100.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/100-16px.png '100.0%')](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3375&metric=new_coverage&view=list) [100.0% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3375&metric=new_coverage&view=list)  
   [![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3375&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3375&metric=new_duplicated_lines_density&view=list)
   
   


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


[GitHub] [calcite] guestli commented on a diff in pull request #3375: [CALCITE-5583] JDBC adapter does not generate 'SELECT *' when duplicate field names are possible

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


##########
core/src/main/java/org/apache/calcite/rel/rel2sql/SqlImplementor.java:
##########
@@ -261,9 +263,24 @@ protected SqlCall as(SqlNode e, String alias, String... fieldNames) {
 
   /** Returns whether a list of expressions projects all fields, in order,
    * from the input, with the same names. */
-  public static boolean isStar(List<RexNode> exps, RelDataType inputRowType,
+  public static boolean isStar(List<RexNode> exps, RelNode input,
       RelDataType projectRowType) {
     assert exps.size() == projectRowType.getFieldCount();
+    // Duplicate column names
+    // For example, EMP JOIN DEPT, both of which have columns named DEPTNO
+    if (input instanceof Join || input instanceof MultiJoin) {

Review Comment:
   Oh, combined with the first comment, I get it, because it's a public static method, so the method name won't change. Then I'll take your advice.



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


[GitHub] [calcite] sonarcloud[bot] commented on pull request #3375: [CALCITE-5583] JDBC adapter does not generate 'SELECT *' when duplicate field names

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

   Kudos, SonarCloud Quality Gate passed!&nbsp; &nbsp; [![Quality Gate passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/passed-16px.png 'Quality Gate passed')](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3375)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3375&resolved=false&types=BUG) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3375&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3375&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3375&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3375&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3375&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3375&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3375&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3375&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3375&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3375&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3375&resolved=false&types=CODE_SMELL)
   
   [![95.8%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/90-16px.png '95.8%')](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3375&metric=new_coverage&view=list) [95.8% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3375&metric=new_coverage&view=list)  
   [![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3375&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3375&metric=new_duplicated_lines_density&view=list)
   
   


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


[GitHub] [calcite] sonarcloud[bot] commented on pull request #3375: [CALCITE-5583] JDBC adapter does not generate 'SELECT *' when duplicate field names

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

   Kudos, SonarCloud Quality Gate passed!&nbsp; &nbsp; [![Quality Gate passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/passed-16px.png 'Quality Gate passed')](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3375)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3375&resolved=false&types=BUG) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3375&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3375&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3375&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3375&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3375&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3375&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3375&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3375&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3375&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3375&resolved=false&types=CODE_SMELL) [1 Code Smell](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3375&resolved=false&types=CODE_SMELL)
   
   [![97.6%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/90-16px.png '97.6%')](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3375&metric=new_coverage&view=list) [97.6% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3375&metric=new_coverage&view=list)  
   [![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3375&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3375&metric=new_duplicated_lines_density&view=list)
   
   


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


[GitHub] [calcite] guestli commented on a diff in pull request #3375: [CALCITE-5583] JDBC adapter does not generate 'SELECT *' when duplicate field names

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


##########
core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java:
##########
@@ -7013,6 +7157,38 @@ private void checkLiteral2(String expression, String expected) {
         .withOracle(11).throws_("Lower Oracle version(<12) doesn't support offset/fetch syntax!");
   }
 
+  /** Test case for
+   * <a href="https://issues.apache.org/jira/browse/CALCITE-5583">[CALCITE-5583]
+   * Rel2Sql will get an error when select * and join is present</a>. */
+  @Test void testSelectStarWithJoin() {
+    final RelBuilder builder = relBuilder();

Review Comment:
   julianhyde, thank you for your review and pointing out these errors. I will fix them.



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


[GitHub] [calcite] sonarcloud[bot] commented on pull request #3375: [CALCITE-5583] JDBC adapter does not generate 'SELECT *' when duplicate field names

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

   Kudos, SonarCloud Quality Gate passed!&nbsp; &nbsp; [![Quality Gate passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/passed-16px.png 'Quality Gate passed')](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3375)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3375&resolved=false&types=BUG) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3375&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3375&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3375&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3375&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3375&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3375&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3375&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3375&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3375&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3375&resolved=false&types=CODE_SMELL) [1 Code Smell](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3375&resolved=false&types=CODE_SMELL)
   
   [![97.6%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/90-16px.png '97.6%')](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3375&metric=new_coverage&view=list) [97.6% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3375&metric=new_coverage&view=list)  
   [![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3375&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3375&metric=new_duplicated_lines_density&view=list)
   
   


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


[GitHub] [calcite] sonarcloud[bot] commented on pull request #3375: [CALCITE-5583] JDBC adapter does not generate 'SELECT *' when duplicate field names

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

   Kudos, SonarCloud Quality Gate passed!&nbsp; &nbsp; [![Quality Gate passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/passed-16px.png 'Quality Gate passed')](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3375)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3375&resolved=false&types=BUG) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3375&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3375&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3375&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3375&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3375&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3375&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3375&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3375&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3375&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3375&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3375&resolved=false&types=CODE_SMELL)
   
   [![97.3%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/90-16px.png '97.3%')](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3375&metric=new_coverage&view=list) [97.3% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3375&metric=new_coverage&view=list)  
   [![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3375&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3375&metric=new_duplicated_lines_density&view=list)
   
   


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


[GitHub] [calcite] sonarcloud[bot] commented on pull request #3375: [CALCITE-5583] JDBC adapter does not generate 'SELECT *' when duplicate field names are possible

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

   Kudos, SonarCloud Quality Gate passed!&nbsp; &nbsp; [![Quality Gate passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/passed-16px.png 'Quality Gate passed')](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3375)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3375&resolved=false&types=BUG) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3375&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3375&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3375&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3375&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3375&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3375&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3375&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3375&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3375&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3375&resolved=false&types=CODE_SMELL) [3 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3375&resolved=false&types=CODE_SMELL)
   
   [![95.8%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/90-16px.png '95.8%')](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3375&metric=new_coverage&view=list) [95.8% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3375&metric=new_coverage&view=list)  
   [![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3375&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3375&metric=new_duplicated_lines_density&view=list)
   
   


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


[GitHub] [calcite] julianhyde commented on a diff in pull request #3375: [CALCITE-5583] JDBC adapter does not generate 'SELECT *' when duplicate field names

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


##########
core/src/main/java/org/apache/calcite/rel/rel2sql/RelToSqlConverter.java:
##########
@@ -451,7 +453,8 @@ public Result visit(Project e) {
     }
     parseCorrelTable(e, x);
     final Builder builder = x.builder(e);
-    if (!isStar(e.getProjects(), e.getInput().getRowType(), e.getRowType())) {
+    if (isJoinDuplicateName(e.getInput()) || !isStar(e.getProjects(), e.getInput().getRowType(),

Review Comment:
   Break the line so that '||' is at start of line.



##########
core/src/main/java/org/apache/calcite/rel/rel2sql/RelToSqlConverter.java:
##########
@@ -478,6 +481,26 @@ public Result visit(Project e) {
     return builder.result();
   }
 
+  /**
+   * Whether the relNode is a join and whether its inputs have duplicate field names.
+   * For example, EMP JOIN DEPT, both of which have columns named DEPTNO
+   */
+  private boolean isJoinDuplicateName(RelNode relNode) {
+    if (relNode instanceof Join || relNode instanceof MultiJoin) {
+      List<RelNode> inputs = relNode.getInputs();
+      RelNode firstRel = inputs.get(0);
+      List<String> firstRelFiledNames = Lists.newArrayList(firstRel.getRowType().getFieldNames());
+      for (int i = 1; i < inputs.size(); i++) {
+        RelNode input = inputs.get(i);

Review Comment:
   What if the target SQL dialect is case-insensitive? For such a dialect "select t0.x, t1.X from t0, t1" would be a problem.
   
   Maybe add a `boolean caseSensitive` parameter to `Util.isDistinct`, default true, and if it's false create a set with `new TreeSet<String>(String.CASE_INSENSITIVE_ORDER)`



##########
core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java:
##########
@@ -3529,21 +3553,39 @@ private SqlDialect nonOrdinalDialect() {
         + "from \"foodmart\".\"employee\" A "
         + "join \"foodmart\".\"department\" B\n"
         + "on A.\"department_id\" = B.\"department_id\"";
-    final String expected = "SELECT *\n"
+    final String expected = "SELECT employee.employee_id, employee.full_name, employee.first_name, "
+        + "employee.last_name, employee.position_id, employee.position_title, employee.store_id, "
+        + "employee.department_id, employee.birth_date, employee.hire_date, employee.end_date, "
+        + "employee.salary, employee.supervisor_id, employee.education_level, "
+        + "employee.marital_status, employee.gender, employee.management_role, "
+        + "department.department_id AS department_id0, department.department_description\n"
         + "FROM foodmart.employee AS employee\n"
-        + "INNER JOIN foodmart.department AS department "
-        + "ON employee.department_id = department.department_id";

Review Comment:
   this diff (and a few others below) is spurious. you've just moved `ON` from one line to the next. please revert to the original formatting.



##########
core/src/main/java/org/apache/calcite/rel/rel2sql/RelToSqlConverter.java:
##########
@@ -478,6 +481,26 @@ public Result visit(Project e) {
     return builder.result();
   }
 
+  /**
+   * Whether the relNode is a join and whether its inputs have duplicate field names.
+   * For example, EMP JOIN DEPT, both of which have columns named DEPTNO
+   */
+  private boolean isJoinDuplicateName(RelNode relNode) {
+    if (relNode instanceof Join || relNode instanceof MultiJoin) {
+      List<RelNode> inputs = relNode.getInputs();
+      RelNode firstRel = inputs.get(0);
+      List<String> firstRelFiledNames = Lists.newArrayList(firstRel.getRowType().getFieldNames());
+      for (int i = 1; i < inputs.size(); i++) {
+        RelNode input = inputs.get(i);

Review Comment:
   logic doesn't seem right if there are more than two inputs, and inputs 1 and 2 (0-based) have duplicate fields.
   
   maybe write the field names into a list, and use `Util.isDistinct`



##########
core/src/main/java/org/apache/calcite/rel/rel2sql/RelToSqlConverter.java:
##########
@@ -478,6 +481,26 @@ public Result visit(Project e) {
     return builder.result();
   }
 
+  /**
+   * Whether the relNode is a join and whether its inputs have duplicate field names.
+   * For example, EMP JOIN DEPT, both of which have columns named DEPTNO
+   */
+  private boolean isJoinDuplicateName(RelNode relNode) {
+    if (relNode instanceof Join || relNode instanceof MultiJoin) {
+      List<RelNode> inputs = relNode.getInputs();
+      RelNode firstRel = inputs.get(0);
+      List<String> firstRelFiledNames = Lists.newArrayList(firstRel.getRowType().getFieldNames());
+      for (int i = 1; i < inputs.size(); i++) {
+        RelNode input = inputs.get(i);

Review Comment:
   If it is a join with `USING` or `NATURAL JOIN` then the duplicate columns will have been filtered out. It's worth adding a test for `select * from emp join dept using (deptno)` and `select * from emp natural join dept` and make sure that `deptno` only occurs once in the output.



##########
core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java:
##########
@@ -3750,17 +3792,35 @@ private SqlDialect nonOrdinalDialect() {
         + "on \"t1\".\"product_id\" = \"t3\".\"product_id\" or "
         + "(\"t1\".\"product_id\" is not null or "
         + "\"t3\".\"product_id\" is not null)";
-    String expected = "SELECT *\n"
+    String expected = "SELECT \"sales_fact_1997\".\"product_id\", \"sales_fact_1997\".\"time_id\", "
+        + "\"sales_fact_1997\".\"customer_id\", \"sales_fact_1997\".\"promotion_id\", "
+        + "\"sales_fact_1997\".\"store_id\", \"sales_fact_1997\".\"store_sales\", "
+        + "\"sales_fact_1997\".\"store_cost\", \"sales_fact_1997\".\"unit_sales\", "
+        + "\"customer\".\"customer_id\" AS \"customer_id0\", \"customer\".\"account_num\", "
+        + "\"customer\".\"lname\", \"customer\".\"fname\", \"customer\".\"mi\", "
+        + "\"customer\".\"address1\", \"customer\".\"address2\", \"customer\".\"address3\", "
+        + "\"customer\".\"address4\", \"customer\".\"city\", \"customer\".\"state_province\", "
+        + "\"customer\".\"postal_code\", \"customer\".\"country\", "
+        + "\"customer\".\"customer_region_id\", \"customer\".\"phone1\", \"customer\".\"phone2\", "
+        + "\"customer\".\"birthdate\", \"customer\".\"marital_status\", "
+        + "\"customer\".\"yearly_income\", \"customer\".\"gender\", "
+        + "\"customer\".\"total_children\", \"customer\".\"num_children_at_home\", "
+        + "\"customer\".\"education\", \"customer\".\"date_accnt_opened\", "
+        + "\"customer\".\"member_card\", \"customer\".\"occupation\", \"customer\".\"houseowner\", "
+        + "\"customer\".\"num_cars_owned\", \"customer\".\"fullname\", "
+        + "\"product\".\"product_class_id\", \"product\".\"product_id\" AS \"product_id0\", "
+        + "\"product\".\"brand_name\", \"product\".\"product_name\", \"product\".\"SKU\", "
+        + "\"product\".\"SRP\", \"product\".\"gross_weight\", \"product\".\"net_weight\", "
+        + "\"product\".\"recyclable_package\", \"product\".\"low_fat\", "
+        + "\"product\".\"units_per_case\", \"product\".\"cases_per_pallet\", "
+        + "\"product\".\"shelf_width\", \"product\".\"shelf_height\", \"product\".\"shelf_depth\"\n"
         + "FROM \"foodmart\".\"sales_fact_1997\"\n"
-        + "INNER JOIN \"foodmart\".\"customer\" "
-        + "ON \"sales_fact_1997\".\"customer_id\" = \"customer\".\"customer_id\""
-        + " OR \"sales_fact_1997\".\"customer_id\" IS NULL"
-        + " AND \"customer\".\"customer_id\" IS NULL"
-        + " OR \"customer\".\"occupation\" IS NULL\n"
-        + "INNER JOIN \"foodmart\".\"product\" "
-        + "ON \"sales_fact_1997\".\"product_id\" = \"product\".\"product_id\""
-        + " OR \"sales_fact_1997\".\"product_id\" IS NOT NULL"
-        + " OR \"product\".\"product_id\" IS NOT NULL";
+        + "INNER JOIN \"foodmart\".\"customer\" ON \"sales_fact_1997\".\"customer_id\" = "

Review Comment:
   this diff is spurious. and the formatting is worse than before.



##########
core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java:
##########
@@ -7013,6 +7157,38 @@ private void checkLiteral2(String expression, String expected) {
         .withOracle(11).throws_("Lower Oracle version(<12) doesn't support offset/fetch syntax!");
   }
 
+  /** Test case for
+   * <a href="https://issues.apache.org/jira/browse/CALCITE-5583">[CALCITE-5583]
+   * Rel2Sql will get an error when select * and join is present</a>. */
+  @Test void testSelectStarWithJoin() {
+    final RelBuilder builder = relBuilder();

Review Comment:
   modern tests use the `relFn()` method rather than `toSql(RelNode)`; don't call `relBuilder()` explicitly



-- 
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-5583] JDBC adapter does not generate 'SELECT *' when duplicate field names [calcite]

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

   This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 90 days if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@calcite.apache.org list. Thank you for your contributions.


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


[GitHub] [calcite] JiajunBernoulli commented on a diff in pull request #3375: [CALCITE-5583] JDBC adapter does not generate 'SELECT *' when duplicate field names are possible

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


##########
core/src/main/java/org/apache/calcite/rel/rel2sql/SqlImplementor.java:
##########
@@ -261,9 +263,24 @@ protected SqlCall as(SqlNode e, String alias, String... fieldNames) {
 
   /** Returns whether a list of expressions projects all fields, in order,
    * from the input, with the same names. */
-  public static boolean isStar(List<RexNode> exps, RelDataType inputRowType,
+  public static boolean isStar(List<RexNode> exps, RelNode input,

Review Comment:
   This is a public static method, it is best not to modify its parameters.



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


[GitHub] [calcite] guestli commented on a diff in pull request #3375: [CALCITE-5583] JDBC adapter does not generate 'SELECT *' when duplicate field names are possible

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


##########
core/src/main/java/org/apache/calcite/rel/rel2sql/SqlImplementor.java:
##########
@@ -261,9 +263,24 @@ protected SqlCall as(SqlNode e, String alias, String... fieldNames) {
 
   /** Returns whether a list of expressions projects all fields, in order,
    * from the input, with the same names. */
-  public static boolean isStar(List<RexNode> exps, RelDataType inputRowType,
+  public static boolean isStar(List<RexNode> exps, RelNode input,
       RelDataType projectRowType) {
     assert exps.size() == projectRowType.getFieldCount();
+    // Duplicate column names
+    // For example, EMP JOIN DEPT, both of which have columns named DEPTNO
+    if (input instanceof Join || input instanceof MultiJoin) {

Review Comment:
   @JiajunBernoulli Thank you for your advice. I've considered adding a method before, but it might still cause an error when the isStar method is called from somewhere else. If the method name of 'isStar' is changed to something like 'projectAllField', `!projectAllField() && ! isJoinDuplicateName()` would that be better?
   If you insist that the method name not be changed, I will also take your advice.



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


[GitHub] [calcite] julianhyde commented on a diff in pull request #3375: [CALCITE-5583] JDBC adapter does not generate 'SELECT *' when duplicate field names

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


##########
core/src/main/java/org/apache/calcite/util/Util.java:
##########
@@ -2166,6 +2167,24 @@ public static <E> boolean isDistinct(List<E> list) {
     return firstDuplicate(list) < 0;
   }
 
+
+  /**
+   * Returns whether the elements of {@code list} are distinct according to a given
+   * case-sensitivity policy.
+   */
+  public static boolean isDistinct(List<String> list, boolean caseSensitive) {
+    if (caseSensitive) {
+      return isDistinct(list);
+    }
+    final Set<String> set = new TreeSet<>(String.CASE_INSENSITIVE_ORDER);

Review Comment:
   if list has 0 or 1 elements you can immediately return true



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


[GitHub] [calcite] guestli commented on a diff in pull request #3375: [CALCITE-5583] JDBC adapter does not generate 'SELECT *' when duplicate field names

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


##########
core/src/main/java/org/apache/calcite/util/Util.java:
##########
@@ -2166,6 +2167,24 @@ public static <E> boolean isDistinct(List<E> list) {
     return firstDuplicate(list) < 0;
   }
 
+
+  /**
+   * Returns whether the elements of {@code list} are distinct according to a given
+   * case-sensitivity policy.
+   */
+  public static boolean isDistinct(List<String> list, boolean caseSensitive) {
+    if (caseSensitive) {
+      return isDistinct(list);
+    }
+    final Set<String> set = new TreeSet<>(String.CASE_INSENSITIVE_ORDER);

Review Comment:
   @julianhyde Thank you for taking the time to review, and please review it again.



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


[GitHub] [calcite] sonarcloud[bot] commented on pull request #3375: [CALCITE-5583] JDBC adapter does not generate 'SELECT *' when duplicate field names are possible

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

   Kudos, SonarCloud Quality Gate passed!&nbsp; &nbsp; [![Quality Gate passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/passed-16px.png 'Quality Gate passed')](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3375)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3375&resolved=false&types=BUG) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3375&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3375&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3375&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3375&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3375&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3375&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3375&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3375&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3375&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3375&resolved=false&types=CODE_SMELL) [3 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3375&resolved=false&types=CODE_SMELL)
   
   [![95.8%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/90-16px.png '95.8%')](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3375&metric=new_coverage&view=list) [95.8% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3375&metric=new_coverage&view=list)  
   [![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3375&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3375&metric=new_duplicated_lines_density&view=list)
   
   


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


[GitHub] [calcite] guestli commented on a diff in pull request #3375: [CALCITE-5583] JDBC adapter does not generate 'SELECT *' when duplicate field names are possible

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


##########
core/src/main/java/org/apache/calcite/rel/rel2sql/SqlImplementor.java:
##########
@@ -261,9 +263,24 @@ protected SqlCall as(SqlNode e, String alias, String... fieldNames) {
 
   /** Returns whether a list of expressions projects all fields, in order,

Review Comment:
   Thanks for correction.



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


[GitHub] [calcite] JiajunBernoulli commented on a diff in pull request #3375: [CALCITE-5583] JDBC adapter does not generate 'SELECT *' when duplicate field names are possible

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


##########
core/src/main/java/org/apache/calcite/rel/rel2sql/SqlImplementor.java:
##########
@@ -261,9 +263,24 @@ protected SqlCall as(SqlNode e, String alias, String... fieldNames) {
 
   /** Returns whether a list of expressions projects all fields, in order,
    * from the input, with the same names. */
-  public static boolean isStar(List<RexNode> exps, RelDataType inputRowType,
+  public static boolean isStar(List<RexNode> exps, RelNode input,
       RelDataType projectRowType) {
     assert exps.size() == projectRowType.getFieldCount();
+    // Duplicate column names
+    // For example, EMP JOIN DEPT, both of which have columns named DEPTNO
+    if (input instanceof Join || input instanceof MultiJoin) {

Review Comment:
   Encapsulate this part of the code into an independent function, maybe it named `isJoinDuplicateName`.
   
   Call function by `!isStar() && !isJoinDuplicateName()`.



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


[GitHub] [calcite] JiajunBernoulli commented on a diff in pull request #3375: [CALCITE-5583] JDBC adapter does not generate 'SELECT *' when duplicate field names are possible

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


##########
core/src/main/java/org/apache/calcite/rel/rel2sql/SqlImplementor.java:
##########
@@ -261,9 +263,24 @@ protected SqlCall as(SqlNode e, String alias, String... fieldNames) {
 
   /** Returns whether a list of expressions projects all fields, in order,

Review Comment:
   According to the code description, users may use it to determine Star.



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


[GitHub] [calcite] JiajunBernoulli commented on a diff in pull request #3375: [CALCITE-5583] JDBC adapter does not generate 'SELECT *' when duplicate field names are possible

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


##########
core/src/main/java/org/apache/calcite/rel/rel2sql/SqlImplementor.java:
##########
@@ -261,9 +263,24 @@ protected SqlCall as(SqlNode e, String alias, String... fieldNames) {
 
   /** Returns whether a list of expressions projects all fields, in order,

Review Comment:
   Maybe, but it's not necessary(The public static method needs to be stable).
   
   Please remove 264-265 line, because it is same as 262-263 line.



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


[GitHub] [calcite] sonarcloud[bot] commented on pull request #3375: [CALCITE-5583] JDBC adapter does not generate 'SELECT *' when duplicate field names

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

   Kudos, SonarCloud Quality Gate passed!&nbsp; &nbsp; [![Quality Gate passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/passed-16px.png 'Quality Gate passed')](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3375)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3375&resolved=false&types=BUG) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3375&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3375&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3375&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3375&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3375&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3375&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3375&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3375&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3375&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3375&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3375&resolved=false&types=CODE_SMELL)
   
   [![97.3%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/90-16px.png '97.3%')](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3375&metric=new_coverage&view=list) [97.3% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3375&metric=new_coverage&view=list)  
   [![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3375&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3375&metric=new_duplicated_lines_density&view=list)
   
   


-- 
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-5583] JDBC adapter does not generate 'SELECT *' when duplicate field names [calcite]

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

   This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 90 days if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@calcite.apache.org list. Thank you for your contributions.


-- 
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-5583] JDBC adapter does not generate 'SELECT *' when duplicate field names [calcite]

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

   This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 90 days if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@calcite.apache.org list. Thank you for your contributions.


-- 
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-5583] JDBC adapter does not generate 'SELECT *' when duplicate field names [calcite]

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

   This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 90 days if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@calcite.apache.org list. Thank you for your contributions.


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