twalthr commented on code in PR #28277:
URL: https://github.com/apache/flink/pull/28277#discussion_r3334787309
##########
flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/operations/converters/materializedtable/AbstractCreateMaterializedTableConverter.java:
##########
@@ -141,10 +141,20 @@ protected final RefreshMode
getDerivedRefreshMode(LogicalRefreshMode logicalRefr
return
MaterializedTableUtils.fromLogicalRefreshModeToRefreshMode(logicalRefreshMode);
}
+ /**
+ * Returns the {@code AS} query exactly as the user wrote it.
+ *
+ * <p>The query text is sliced out of the original sql statement using the
parser position
Review Comment:
nit: SQL should be upper case in comments.
```suggestion
* <p>The query text is sliced out of the original SQL statement using
the parser position
```
##########
flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/operations/converters/SqlNodeConvertUtils.java:
##########
@@ -110,6 +103,14 @@ static CatalogView toCatalogView(
schema = ResolvedSchema.physical(aliasFieldNames,
schema.getColumnDataTypes());
}
+ // Capture the user's original wording (case, formatting, type
aliases) by slicing the
+ // sql statement using the parser position on the query node, instead
of unparsing the
+ // SqlNode. Re-rendering the parsed tree would normalize identifier
casing and expand
+ // type aliases (e.g. `int` to `INTEGER`), which is undesirable for
the original query
+ // we persist on the catalog view.
+ final String originalQuery =
+
context.toRawSqlString(query).orElse(context.toQuotedSqlString(query));
Review Comment:
Is there a chance this can ever happen? If not just throw. Otherwise it just
spams the code.
```suggestion
context.toRawSqlString(query).orElseThfow(IllegalState::new);
```
##########
flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/operations/converters/table/SqlCreateTableAsConverter.java:
##########
@@ -47,21 +47,30 @@ public class SqlCreateTableAsConverter extends
AbstractCreateTableConverter<SqlC
public Operation convertSqlNode(SqlCreateTableAs sqlCreateTableAs,
ConvertContext context) {
final FlinkPlannerImpl flinkPlanner = context.getFlinkPlanner();
final CatalogManager catalogManager = context.getCatalogManager();
- SqlNode asQuerySqlNode = sqlCreateTableAs.getAsQuery();
- SqlNode validatedAsQuery = flinkPlanner.validate(asQuerySqlNode);
+ final SqlNode asQuerySqlNode = sqlCreateTableAs.getAsQuery();
+ final SqlNode validatedAsQuery = flinkPlanner.validate(asQuerySqlNode);
+ final String rawSqlString =
+ context.toRawSqlString(asQuerySqlNode)
+ .orElseThrow(
+ () ->
+ new TableException(
+ "Failed to get raw SQL string
for the AS query in CREATE TABLE AS statement."));
Review Comment:
Why do we throw for some cases and for other we fall back to quoted?
##########
flink-table/flink-table-planner/src/test/java/org/apache/flink/table/planner/operations/SqlMaterializedTableNodeToOperationConverterTest.java:
##########
@@ -320,6 +320,38 @@ void testCreateMaterializedTableWithUDTFQuery() {
+ "LATERAL
TABLE(`builtin`.`default`.`myFunc`(`b`)) AS `T` (`f1`, `f2`)");
}
+ @ParameterizedTest(name = "{0}")
+ @MethodSource("originalQueryCommentCases")
+ void testOriginalQueryCommentHandling(String name, String sql, String
expectedOriginalQuery) {
+ final String originalQuery =
+ createMaterializedTableOperation(sql)
+ .getCatalogMaterializedTable()
+ .getOriginalQuery();
+ assertThat(originalQuery).isEqualTo(expectedOriginalQuery);
+ }
+
+ private static Collection<Arguments> originalQueryCommentCases() {
+ // comments before the first token or after the last token fall
outside the node and are
+ // not preserved.
+ return List.of(
+ Arguments.of(
+ "inline line comment inside query is kept",
Review Comment:
can you test:
```
CREATE MATERIALIZED TABLE `mtbl` AS
CREATE MATERIALIZED TABLE `m``tbl` AS
CREATE MATERIALIZED TABLE `m``tbl` COMMENT 'a' AS
CREATE MATERIALIZED TABLE `m``tbl` COMMENT 'a' || 'a' AS
```
a bit more sophisticated prefix, to check the parser pos logic.
--
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]
