You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by jh...@apache.org on 2017/10/02 21:00:08 UTC

[08/15] calcite git commit: [CALCITE-1988] Various code quality issues

[CALCITE-1988] Various code quality issues

* Fix error that SqlNode.clone does not call super.clone; deprecate
  it, and add SqlNode.clone(SqlNode)
* Fix error that NlsString.clone does not call super.clone
* Make AggregateNode.AccumulatorList static, because non-static inner
  classes must not implement Serializable
* Change XmlOutput.content to not use LineNumberReader.readLine, which
  is susceptible to a DoS attack


Project: http://git-wip-us.apache.org/repos/asf/calcite/repo
Commit: http://git-wip-us.apache.org/repos/asf/calcite/commit/8441e79c
Tree: http://git-wip-us.apache.org/repos/asf/calcite/tree/8441e79c
Diff: http://git-wip-us.apache.org/repos/asf/calcite/diff/8441e79c

Branch: refs/heads/master
Commit: 8441e79cdde01a901de894d44b597cf582a259d1
Parents: d173640
Author: Julian Hyde <jh...@apache.org>
Authored: Mon Sep 18 11:56:35 2017 -0700
Committer: Julian Hyde <jh...@apache.org>
Committed: Mon Oct 2 11:13:43 2017 -0700

----------------------------------------------------------------------
 .../calcite/interpreter/AggregateNode.java      |  2 +-
 .../calcite/sql/SqlBinaryStringLiteral.java     |  2 +-
 .../calcite/sql/SqlCharStringLiteral.java       |  2 +-
 .../org/apache/calcite/sql/SqlDateLiteral.java  |  2 +-
 .../apache/calcite/sql/SqlIntervalLiteral.java  |  7 ++--
 .../java/org/apache/calcite/sql/SqlLiteral.java |  2 +-
 .../java/org/apache/calcite/sql/SqlNode.java    | 12 ++++++-
 .../apache/calcite/sql/SqlNumericLiteral.java   | 10 ++----
 .../org/apache/calcite/sql/SqlTimeLiteral.java  |  2 +-
 .../apache/calcite/sql/SqlTimestampLiteral.java |  2 +-
 .../calcite/sql/fun/SqlCoalesceFunction.java    | 11 +++---
 .../calcite/sql/fun/SqlNullifFunction.java      |  5 ++-
 .../calcite/sql/validate/SqlValidatorImpl.java  | 11 +++---
 .../calcite/sql/validate/SqlValidatorUtil.java  |  8 ++---
 .../java/org/apache/calcite/util/NlsString.java | 10 ++++--
 .../java/org/apache/calcite/util/XmlOutput.java | 28 +++++++++------
 .../java/org/apache/calcite/util/UtilTest.java  | 37 ++++++++++++++++++++
 17 files changed, 98 insertions(+), 55 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/calcite/blob/8441e79c/core/src/main/java/org/apache/calcite/interpreter/AggregateNode.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/calcite/interpreter/AggregateNode.java b/core/src/main/java/org/apache/calcite/interpreter/AggregateNode.java
