dssysolyatin commented on code in PR #4644:
URL: https://github.com/apache/calcite/pull/4644#discussion_r2554300828
##########
babel/src/main/codegen/includes/parserImpls.ftl:
##########
@@ -145,7 +145,7 @@ void ColumnWithType(List<SqlNode> list) :
]
{
list.add(SqlDdlNodes.column(s.add(id).end(this), id,
- type.withNullable(nullable), null, null));
Review Comment:
It was added because `SqlDdlNodes.column` does not expect `null` for
`ColumnStrategy`.
##########
site/_docs/history.md:
##########
@@ -49,6 +49,12 @@ other software versions as specified in gradle.properties.
#### Breaking Changes
{: #breaking-1-42-0}
+* [<a
href="https://issues.apache.org/jira/browse/CALCITE-7301";>CALCITE-7301</a>]
Review Comment:
It is a breaking change, but it should not affect anyone. All `SqlNode`
classes inside `org.apache.calcite.sql.ddl` expose their operands through
public fields, and none of these nodes implement `setOperand`. Because of that,
I expect that getOperandList() is not used outside the Calcite codebase for
these SqlNode types.
For `SqlUnpivot`, I added the SqlLiteral at the end of the operand list.
##########
babel/src/main/java/org/apache/calcite/sql/babel/SqlBabelCreateTable.java:
##########
@@ -36,13 +58,21 @@ public class SqlBabelCreateTable extends SqlCreateTable {
/** Creates a SqlBabelCreateTable. */
public SqlBabelCreateTable(SqlParserPos pos, boolean replace,
TableCollectionType tableCollectionType, boolean volatile_,
- boolean ifNotExists, SqlIdentifier name, SqlNodeList columnList,
- SqlNode query) {
- super(pos, replace, ifNotExists, name, columnList, query);
+ boolean ifNotExists, SqlIdentifier name, @Nullable SqlNodeList
columnList,
+ @Nullable SqlNode query) {
+ super(OPERATOR, pos, replace, ifNotExists, name, columnList, query);
this.tableCollectionType = tableCollectionType;
this.volatile_ = volatile_;
}
+ @SuppressWarnings("nullness")
+ @Override public List<SqlNode> getOperandList() {
+ return ImmutableNullableList.of(SqlLiteral.createBoolean(getReplace(),
pos),
+ SqlLiteral.createSymbol(tableCollectionType, pos),
Review Comment:
I’m taking the most straightforward approach using SqlLiteral for boolean
flag for `replace` and i`fNotExists`. Ideally, `replace` and `ifNotExists`
should each have their own `SqlNode` implementation.
That way, we won’t need to write code like this every time:
```
if (ifNotExists) {
writer.keyword("IF NOT EXISTS");
}
```
Instead, we could just call:
```
ifNotExists.unparse(...)
```
--
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]
