[GitHub] vlsi commented on a change in pull request #1080: [CALCITE-2887] Improve performance of RelLiteral#toJavaString

[GitBox <gi...@apache.org>]

vlsi commented on a change in pull request #1080: [CALCITE-2887] Improve performance of RelLiteral#toJavaString
URL: https://github.com/apache/calcite/pull/1080#discussion_r261695688
 
 

 ##########
 File path: core/src/main/java/org/apache/calcite/rex/RexLiteral.java
 ##########
 @@ -568,83 +573,84 @@ public void printAsJava(PrintWriter pw) {
    * <li>1234ABCD</li>
    * </ul>
    *  @param value    Value
-   * @param pw       Writer to write to
+   * @param dest the destination where to append the value.
    * @param typeName Type family
    * @param includeType if representation should include data type
    */
-  private static void printAsJava(
+  private static void appendAsJava(
       Comparable value,
-      PrintWriter pw,
+      Appendable dest,
       SqlTypeName typeName,
-      boolean java, RexDigestIncludeType includeType) {
+      boolean java, RexDigestIncludeType includeType) throws IOException {
     switch (typeName) {
     case CHAR:
       NlsString nlsString = (NlsString) value;
       if (java) {
         Util.printJavaString(
-            pw,
+            dest,
             nlsString.getValue(),
             true);
       } else {
         boolean includeCharset =
             (nlsString.getCharsetName() != null)
                 && !nlsString.getCharsetName().equals(
                 CalciteSystemProperty.DEFAULT_CHARSET.value());
-        pw.print(nlsString.asSql(includeCharset, false));
+        dest.append(nlsString.asSql(includeCharset, false));
       }
       break;
     case BOOLEAN:
       assert value instanceof Boolean;
-      pw.print(((Boolean) value).booleanValue());
+      dest.append(value.toString());
       break;
     case DECIMAL:
       assert value instanceof BigDecimal;
-      pw.print(value.toString());
+      dest.append(value.toString());
       break;
     case DOUBLE:
       assert value instanceof BigDecimal;
-      pw.print(Util.toScientificNotation((BigDecimal) value));
+      dest.append(Util.toScientificNotation((BigDecimal) value));
       break;
     case BIGINT:
       assert value instanceof BigDecimal;
-      pw.print(((BigDecimal) value).longValue());
-      pw.print('L');
+      long narrowLong = ((BigDecimal) value).longValue();
+      dest.append(String.valueOf(narrowLong));
+      dest.append('L');
       break;
     case BINARY:
       assert value instanceof ByteString;
-      pw.print("X'");
-      pw.print(((ByteString) value).toString(16));
-      pw.print("'");
+      dest.append("X'");
+      dest.append(((ByteString) value).toString(16));
+      dest.append("'");
       break;
     case NULL:
       assert value == null;
-      pw.print("null");
+      dest.append("null");
       break;
     case SYMBOL:
       assert value instanceof Enum;
-      pw.print("FLAG(");
-      pw.print(value);
-      pw.print(")");
+      dest.append("FLAG(");
+      dest.append(value.toString());
+      dest.append(")");
       break;
     case DATE:
       assert value instanceof DateString;
-      pw.print(value);
+      dest.append(value.toString());
       break;
     case TIME:
       assert value instanceof TimeString;
-      pw.print(value);
+      dest.append(value.toString());
       break;
     case TIME_WITH_LOCAL_TIME_ZONE:
       assert value instanceof TimeString;
-      pw.print(value);
+      dest.append(value.toString());
       break;
     case TIMESTAMP:
       assert value instanceof TimestampString;
-      pw.print(value);
+      dest.append(value.toString());
 
 Review comment:
   is toString required here? Is it different from `dest.append(value)`?

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services