That looks like a bug. Cloning a SqlCall and then calling setOperand on it is valid, and should not modify the original SqlCall.
We tend to avoid calling ’set’ methods on SqlNodes - we treat them as immutable except that the validator will replace an argument with an equivalent argument (e.g. replacing an identifier with a fully-qualified identifier). That’s an explanation of why we don’t tend to hit this problem - I’m not claiming it’s not a bug. Can you log a JIRA case please? Separately, it’s crazy that the “operands" field [1] of SqlBasicCall is public. We should deprecate the field for one point release (mirroring it into a private mutable list field) and then remove it. Later we can make the private list field immutable (copy on write) or something. Julian [1] https://github.com/apache/calcite/blob/4bc916619fd286b2c0cc4d5c653c96a68801d74e/core/src/main/java/org/apache/calcite/sql/SqlBasicCall.java#L34 <https://github.com/apache/calcite/blob/4bc916619fd286b2c0cc4d5c653c96a68801d74e/core/src/main/java/org/apache/calcite/sql/SqlBasicCall.java#L34> > On Sep 22, 2021, at 3:36 PM, Wen Zhang <zhang...@cs.berkeley.edu> wrote: > > Hello, > > I'm trying to make shallow copies of `SqlNode` objects by calling the > `clone(SqlParserPos pos) > <https://calcite.apache.org/javadocAggregate/org/apache/calcite/sql/SqlNode.html#clone(org.apache.calcite.sql.parser.SqlParserPos)>` > method. However, the `SqlBasicCall` class overrides this method with an > implementation > <https://github.com/apache/calcite/blob/4b1e9ed1a513eae0c873a660b755bb4f27b39da9/core/src/main/java/org/apache/calcite/sql/SqlBasicCall.java#L100> > that might return a new `SqlCall` that shares the operand array with the > original. This is due to it calling `SqlOperator.createCall(SqlLiteral > functionQualifier, SqlParserPos pos, SqlNode... operands) > <https://calcite.apache.org/javadocAggregate/org/apache/calcite/sql/SqlOperator.html#createCall(org.apache.calcite.sql.SqlLiteral,org.apache.calcite.sql.parser.SqlParserPos,org.apache.calcite.sql.SqlNode...)>`, > whose default implementation > <https://github.com/apache/calcite/blob/4b1e9ed1a513eae0c873a660b755bb4f27b39da9/core/src/main/java/org/apache/calcite/sql/SqlOperator.java#L272> > creates a new `SqlBasicCall` object without copying `operands`. > > Is my understanding correct? > > Thank you! > > Best regards, > Wen >
