raminqaf commented on code in PR #28277:
URL: https://github.com/apache/flink/pull/28277#discussion_r3354420135
##########
flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/operations/converters/materializedtable/AbstractCreateMaterializedTableConverter.java:
##########
@@ -141,10 +142,17 @@ protected final RefreshMode
getDerivedRefreshMode(LogicalRefreshMode logicalRefr
return
MaterializedTableUtils.fromLogicalRefreshModeToRefreshMode(logicalRefreshMode);
}
+ /**
+ * Returns the user's original {@code AS} query text, sliced verbatim from
the statement so
+ * formatting and identifier casing are preserved (e.g. {@code int} is not
normalized to {@code
+ * INTEGER}). A comment placed between {@code AS} and the query is kept;
the {@code AS} keyword
+ * itself is excluded.
+ */
protected final String getDerivedOriginalQuery(
T sqlCreateMaterializedTable, ConvertContext context) {
- SqlNode selectQuery = sqlCreateMaterializedTable.getAsQuery();
- return context.toQuotedSqlString(selectQuery);
+ return SqlNodeConvertUtils.rawAsQueryText(
Review Comment:
Renamed to `originalAsQueryText`. I agree with you that the `As...` in the
method name makes it harder to read. In order to keep it consistent with the
other variable like the `asQuery`, I would keep the `As...` in the method name.
##########
flink-table/flink-sql-parser/src/main/codegen/includes/parserImpls.ftl:
##########
@@ -2062,24 +2063,27 @@ SqlCreate SqlCreateOrAlterMaterializedTable(Span s,
boolean replace, boolean isT
}
)
]
- <AS>
+ <AS> { asKeywordPos = getPos(); }
Review Comment:
`getPos()` returns the position of the token that was just consumed.
```sql
CREATE MATERIALIZED TABLE m AS SELECT 1
^^
cols 29–30 → getPos() = pos(line 1, col 29 → line
1, col 30)
```
`startPos.plus(getPos())` the position captured at the start of the
statement (the CREATE keyword, col 1)
```sql
CREATE MATERIALIZED TABLE m AS SELECT 1
^---------------------------^
cols 1 .. 30 = the whole statement so far (CREATE … AS)
```
so `getPos()` here is correct, since we want just the position of the `AS`
keyword.
##########
flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/operations/converters/SqlNodeConvertUtils.java:
##########
@@ -46,14 +48,74 @@
import javax.annotation.Nullable;
import java.util.HashMap;
+import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;
import java.util.stream.Collectors;
/** Utilities for SqlNode conversions. */
-class SqlNodeConvertUtils {
+public class SqlNodeConvertUtils {
+
+ /**
+ * Returns the user's original {@code AS}-query text: the verbatim
substring of the statement
+ * from immediately after the {@code AS} keyword to the end of the
statement, trimmed of
+ * surrounding whitespace.
+ *
+ * <p>The {@code AS}-query is the final clause of {@code CREATE/ALTER
MATERIALIZED TABLE} and
+ * {@code CREATE/ALTER VIEW}, so everything after {@code AS} is the query:
comments around it
+ * are kept, the {@code AS} keyword is excluded. Slicing to the statement
end (rather than the
+ * query node's parser position) keeps the whole query for node types
whose position does not
+ * span their full text, e.g. {@code WITH ... SELECT}, where {@code
SqlWith} ends at the
+ * with-list and would otherwise drop the trailing {@code SELECT}. Slicing
from the source also
+ * preserves the user's wording (case, formatting, type aliases) that
re-rendering the parsed
+ * tree would normalize.
+ *
+ * <p>Returns {@link Optional#empty()} when the statement text is
unavailable (e.g. a
+ * synthesized node with no source text); callers decide the fallback.
+ *
+ * @param asKeywordPos parser position of the {@code AS} keyword
+ * @return the verbatim AS-query text, or empty when the statement text is
unavailable
+ */
+ public static Optional<String> rawAsQueryText(
+ ConvertContext context, @Nullable SqlParserPos asKeywordPos) {
+ if (asKeywordPos == null) {
+ return Optional.empty();
+ }
+ final String statementText = context.getStatementText();
+ if (statementText != null) {
+ final int start = offsetAfterAs(statementText, asKeywordPos);
+ if (start >= 0) {
+ return Optional.of(statementText.substring(start).strip());
+ }
+ }
+ return Optional.empty();
+ }
+
+ /**
+ * Returns the offset of the first character after the {@code AS} keyword,
or {@code -1} if the
+ * keyword position falls outside {@code statement}.
+ */
+ private static int offsetAfterAs(String statement, SqlParserPos
asKeywordPos) {
Review Comment:
Use `OptionalInt` now
##########
flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/operations/SqlNodeConvertContext.java:
##########
@@ -110,6 +123,19 @@ public String toQuotedSqlString(SqlNode sqlNode) {
return sqlNode.toSqlString(getSqlDialect()).getSql();
}
+ @Override
+ @Nullable
+ public String getStatementText() {
+ return this.statement;
+ }
+
+ @Override
+ public String expandSqlIdentifiers(String originalSql) {
Review Comment:
Just wanted to have all the Overrides in the correct order. It is unrelated
so I moved it back.
##########
flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/operations/converters/SqlNodeConvertUtils.java:
##########
@@ -46,14 +48,74 @@
import javax.annotation.Nullable;
import java.util.HashMap;
+import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;
import java.util.stream.Collectors;
/** Utilities for SqlNode conversions. */
-class SqlNodeConvertUtils {
+public class SqlNodeConvertUtils {
+
+ /**
Review Comment:
The wall of text is gone now
##########
flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/operations/converters/SqlNodeConvertUtils.java:
##########
@@ -46,14 +48,74 @@
import javax.annotation.Nullable;
import java.util.HashMap;
+import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;
import java.util.stream.Collectors;
/** Utilities for SqlNode conversions. */
-class SqlNodeConvertUtils {
+public class SqlNodeConvertUtils {
+
+ /**
+ * Returns the user's original {@code AS}-query text: the verbatim
substring of the statement
+ * from immediately after the {@code AS} keyword to the end of the
statement, trimmed of
+ * surrounding whitespace.
+ *
+ * <p>The {@code AS}-query is the final clause of {@code CREATE/ALTER
MATERIALIZED TABLE} and
+ * {@code CREATE/ALTER VIEW}, so everything after {@code AS} is the query:
comments around it
+ * are kept, the {@code AS} keyword is excluded. Slicing to the statement
end (rather than the
+ * query node's parser position) keeps the whole query for node types
whose position does not
+ * span their full text, e.g. {@code WITH ... SELECT}, where {@code
SqlWith} ends at the
+ * with-list and would otherwise drop the trailing {@code SELECT}. Slicing
from the source also
+ * preserves the user's wording (case, formatting, type aliases) that
re-rendering the parsed
+ * tree would normalize.
+ *
+ * <p>Returns {@link Optional#empty()} when the statement text is
unavailable (e.g. a
+ * synthesized node with no source text); callers decide the fallback.
+ *
+ * @param asKeywordPos parser position of the {@code AS} keyword
+ * @return the verbatim AS-query text, or empty when the statement text is
unavailable
+ */
+ public static Optional<String> rawAsQueryText(
+ ConvertContext context, @Nullable SqlParserPos asKeywordPos) {
+ if (asKeywordPos == null) {
+ return Optional.empty();
+ }
+ final String statementText = context.getStatementText();
+ if (statementText != null) {
+ final int start = offsetAfterAs(statementText, asKeywordPos);
+ if (start >= 0) {
+ return Optional.of(statementText.substring(start).strip());
+ }
+ }
+ return Optional.empty();
+ }
+
+ /**
+ * Returns the offset of the first character after the {@code AS} keyword,
or {@code -1} if the
+ * keyword position falls outside {@code statement}.
+ */
+ private static int offsetAfterAs(String statement, SqlParserPos
asKeywordPos) {
Review Comment:
Made the method more general now
##########
flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/operations/converters/SqlNodeConvertUtils.java:
##########
@@ -110,6 +166,13 @@ static CatalogView toCatalogView(
schema = ResolvedSchema.physical(aliasFieldNames,
schema.getColumnDataTypes());
}
+ // Capture the user's original wording (case, formatting, type
aliases) by slicing the
Review Comment:
Removed the comments
##########
flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/operations/converters/SqlNodeConvertUtils.java:
##########
@@ -46,14 +48,74 @@
import javax.annotation.Nullable;
import java.util.HashMap;
+import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;
import java.util.stream.Collectors;
/** Utilities for SqlNode conversions. */
-class SqlNodeConvertUtils {
+public class SqlNodeConvertUtils {
+
+ /**
+ * Returns the user's original {@code AS}-query text: the verbatim
substring of the statement
+ * from immediately after the {@code AS} keyword to the end of the
statement, trimmed of
+ * surrounding whitespace.
+ *
+ * <p>The {@code AS}-query is the final clause of {@code CREATE/ALTER
MATERIALIZED TABLE} and
+ * {@code CREATE/ALTER VIEW}, so everything after {@code AS} is the query:
comments around it
+ * are kept, the {@code AS} keyword is excluded. Slicing to the statement
end (rather than the
+ * query node's parser position) keeps the whole query for node types
whose position does not
+ * span their full text, e.g. {@code WITH ... SELECT}, where {@code
SqlWith} ends at the
+ * with-list and would otherwise drop the trailing {@code SELECT}. Slicing
from the source also
+ * preserves the user's wording (case, formatting, type aliases) that
re-rendering the parsed
+ * tree would normalize.
+ *
+ * <p>Returns {@link Optional#empty()} when the statement text is
unavailable (e.g. a
+ * synthesized node with no source text); callers decide the fallback.
+ *
+ * @param asKeywordPos parser position of the {@code AS} keyword
+ * @return the verbatim AS-query text, or empty when the statement text is
unavailable
+ */
+ public static Optional<String> rawAsQueryText(
+ ConvertContext context, @Nullable SqlParserPos asKeywordPos) {
+ if (asKeywordPos == null) {
+ return Optional.empty();
+ }
+ final String statementText = context.getStatementText();
+ if (statementText != null) {
Review Comment:
Inverted and refactored the method
--
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]
