You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by "tanclary (via GitHub)" <gi...@apache.org> on 2024/03/04 18:02:23 UTC

[PR] [CALCITE-6290] Incorrect return type for BigQuery TRUNC [calcite]

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

   (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-6290] Incorrect return type for BigQuery TRUNC [calcite]

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

   ## [![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=3714) **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=3714&resolved=false&inNewCodePeriod=true)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/accepted-16px.png '') [0 Accepted issues](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3714&metric=new_accepted_issues&view=list)
   
   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=3714&resolved=false&inNewCodePeriod=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=3714&metric=new_coverage&view=list)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [12.5% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3714&metric=new_duplicated_lines_density&view=list)  
     
   [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3714)
   
   


-- 
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-6290] Incorrect return type for BigQuery TRUNC [calcite]

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


##########
core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java:
##########
@@ -2161,9 +2161,12 @@ private static RelDataType deriveTypeMapFromEntries(SqlOperatorBinding opBinding
   public static final SqlFunction POW =
       SqlStdOperatorTable.POWER.withName("POW");
 
+  /** The "TRUNC(numeric1 [, integer2])" function. Identical to the standard <code>TRUNCATE</code>
+  * function except the return type should be a double if numeric1 is an integer. */
   @LibraryOperator(libraries = {BIG_QUERY})
-  public static final SqlFunction TRUNC =
-      SqlStdOperatorTable.TRUNCATE.withName("TRUNC");
+  public static final SqlFunction TRUNC = SqlStdOperatorTable.TRUNCATE

Review Comment:
   maintainers will do the wrong thing if you don't make it very obvious. Find a way to make it obvious.



-- 
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-6290] Incorrect return type for BigQuery TRUNC [calcite]

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

   ## [![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=3714) **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=3714&resolved=false&inNewCodePeriod=true)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/accepted-16px.png '') [0 Accepted issues](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3714&metric=new_accepted_issues&view=list)
   
   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=3714&resolved=false&inNewCodePeriod=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=3714&metric=new_coverage&view=list)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [14.3% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3714&metric=new_duplicated_lines_density&view=list)  
     
   [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3714)
   
   


-- 
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-6290] Incorrect return type for BigQuery TRUNC [calcite]

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


##########
core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java:
##########
@@ -2161,9 +2161,12 @@ private static RelDataType deriveTypeMapFromEntries(SqlOperatorBinding opBinding
   public static final SqlFunction POW =
       SqlStdOperatorTable.POWER.withName("POW");
 
+  /** The "TRUNC(numeric1 [, integer2])" function. Identical to the standard <code>TRUNCATE</code>
+  * function except the return type should be a double if numeric1 is an integer. */
   @LibraryOperator(libraries = {BIG_QUERY})
-  public static final SqlFunction TRUNC =
-      SqlStdOperatorTable.TRUNCATE.withName("TRUNC");
+  public static final SqlFunction TRUNC = SqlStdOperatorTable.TRUNCATE

Review Comment:
   I think renaming it might be a bit redundant, I only renamed FLOOR and CEIL because they have standard counterparts with the same name, TRUNC however is unique from the standard TRUNCATE. What do you think? I don't feel very strongly either way. @julianhyde 



-- 
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-6290] Incorrect return type for BigQuery TRUNC [calcite]

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


##########
core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java:
##########
@@ -2161,9 +2161,12 @@ private static RelDataType deriveTypeMapFromEntries(SqlOperatorBinding opBinding
   public static final SqlFunction POW =
       SqlStdOperatorTable.POWER.withName("POW");
 
+  /** The "TRUNC(numeric1 [, integer2])" function. Identical to the standard <code>TRUNCATE</code>
+  * function except the return type should be a double if numeric1 is an integer. */
   @LibraryOperator(libraries = {BIG_QUERY})
-  public static final SqlFunction TRUNC =
-      SqlStdOperatorTable.TRUNCATE.withName("TRUNC");
+  public static final SqlFunction TRUNC = SqlStdOperatorTable.TRUNCATE

Review Comment:
   And I think the JIRA should already have a link to 5747 let me know if it looks wrong



-- 
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-6290] Incorrect return type for BigQuery TRUNC [calcite]

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


##########
core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java:
##########
@@ -2161,9 +2161,12 @@ private static RelDataType deriveTypeMapFromEntries(SqlOperatorBinding opBinding
   public static final SqlFunction POW =
       SqlStdOperatorTable.POWER.withName("POW");
 
+  /** The "TRUNC(numeric1 [, integer2])" function. Identical to the standard <code>TRUNCATE</code>
+  * function except the return type should be a double if numeric1 is an integer. */
   @LibraryOperator(libraries = {BIG_QUERY})
-  public static final SqlFunction TRUNC =
-      SqlStdOperatorTable.TRUNCATE.withName("TRUNC");
+  public static final SqlFunction TRUNC = SqlStdOperatorTable.TRUNCATE

Review Comment:
   Consider renaming the function to TRUNC_BIG_QUERY. The BigQuery FLOOR and TRUNC functions are now in sync, but need to remain so, and linking their names will make that more likely. 
   
   In Jira please link to CALCITE-5747. That's where these came adrift in the first place.



##########
core/src/main/java/org/apache/calcite/sql/type/ReturnTypes.java:
##########
@@ -550,12 +550,12 @@ public static SqlCall stripSeparator(SqlCall call) {
    */
   public static final SqlReturnTypeInference ARG0_EXCEPT_INTEGER = opBinding ->  {
     RelDataTypeFactory typeFactory = opBinding.getTypeFactory();
-    SqlTypeName op = opBinding.getOperandType(0).getSqlTypeName();
-    if (SqlTypeName.INT_TYPES.contains(op)) {
+    RelDataType op = opBinding.getOperandType(0);

Review Comment:
   `op` is no longer a good name for this variable



##########
core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java:
##########
@@ -592,6 +592,20 @@ private static String toSql(RelNode root, SqlDialect dialect,
         + "created_thing\nFROM foodmart.product");
   }
 
+    /** Test case for
+   * <a href="https://issues.apache.org/jira/browse/CALCITE-6290">[CALCITE-6290]
+   * Incorrect return type for BigQuery TRUNC</a>. */
+  @Test void testBigQueryTruncPreservesCast() {
+    final String query = "SELECT CAST(TRUNC(3) AS BIGINT) "
+        + "as created_thing\n FROM `foodmart`.`product`";

Review Comment:
   add line break after `\n`



-- 
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-6290] Incorrect return type for BigQuery TRUNC [calcite]

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


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