dssysolyatin commented on code in PR #4696:
URL: https://github.com/apache/calcite/pull/4696#discussion_r2642207496
##########
core/src/main/java/org/apache/calcite/sql/ddl/SqlCreateFunction.java:
##########
@@ -91,6 +104,9 @@ private List<Pair<SqlLiteral, SqlLiteral>> pairs() {
}
@Override public List<SqlNode> getOperandList() {
- return Arrays.asList(name, className, usingList);
+ return Arrays.asList(
Review Comment:
Can we use `ImmutableList` as in another places ?
##########
core/src/main/java/org/apache/calcite/sql/SqlCollation.java:
##########
@@ -113,6 +115,31 @@ public SqlCollation(
this.collationName = generateCollationName(charset);
}
+ /** Encode all the information require to reconstruct a SqlCollection in a
SqlList object. */
+ public SqlNodeList asList() {
Review Comment:
Why we can not just use string generated
by`SqlCollation.generateCollationName` ? and construct SqlCollation using
```
@JsonCreator
public SqlCollation(
@JsonProperty("collationName") String collation,
@JsonProperty("coercibility") Coercibility coercibility) {
```
##########
core/src/main/java/org/apache/calcite/sql/SqlLambda.java:
##########
@@ -107,6 +107,13 @@ private static class SqlLambdaOperator extends
SqlSpecialOperator {
super("->", SqlKind.LAMBDA);
}
+ @Override public SqlCall createCall(
Review Comment:
Do you need lambda in this PR ?
##########
server/src/test/java/org/apache/calcite/test/ServerParserTest.java:
##########
@@ -86,6 +99,24 @@ class ServerParserTest extends SqlParserTest {
sql(sql).ok(expected);
}
+ /** Test case for <a
href="https://issues.apache.org/jira/browse/CALCITE-7339";>[CALCITE-7339]
+ * Most classes in SqlDdlNodes use an incorrect SqlCallFactory</a>. */
+ @Test void testShuttle() throws SqlParseException {
Review Comment:
Does this test tests everything ?
Can we just add common test
https://github.com/apache/calcite/pull/4644/files#diff-766dbf236baaf33399a8d3620404b261faa80c27dadb95379e0747f915380e44
but have boolean flag which enable it only for `ServerUnParserTest` ?
##########
core/src/main/java/org/apache/calcite/sql/SqlCollation.java:
##########
@@ -113,6 +115,31 @@ public SqlCollation(
this.collationName = generateCollationName(charset);
}
+ /** Encode all the information require to reconstruct a SqlCollection in a
SqlList object. */
+ public SqlNodeList asList() {
+ return new SqlNodeList(
+ ImmutableNullableList.of(
+ SqlLiteral.createCharString(coercibility.name(),
SqlParserPos.ZERO),
Review Comment:
Can we use SqlLiteral.createSymbol as it works with enums ?
--
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]
