mihaibudiu commented on code in PR #4644:
URL: https://github.com/apache/calcite/pull/4644#discussion_r2557360651
##########
babel/src/test/java/org/apache/calcite/test/BabelParserTest.java:
##########
@@ -390,7 +391,12 @@ private void checkParseInfixCast(String sqlType) {
}
@Test void testPostgresSqlSetOption() {
- SqlParserFixture f = fixture().withDialect(PostgresqlSqlDialect.DEFAULT);
+ // UnparsingTesterImpl has a check where it unparses a SqlNode into a SQL
string
+ // using the calcite dialect, and then parses it back into a SqlNode.
+ // But the SQL string produced by the calcite dialect for `SET` cannot
always be parsed back.
Review Comment:
does this require filing a new issue?
if so, perhaps you can add here the issue link.
##########
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));
+ type.withNullable(nullable), null, ColumnStrategy.DEFAULT));
Review Comment:
what is the relationship of this change with the issue that is being
addressed?
##########
core/src/main/java/org/apache/calcite/sql/ddl/SqlCheckConstraint.java:
##########
@@ -21,23 +21,30 @@
import org.apache.calcite.sql.SqlKind;
import org.apache.calcite.sql.SqlNode;
import org.apache.calcite.sql.SqlOperator;
-import org.apache.calcite.sql.SqlSpecialOperator;
import org.apache.calcite.sql.SqlWriter;
+import org.apache.calcite.sql.fun.SqlBasicOperator;
import org.apache.calcite.sql.parser.SqlParserPos;
import org.apache.calcite.util.ImmutableNullableList;
import org.checkerframework.checker.nullness.qual.Nullable;
import java.util.List;
+import static java.util.Objects.requireNonNull;
+
/**
* Parse tree for {@code UNIQUE}, {@code PRIMARY KEY} constraints.
*
* <p>And {@code FOREIGN KEY}, when we support it.
*/
public class SqlCheckConstraint extends SqlCall {
- private static final SqlSpecialOperator OPERATOR =
- new SqlSpecialOperator("CHECK", SqlKind.CHECK);
+ private static final SqlOperator OPERATOR =
Review Comment:
I can sense some scope creep; you started with lambda and now you are doing
many other things.
This is not even part of the list of operators in the JIRA issue.
why not do them in separate PRs?
##########
core/src/main/java/org/apache/calcite/sql/ddl/SqlAttributeDefinition.java:
##########
@@ -23,23 +23,30 @@
import org.apache.calcite.sql.SqlKind;
import org.apache.calcite.sql.SqlNode;
import org.apache.calcite.sql.SqlOperator;
-import org.apache.calcite.sql.SqlSpecialOperator;
import org.apache.calcite.sql.SqlWriter;
+import org.apache.calcite.sql.fun.SqlBasicOperator;
import org.apache.calcite.sql.parser.SqlParserPos;
-
-import com.google.common.collect.ImmutableList;
+import org.apache.calcite.util.ImmutableNullableList;
import org.checkerframework.checker.nullness.qual.Nullable;
import java.util.List;
+import static java.util.Objects.requireNonNull;
+
/**
* Parse tree for SqlAttributeDefinition,
* which is part of a {@link SqlCreateType}.
*/
public class SqlAttributeDefinition extends SqlCall {
- private static final SqlSpecialOperator OPERATOR =
- new SqlSpecialOperator("ATTRIBUTE_DEF", SqlKind.ATTRIBUTE_DEF);
+ private static final SqlOperator OPERATOR =
Review Comment:
is this in scope for the current issue?
--
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]