index b25a268..47b933b 100644
--- a/core/src/main/java/org/apache/calcite/interpreter/AggregateNode.java
+++ b/core/src/main/java/org/apache/calcite/interpreter/AggregateNode.java
@@ -356,7 +356,7 @@ public class AggregateNode extends AbstractSingleNode<Aggregate> {
   /**
    * A list of accumulators used during grouping.
    */
-  private class AccumulatorList extends ArrayList<Accumulator> {
+  private static class AccumulatorList extends ArrayList<Accumulator> {
     public void send(Row row) {
       for (Accumulator a : this) {
         a.send(row);

http://git-wip-us.apache.org/repos/asf/calcite/blob/8441e79c/core/src/main/java/org/apache/calcite/sql/SqlBinaryStringLiteral.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/calcite/sql/SqlBinaryStringLiteral.java b/core/src/main/java/org/apache/calcite/sql/SqlBinaryStringLiteral.java
index 01ad109..71380e1 100644
--- a/core/src/main/java/org/apache/calcite/sql/SqlBinaryStringLiteral.java
+++ b/core/src/main/java/org/apache/calcite/sql/SqlBinaryStringLiteral.java
@@ -56,7 +56,7 @@ public class SqlBinaryStringLiteral extends SqlAbstractStringLiteral {
     return (BitString) value;
   }
 
-  public SqlNode clone(SqlParserPos pos) {
+  @Override public SqlBinaryStringLiteral clone(SqlParserPos pos) {
     return new SqlBinaryStringLiteral((BitString) value, pos);
   }
 

http://git-wip-us.apache.org/repos/asf/calcite/blob/8441e79c/core/src/main/java/org/apache/calcite/sql/SqlCharStringLiteral.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/calcite/sql/SqlCharStringLiteral.java b/core/src/main/java/org/apache/calcite/sql/SqlCharStringLiteral.java
index 9f15a22..b74176c 100644
--- a/core/src/main/java/org/apache/calcite/sql/SqlCharStringLiteral.java
+++ b/core/src/main/java/org/apache/calcite/sql/SqlCharStringLiteral.java
@@ -63,7 +63,7 @@ public class SqlCharStringLiteral extends SqlAbstractStringLiteral {
     return getNlsString().getCollation();
   }
 
-  public SqlNode clone(SqlParserPos pos) {
+  @Override public SqlCharStringLiteral clone(SqlParserPos pos) {
     return new SqlCharStringLiteral((NlsString) value, pos);
   }
 

http://git-wip-us.apache.org/repos/asf/calcite/blob/8441e79c/core/src/main/java/org/apache/calcite/sql/SqlDateLiteral.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/calcite/sql/SqlDateLiteral.java b/core/src/main/java/org/apache/calcite/sql/SqlDateLiteral.java
index 1a757f6..27cc8b4 100644
--- a/core/src/main/java/org/apache/calcite/sql/SqlDateLiteral.java
+++ b/core/src/main/java/org/apache/calcite/sql/SqlDateLiteral.java
@@ -42,7 +42,7 @@ public class SqlDateLiteral extends SqlAbstractDateTimeLiteral {
     return (DateString) value;
   }
 
-  public SqlNode clone(SqlParserPos pos) {
+  @Override public SqlDateLiteral clone(SqlParserPos pos) {
     return new SqlDateLiteral((DateString) value, pos);
   }
 

http://git-wip-us.apache.org/repos/asf/calcite/blob/8441e79c/core/src/main/java/org/apache/calcite/sql/SqlIntervalLiteral.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/calcite/sql/SqlIntervalLiteral.java b/core/src/main/java/org/apache/calcite/sql/SqlIntervalLiteral.java
index bd2969c..246e673 100644
--- a/core/src/main/java/org/apache/calcite/sql/SqlIntervalLiteral.java
+++ b/core/src/main/java/org/apache/calcite/sql/SqlIntervalLiteral.java
@@ -64,11 +64,8 @@ public class SqlIntervalLiteral extends SqlLiteral {
 
   //~ Methods ----------------------------------------------------------------
 
-  public SqlNode clone(SqlParserPos pos) {
-    return new SqlIntervalLiteral(
-        (IntervalValue) value,
-        getTypeName(),
-        pos);
+  @Override public SqlIntervalLiteral clone(SqlParserPos pos) {
+    return new SqlIntervalLiteral((IntervalValue) value, getTypeName(), pos);
   }
 
   public void unparse(

http://git-wip-us.apache.org/repos/asf/calcite/blob/8441e79c/core/src/main/java/org/apache/calcite/sql/SqlLiteral.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/calcite/sql/SqlLiteral.java b/core/src/main/java/org/apache/calcite/sql/SqlLiteral.java
index 4fcafb9..4cf88d7 100644
--- a/core/src/main/java/org/apache/calcite/sql/SqlLiteral.java
+++ b/core/src/main/java/org/apache/calcite/sql/SqlLiteral.java
@@ -234,7 +234,7 @@ public class SqlLiteral extends SqlNode {
     }
   }
 
-  public SqlNode clone(SqlParserPos pos) {
+  public SqlLiteral clone(SqlParserPos pos) {
     return new SqlLiteral(value, typeName, pos);
   }
 

http://git-wip-us.apache.org/repos/asf/calcite/blob/8441e79c/core/src/main/java/org/apache/calcite/sql/SqlNode.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/calcite/sql/SqlNode.java b/core/src/main/java/org/apache/calcite/sql/SqlNode.java
index e7c77fb..1d22f96 100644
--- a/core/src/main/java/org/apache/calcite/sql/SqlNode.java
+++ b/core/src/main/java/org/apache/calcite/sql/SqlNode.java
@@ -62,10 +62,20 @@ public abstract class SqlNode implements Cloneable {
 
   //~ Methods ----------------------------------------------------------------
 
+  /** @deprecated Please use {@link #clone(SqlNode)}; this method brings
+   * along too much baggage from early versions of Java */
+  @Deprecated
+  @SuppressWarnings("MethodDoesntCallSuperMethod")
   public Object clone() {
     return clone(getParserPosition());
   }
 
+  /** Creates a copy of a SqlNode. */
+  public static <E extends SqlNode> E clone(E e) {
+    //noinspection unchecked
+    return (E) e.clone(e.pos);
+  }
+
   /**
    * Clones a SqlNode with a different position.
    */
@@ -104,7 +114,7 @@ public abstract class SqlNode implements Cloneable {
     for (int i = 0; i < clones.length; i++) {
       SqlNode node = clones[i];
       if (node != null) {
-        clones[i] = (SqlNode) node.clone();
+        clones[i] = SqlNode.clone(node);
       }
     }
     return clones;

http://git-wip-us.apache.org/repos/asf/calcite/blob/8441e79c/core/src/main/java/org/apache/calcite/sql/SqlNumericLiteral.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/calcite/sql/SqlNumericLiteral.java b/core/src/main/java/org/apache/calcite/sql/SqlNumericLiteral.java
index b754a36..45f9dfb 100644
--- a/core/src/main/java/org/apache/calcite/sql/SqlNumericLiteral.java
+++ b/core/src/main/java/org/apache/calcite/sql/SqlNumericLiteral.java
@@ -65,13 +65,9 @@ public class SqlNumericLiteral extends SqlLiteral {
     return isExact;
   }
 
-  public SqlNode clone(SqlParserPos pos) {
-    return new SqlNumericLiteral(
-        (BigDecimal) value,
-        getPrec(),
-        getScale(),
-        isExact,
-        pos);
+  @Override public SqlNumericLiteral clone(SqlParserPos pos) {
+    return new SqlNumericLiteral((BigDecimal) value, getPrec(), getScale(),
+        isExact, pos);
   }
 
   public void unparse(

http://git-wip-us.apache.org/repos/asf/calcite/blob/8441e79c/core/src/main/java/org/apache/calcite/sql/SqlTimeLiteral.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/calcite/sql/SqlTimeLiteral.java b/core/src/main/java/org/apache/calcite/sql/SqlTimeLiteral.java
index 4327e2a..be3a04d 100644
--- a/core/src/main/java/org/apache/calcite/sql/SqlTimeLiteral.java
+++ b/core/src/main/java/org/apache/calcite/sql/SqlTimeLiteral.java
@@ -44,7 +44,7 @@ public class SqlTimeLiteral extends SqlAbstractDateTimeLiteral {
     return (TimeString) value;
   }
 
-  public SqlNode clone(SqlParserPos pos) {
+  @Override public SqlTimeLiteral clone(SqlParserPos pos) {
     return new SqlTimeLiteral((TimeString) value, precision, hasTimeZone, pos);
   }
 

http://git-wip-us.apache.org/repos/asf/calcite/blob/8441e79c/core/src/main/java/org/apache/calcite/sql/SqlTimestampLiteral.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/calcite/sql/SqlTimestampLiteral.java b/core/src/main/java/org/apache/calcite/sql/SqlTimestampLiteral.java
index cc659d5..41d9fe4 100644
--- a/core/src/main/java/org/apache/calcite/sql/SqlTimestampLiteral.java
+++ b/core/src/main/java/org/apache/calcite/sql/SqlTimestampLiteral.java
@@ -39,7 +39,7 @@ public class SqlTimestampLiteral extends SqlAbstractDateTimeLiteral {
 
   //~ Methods ----------------------------------------------------------------
 
-  public SqlNode clone(SqlParserPos pos) {
+  @Override public SqlTimestampLiteral clone(SqlParserPos pos) {
     return new SqlTimestampLiteral((TimestampString) value, precision,
         hasTimeZone, pos);
   }

http://git-wip-us.apache.org/repos/asf/calcite/blob/8441e79c/core/src/main/java/org/apache/calcite/sql/fun/SqlCoalesceFunction.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/calcite/sql/fun/SqlCoalesceFunction.java b/core/src/main/java/org/apache/calcite/sql/fun/SqlCoalesceFunction.java
index fc8d3c8..e2efc47 100644
--- a/core/src/main/java/org/apache/calcite/sql/fun/SqlCoalesceFunction.java
+++ b/core/src/main/java/org/apache/calcite/sql/fun/SqlCoalesceFunction.java
@@ -70,17 +70,14 @@ public class SqlCoalesceFunction extends SqlFunction {
 
     // todo: optimize when know operand is not null.
 
-    for (int i = 0; (i + 1) < operands.size(); ++i) {
+    for (SqlNode operand : Util.skipLast(operands)) {
       whenList.add(
-          SqlStdOperatorTable.IS_NOT_NULL.createCall(
-              pos,
-              operands.get(i)));
-      thenList.add(operands.get(i).clone(operands.get(i).getParserPosition()));
+          SqlStdOperatorTable.IS_NOT_NULL.createCall(pos, operand));
+      thenList.add(SqlNode.clone(operand));
     }
     SqlNode elseExpr = Util.last(operands);
     assert call.getFunctionQuantifier() == null;
-    return SqlCase.createSwitched(
-        pos, null, whenList, thenList, elseExpr);
+    return SqlCase.createSwitched(pos, null, whenList, thenList, elseExpr);
   }
 }
 

http://git-wip-us.apache.org/repos/asf/calcite/blob/8441e79c/core/src/main/java/org/apache/calcite/sql/fun/SqlNullifFunction.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/calcite/sql/fun/SqlNullifFunction.java b/core/src/main/java/org/apache/calcite/sql/fun/SqlNullifFunction.java
index 0aa310e..63cc774 100644
--- a/core/src/main/java/org/apache/calcite/sql/fun/SqlNullifFunction.java
+++ b/core/src/main/java/org/apache/calcite/sql/fun/SqlNullifFunction.java
@@ -67,9 +67,8 @@ public class SqlNullifFunction extends SqlFunction {
     SqlNodeList thenList = new SqlNodeList(pos);
     whenList.add(operands.get(1));
     thenList.add(SqlLiteral.createNull(SqlParserPos.ZERO));
-    return SqlCase.createSwitched(
-        pos, operands.get(0), whenList, thenList, operands.get(0).clone(
-        operands.get(0).getParserPosition()));
+    return SqlCase.createSwitched(pos, operands.get(0), whenList, thenList,
+        SqlNode.clone(operands.get(0)));
   }
 }
 

http://git-wip-us.apache.org/repos/asf/calcite/blob/8441e79c/core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java b/core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java
index cd83186..90bdb88 100644
--- a/core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java
+++ b/core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java
@@ -1191,8 +1191,7 @@ public class SqlValidatorImpl implements SqlValidatorWithHints {
       selectList.add(SqlIdentifier.star(SqlParserPos.ZERO));
       final SqlNodeList orderList;
       if (getInnerSelect(node) != null && isAggregate(getInnerSelect(node))) {
-        orderList =
-            orderBy.orderList.clone(orderBy.orderList.getParserPosition());
+        orderList = SqlNode.clone(orderBy.orderList);
         // We assume that ORDER BY item does not have ASC etc.
         // We assume that ORDER BY item is present in SELECT list.
         for (int i = 0; i < orderList.size(); i++) {
@@ -1281,9 +1280,7 @@ public class SqlValidatorImpl implements SqlValidatorWithHints {
       // from the update statement's source since it's the same as
       // what we want for the select list of the merge source -- '*'
       // followed by the update set expressions
-      selectList =
-          (SqlNodeList) updateStmt.getSourceSelect().getSelectList()
-              .clone();
+      selectList = SqlNode.clone(updateStmt.getSourceSelect().getSelectList());
     } else {
       // otherwise, just use select *
       selectList = new SqlNodeList(SqlParserPos.ZERO);
@@ -1305,7 +1302,7 @@ public class SqlValidatorImpl implements SqlValidatorWithHints {
     SqlNode sourceTableRef = call.getSourceTableRef();
     SqlInsert insertCall = call.getInsertCall();
     JoinType joinType = (insertCall == null) ? JoinType.INNER : JoinType.LEFT;
-    SqlNode leftJoinTerm = (SqlNode) sourceTableRef.clone();
+    final SqlNode leftJoinTerm = SqlNode.clone(sourceTableRef);
     SqlNode outerJoin =
         new SqlJoin(SqlParserPos.ZERO,
             leftJoinTerm,
@@ -1331,7 +1328,7 @@ public class SqlValidatorImpl implements SqlValidatorWithHints {
           new SqlNodeList(
               rowCall.getOperandList(),
               SqlParserPos.ZERO);
-      SqlNode insertSource = (SqlNode) sourceTableRef.clone();
+      final SqlNode insertSource = SqlNode.clone(sourceTableRef);
       select =
           new SqlSelect(SqlParserPos.ZERO, null, selectList, insertSource, null,
               null, null, null, null, null, null);

http://git-wip-us.apache.org/repos/asf/calcite/blob/8441e79c/core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorUtil.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorUtil.java b/core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorUtil.java
index c56c0ec..2200e01 100644
--- a/core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorUtil.java
+++ b/core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorUtil.java
@@ -1005,7 +1005,7 @@ public class SqlValidatorUtil {
     }
 
     public SqlNode visit(SqlLiteral literal) {
-      return (SqlNode) literal.clone();
+      return SqlNode.clone(literal);
     }
 
     public SqlNode visit(SqlIdentifier id) {
@@ -1020,15 +1020,15 @@ public class SqlValidatorUtil {
     }
 
     public SqlNode visit(SqlDataTypeSpec type) {
-      return (SqlNode) type.clone();
+      return SqlNode.clone(type);
     }
 
     public SqlNode visit(SqlDynamicParam param) {
-      return (SqlNode) param.clone();
+      return SqlNode.clone(param);
     }
 
     public SqlNode visit(SqlIntervalQualifier intervalQualifier) {
-      return (SqlNode) intervalQualifier.clone();
+      return SqlNode.clone(intervalQualifier);
     }
   }
 

http://git-wip-us.apache.org/repos/asf/calcite/blob/8441e79c/core/src/main/java/org/apache/calcite/util/NlsString.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/calcite/util/NlsString.java b/core/src/main/java/org/apache/calcite/util/NlsString.java
index eee7884..fbb9386 100644
--- a/core/src/main/java/org/apache/calcite/util/NlsString.java
+++ b/core/src/main/java/org/apache/calcite/util/NlsString.java
@@ -36,7 +36,7 @@ import static org.apache.calcite.util.Static.RESOURCE;
  * A string, optionally with {@link Charset character set} and
  * {@link SqlCollation}. It is immutable.
  */
-public class NlsString implements Comparable<NlsString> {
+public class NlsString implements Comparable<NlsString>, Cloneable {
   //~ Instance fields --------------------------------------------------------
 
   private final String charsetName;
@@ -47,7 +47,7 @@ public class NlsString implements Comparable<NlsString> {
   //~ Constructors -----------------------------------------------------------
 
   /**
-   * Creates a string in a specfied character set.
+   * Creates a string in a specified character set.
    *
    * @param value       String constant, must not be null
    * @param charsetName Name of the character set, may be null
@@ -91,7 +91,11 @@ public class NlsString implements Comparable<NlsString> {
   //~ Methods ----------------------------------------------------------------
 
   public Object clone() {
-    return new NlsString(value, charsetName, collation);
+    try {
+      return super.clone();
+    } catch (CloneNotSupportedException e) {
+      throw new AssertionError();
+    }
   }
 
   public int hashCode() {

http://git-wip-us.apache.org/repos/asf/calcite/blob/8441e79c/core/src/main/java/org/apache/calcite/util/XmlOutput.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/calcite/util/XmlOutput.java b/core/src/main/java/org/apache/calcite/util/XmlOutput.java
index ba1e5c8..e58cae0 100644
--- a/core/src/main/java/org/apache/calcite/util/XmlOutput.java
+++ b/core/src/main/java/org/apache/calcite/util/XmlOutput.java
@@ -18,10 +18,7 @@ package org.apache.calcite.util;
 
 import com.google.common.collect.Lists;
 
-import java.io.IOException;
-import java.io.LineNumberReader;
 import java.io.PrintWriter;
-import java.io.StringReader;
 import java.io.Writer;
 import java.util.ArrayDeque;
 import java.util.ArrayList;
@@ -397,19 +394,28 @@ public class XmlOutput {
    * Writes content.
    */
   public void content(String content) {
+    // This method previously used a LineNumberReader, but that class is
+    // susceptible to a form of DoS attack. It uses lots of memory and CPU if a
+    // malicious client gives it input with very long lines.
     if (content != null) {
       indent++;
-      LineNumberReader
-          in = new LineNumberReader(new StringReader(content));
-      try {
-        String line;
-        while ((line = in.readLine()) != null) {
+      final char[] chars = content.toCharArray();
+      int prev = 0;
+      for (int i = 0; i < chars.length; i++) {
+        if (chars[i] == '\n'
+            || chars[i] == '\r'
+            && i + 1 < chars.length
+            && chars[i + 1] == '\n') {
           displayIndent(out, indent);
-          out.println(line);
+          out.println(content.substring(prev, i));
+          if (chars[i] == '\r') {
+            ++i;
+          }
+          prev = i + 1;
         }
-      } catch (IOException ex) {
-        throw new AssertionError(ex);
       }
+      displayIndent(out, indent);
+      out.println(content.substring(prev, chars.length));
       indent--;
       out.flush();
     }

http://git-wip-us.apache.org/repos/asf/calcite/blob/8441e79c/core/src/test/java/org/apache/calcite/util/UtilTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/apache/calcite/util/UtilTest.java b/core/src/test/java/org/apache/calcite/util/UtilTest.java
index 080ad4c..3711654 100644
--- a/core/src/test/java/org/apache/calcite/util/UtilTest.java
+++ b/core/src/test/java/org/apache/calcite/util/UtilTest.java
@@ -30,6 +30,7 @@ import org.apache.calcite.runtime.FlatLists;
 import org.apache.calcite.runtime.Resources;
 import org.apache.calcite.runtime.SqlFunctions;
 import org.apache.calcite.runtime.Utilities;
+import org.apache.calcite.sql.SqlCollation;
 import org.apache.calcite.sql.SqlDialect;
 import org.apache.calcite.sql.util.SqlBuilder;
 import org.apache.calcite.sql.util.SqlString;
@@ -2007,6 +2008,42 @@ public class UtilTest {
     assertThat(map.range("BAZ", true).size(), is(0));
     assertThat(map.range("Baz", true).size(), is(1));
   }
+
+  @Test public void testNlsStringClone() {
+    final NlsString s = new NlsString("foo", "LATIN1", SqlCollation.IMPLICIT);
+    assertThat(s.toString(), is("_LATIN1'foo'"));
+    final Object s2 = s.clone();
+    assertThat(s2, instanceOf(NlsString.class));
+    assertThat(s2, not(sameInstance((Object) s)));
+    assertThat(s2.toString(), is(s.toString()));
+  }
+
+  @Test public void testXmlOutput() {
+    final StringWriter w = new StringWriter();
+    final XmlOutput o = new XmlOutput(w);
+    o.beginBeginTag("root");
+    o.attribute("a1", "v1");
+    o.attribute("a2", null);
+    o.endBeginTag("root");
+    o.beginTag("someText", null);
+    o.content("line 1 followed by empty line\n"
+        + "\n"
+        + "line 3 with windows line ending\r\n"
+        + "line 4 with no ending");
+    o.endTag("someText");
+    o.endTag("root");
+    final String s = w.toString();
+    final String expected = ""
+        + "<root a1=\"v1\">\n"
+        + "\t<someText>\n"
+        + "\t\t\tline 1 followed by empty line\n"
+        + "\t\t\t\n"
+        + "\t\t\tline 3 with windows line ending\n"
+        + "\t\t\tline 4 with no ending\n"
+        + "\t</someText>\n"
+        + "</root>\n";
+    assertThat(s, is(expected));
+  }
 }
 
 // End UtilTest.java