[GitHub] [calcite] danny0405 commented on a change in pull request #1859: [CALCITE-3863] Make Truncate/Round return type inference overridable through rel data …

2020-03-24 Thread GitBox
danny0405 commented on a change in pull request #1859: [CALCITE-3863] Make 
Truncate/Round return type inference overridable through rel data …
URL: https://github.com/apache/calcite/pull/1859#discussion_r396948372
 
 

 ##
 File path: 
core/src/main/java/org/apache/calcite/rel/type/RelDataTypeSystem.java
 ##
 @@ -405,4 +405,103 @@ default RelDataType 
deriveDecimalModType(RelDataTypeFactory typeFactory,
 return null;
   }
 
+  /**
+   * Infers the return type of a decimal truncate operation. Decimal truncate
+   * involves at least one decimal operand.
+   *
+   * The default implementation is SQL:2003 compliant: the declared type of
+   * the result is the declared type of the first operand (decimal to be 
truncated).
+   *
+   * @see Glossary#SQL2003 SQL:2003 Part 2 Section 6.27
+   *
+   * Rules:
+   *
+   * 
+   * Let p1, s1 be the precision and scale of the first operand
+   * Let s2 be the scale value to truncate
+   * Let p, s be the precision and scale of the result
+   * Let d be the number of whole digits in the result
+   * Then the result type is a decimal with:
+   *   
+   *   s = s2
+   *   p = p1
+   *   
+   * 
+   * p and s are capped at their maximum values
+   * 
+   *
+   * @param typeFactory TypeFactory used to create output type
+   * @param type1   Type of the first operand
+   * @param scale2  Scale value to truncate to
+   * @return Result type for a decimal truncate
+   */
+  default RelDataType deriveDecimalTruncateType(RelDataTypeFactory typeFactory,
+  RelDataType type1, Integer scale2) {
+if (SqlTypeUtil.isExactNumeric(type1)) {
+  if (SqlTypeUtil.isDecimal(type1)) {
+// Java numeric will always have invalid precision/scale,
 
 Review comment:
   you just return a decimal with same precision and scale of `type1`, so why 
we need these methods in `RelDataTypeSystem`, just return the `type1` seems 
okey ?


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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [calcite] danny0405 commented on a change in pull request #1859: [CALCITE-3863] Make Truncate/Round return type inference overridable through rel data …

2020-03-24 Thread GitBox
danny0405 commented on a change in pull request #1859: [CALCITE-3863] Make 
Truncate/Round return type inference overridable through rel data …
URL: https://github.com/apache/calcite/pull/1859#discussion_r396948646
 
 

 ##
 File path: core/src/main/java/org/apache/calcite/sql/type/ReturnTypes.java
 ##
 @@ -557,6 +557,55 @@ public int size() {
   public static final SqlReturnTypeInference NULLABLE_MOD =
   chain(DECIMAL_MOD_NULLABLE, ARG1_NULLABLE);
 
+  public static final SqlReturnTypeInference DECIMAL_TRUNCATE = opBinding -> {
+RelDataTypeFactory typeFactory = opBinding.getTypeFactory();
+RelDataType type1 = opBinding.getOperandType(0);
+// passing random value for scale2
+// since the default behaviour is to have the return type of result as the 
first operand
+return typeFactory.getTypeSystem().deriveDecimalTruncateType(typeFactory, 
type1, 0);
+  };
 
 Review comment:
   just return `type1` ?


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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [calcite] danny0405 commented on a change in pull request #1859: [CALCITE-3863] Make Truncate/Round return type inference overridable through rel data …

2020-03-24 Thread GitBox
danny0405 commented on a change in pull request #1859: [CALCITE-3863] Make 
Truncate/Round return type inference overridable through rel data …
URL: https://github.com/apache/calcite/pull/1859#discussion_r396947719
 
 

 ##
 File path: 
core/src/main/java/org/apache/calcite/rel/type/RelDataTypeSystem.java
 ##
 @@ -405,4 +405,103 @@ default RelDataType 
deriveDecimalModType(RelDataTypeFactory typeFactory,
 return null;
   }
 
+  /**
+   * Infers the return type of a decimal truncate operation. Decimal truncate
+   * involves at least one decimal operand.
+   *
+   * The default implementation is SQL:2003 compliant: the declared type of
+   * the result is the declared type of the first operand (decimal to be 
truncated).
+   *
+   * @see Glossary#SQL2003 SQL:2003 Part 2 Section 6.27
+   *
+   * Rules:
+   *
+   * 
+   * Let p1, s1 be the precision and scale of the first operand
+   * Let s2 be the scale value to truncate
+   * Let p, s be the precision and scale of the result
+   * Let d be the number of whole digits in the result
+   * Then the result type is a decimal with:
+   *   
+   *   s = s2
+   *   p = p1
+   *   
+   * 
+   * p and s are capped at their maximum values
+   * 
+   *
+   * @param typeFactory TypeFactory used to create output type
+   * @param type1   Type of the first operand
+   * @param scale2  Scale value to truncate to
+   * @return Result type for a decimal truncate
+   */
+  default RelDataType deriveDecimalTruncateType(RelDataTypeFactory typeFactory,
+  RelDataType type1, Integer scale2) {
+if (SqlTypeUtil.isExactNumeric(type1)) {
+  if (SqlTypeUtil.isDecimal(type1)) {
+// Java numeric will always have invalid precision/scale,
 
 Review comment:
   The `scale2` is never used.


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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [calcite] danny0405 commented on a change in pull request #1859: [CALCITE-3863] Make Truncate/Round return type inference overridable through rel data …

2020-03-24 Thread GitBox
danny0405 commented on a change in pull request #1859: [CALCITE-3863] Make 
Truncate/Round return type inference overridable through rel data …
URL: https://github.com/apache/calcite/pull/1859#discussion_r396946347
 
 

 ##
 File path: 
core/src/main/java/org/apache/calcite/rel/type/RelDataTypeSystem.java
 ##
 @@ -405,4 +405,103 @@ default RelDataType 
deriveDecimalModType(RelDataTypeFactory typeFactory,
 return null;
   }
 
+  /**
+   * Infers the return type of a decimal truncate operation. Decimal truncate
+   * involves at least one decimal operand.
+   *
+   * The default implementation is SQL:2003 compliant: the declared type of
+   * the result is the declared type of the first operand (decimal to be 
truncated).
+   *
+   * @see Glossary#SQL2003 SQL:2003 Part 2 Section 6.27
+   *
+   * Rules:
+   *
+   * 
+   * Let p1, s1 be the precision and scale of the first operand
+   * Let s2 be the scale value to truncate
+   * Let p, s be the precision and scale of the result
+   * Let d be the number of whole digits in the result
+   * Then the result type is a decimal with:
 
 Review comment:
   Only `p` `s` `p1` `s2`  are used, remove the useless comments.


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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services