julianhyde commented on code in PR #3663:
URL: https://github.com/apache/calcite/pull/3663#discussion_r1473635196


##########
core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java:
##########
@@ -3027,6 +3027,31 @@ private void 
checkPushJoinThroughUnionOnRightDoesNotMatchSemiOrAntiJoin(JoinRelT
         .check();
   }
 
+  /** Test case for <a 
href="https://issues.apache.org/jira/browse/CALCITE-2067";>

Review Comment:
   Conventional format includes `[`.



##########
core/src/main/java/org/apache/calcite/util/Util.java:
##########
@@ -544,6 +544,21 @@ public static void println(
     pw.println();
   }
 
+  /**
+   * Formats a double value to a String ensuring that the output
+   * is in scientific notation if the value is not "special".
+   * (Special values include infinities and NaN.)
+   */
+  public static String toScientificNotation(Double d) {

Review Comment:
   Need unit tests for this method.



##########
core/src/main/java/org/apache/calcite/rex/RexLiteral.java:
##########
@@ -325,19 +324,18 @@ public static boolean valueMatchesType(
       }
       // fall through
     case DECIMAL:
+    case BIGINT:
+      return value instanceof BigDecimal;
     case DOUBLE:
     case FLOAT:
     case REAL:
-    case BIGINT:
-      return value instanceof BigDecimal;
+      return value instanceof BigDecimal || value instanceof Double;

Review Comment:
   Is there anything to be gained in giving people a choice? 



##########
core/src/main/java/org/apache/calcite/rex/RexBuilder.java:
##########
@@ -1110,6 +1110,14 @@ public RexLiteral makeApproxLiteral(BigDecimal bd) {
     return makeApproxLiteral(bd, 
typeFactory.createSqlType(SqlTypeName.DOUBLE));
   }
 
+  /**

Review Comment:
   I think this method is too easy to invoke by accident.



##########
core/src/main/java/org/apache/calcite/util/Util.java:
##########
@@ -544,6 +544,21 @@ public static void println(
     pw.println();
   }
 
+  /**
+   * Formats a double value to a String ensuring that the output
+   * is in scientific notation if the value is not "special".
+   * (Special values include infinities and NaN.)
+   */
+  public static String toScientificNotation(Double d) {

Review Comment:
   Should this method guarantee that there is a unique string for every 
possible `double` value? (Except for the many representations of `NaN` etc.) 
And that it has an inverse method that will convert the string to the same 
`double` value.
   
   I believe that we are relying on this bijection, because there is no direct 
way to specify a binary floating point value in 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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to