amaliujia commented on a change in pull request #1834: [CALCITE-3823] Do not 
use String.replaceAll
URL: https://github.com/apache/calcite/pull/1834#discussion_r384795872
 
 

 ##########
 File path: core/src/main/java/org/apache/calcite/sql/SqlDialect.java
 ##########
 @@ -346,14 +346,7 @@ public RelDataTypeSystem getTypeSystem() {
    * @return Quoted identifier
    */
   public String quoteIdentifier(String val) {
-    if (identifierQuoteString == null) {
-      return val; // quoting is not supported
-    }
-    String val2 =
-        val.replaceAll(
-            identifierEndQuoteString,
-            identifierEscapedQuote);
-    return identifierQuoteString + val2 + identifierEndQuoteString;
+    return quoteIdentifier(new StringBuilder(), val).toString();
 
 Review comment:
   just a side note: Java has some sorts of string concatenation optimizations 
after JDK9. In the future after JDK usage is upgrade, there might be less need 
to do such rewrite.
   
   see: http://openjdk.java.net/jeps/280
   
   
   But encapsulating logic into functions is definitely good.

----------------------------------------------------------------
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:
[email protected]


With regards,
Apache Git Services

Reply via email to