yuqi1129 commented on code in PR #12016:
URL: https://github.com/apache/gravitino/pull/12016#discussion_r3814360941
##########
catalogs-contrib/catalog-jdbc-clickhouse/src/main/java/org/apache/gravitino/catalog/clickhouse/operations/ClickHouseTableOperations.java:
##########
@@ -1376,23 +1376,174 @@ Transform[] parsePartitioning(String partitionKey) {
return ClickHouseTableSqlUtils.parsePartitioning(partitionKey);
}
+ /**
+ * Strips PROJECTION definition blocks from a {@code SHOW CREATE TABLE} DDL
string so that
+ * internal {@code ORDER BY} / {@code PARTITION BY} clauses inside
projection bodies are not
+ * mistaken for the table-level sort key or partitioning expression.
+ *
+ * <p>A projection block has the form {@code PROJECTION name ( SELECT ... )}
and sits inside the
+ * column-definition body of the DDL. This method removes every such block
including the optional
+ * trailing comma, while preserving string literals and respecting nested
parentheses.
+ *
+ * @param createSql raw {@code SHOW CREATE TABLE} output
+ * @return the DDL with all PROJECTION blocks removed, or the original
string if none are found
+ */
+ @VisibleForTesting
+ String stripProjections(String createSql) {
+ if (StringUtils.isBlank(createSql)) {
+ return createSql;
+ }
+
+ StringBuilder result = new StringBuilder(createSql.length());
+ int i = 0;
+ int len = createSql.length();
+
+ while (i < len) {
+ char ch = createSql.charAt(i);
+
+ // ----- skip single-quoted string literals (preserve as-is) -----
+ if (ch == '\'') {
+ result.append(ch);
+ i++;
+ while (i < len) {
+ char c = createSql.charAt(i);
+ result.append(c);
+ if (c == '\'') {
+ // escaped single quote: ''
+ if (i + 1 < len && createSql.charAt(i + 1) == '\'') {
+ result.append('\'');
+ i += 2;
+ continue;
+ }
+ i++;
+ break;
+ }
+ i++;
+ }
+ continue;
+ }
+
+ // ----- skip backtick-quoted identifiers (preserve as-is) -----
+ // NOTE: does not handle ClickHouse double-backtick escaping
(``col``name`).
+ // This is safe because SHOW CREATE TABLE output uses only simple ASCII
+ // identifiers where escaping is never necessary, and PROJECTION is
always
+ // a keyword (never backtick-quoted).
+ if (ch == '`') {
+ result.append(ch);
+ i++;
+ while (i < len && createSql.charAt(i) != '`') {
+ result.append(createSql.charAt(i));
+ i++;
+ }
+ if (i < len) {
+ result.append(createSql.charAt(i)); // closing backtick
+ i++;
+ }
+ continue;
+ }
+
+ // ----- detect PROJECTION keyword -----
+ if (i + "PROJECTION".length() <= len) {
+ String candidate = createSql.substring(i, i + "PROJECTION".length());
+ if ("PROJECTION".equalsIgnoreCase(candidate)) {
+ // word boundary before
+ boolean boundaryBefore =
+ i == 0 || !Character.isJavaIdentifierPart(createSql.charAt(i -
1));
+ int afterKw = i + "PROJECTION".length();
+ // word boundary after (or end-of-string)
+ boolean boundaryAfter =
+ afterKw >= len ||
!Character.isJavaIdentifierPart(createSql.charAt(afterKw));
+ if (boundaryBefore && boundaryAfter) {
+ // Skip PROJECTION keyword and whitespace
+ i = afterKw;
+ while (i < len && Character.isWhitespace(createSql.charAt(i))) {
+ i++;
+ }
+ // Skip projection name (backtick-quoted or simple identifier)
+ if (i < len && createSql.charAt(i) == '`') {
+ i++;
+ while (i < len && createSql.charAt(i) != '`') {
+ i++;
+ }
+ if (i < len) i++; // closing backtick
+ } else {
+ while (i < len
+ && (Character.isJavaIdentifierPart(createSql.charAt(i))
+ || createSql.charAt(i) == '_')) {
+ i++;
+ }
+ }
+ // Skip whitespace to reach '('
+ while (i < len && Character.isWhitespace(createSql.charAt(i))) {
+ i++;
+ }
+ // Skip the projection body — bracket-counting aware
+ if (i < len && createSql.charAt(i) == '(') {
+ int depth = 1;
+ i++;
+ while (i < len && depth > 0) {
+ char bodyCh = createSql.charAt(i);
+ if (bodyCh == '\'') {
+ // skip string literal inside projection body
+ i++;
+ while (i < len) {
+ if (createSql.charAt(i) == '\'') {
Review Comment:
ClickHouse `SHOW CREATE TABLE` uses backslash escapes for string literals.
For example, input `it''s` is returned as `it\'s`. This code treats the quote
after `\` as the end of the string. It may then skip the rest of the DDL,
including the table-level `ORDER BY`.
The current test uses `''`, but that is not the actual `SHOW CREATE TABLE`
output. Could we handle backslash escapes in both string loops and add a test
with real ClickHouse output?
##########
catalogs-contrib/catalog-jdbc-clickhouse/src/main/java/org/apache/gravitino/catalog/clickhouse/operations/ClickHouseTableOperations.java:
##########
@@ -1376,23 +1376,174 @@ Transform[] parsePartitioning(String partitionKey) {
return ClickHouseTableSqlUtils.parsePartitioning(partitionKey);
}
+ /**
+ * Strips PROJECTION definition blocks from a {@code SHOW CREATE TABLE} DDL
string so that
+ * internal {@code ORDER BY} / {@code PARTITION BY} clauses inside
projection bodies are not
+ * mistaken for the table-level sort key or partitioning expression.
+ *
+ * <p>A projection block has the form {@code PROJECTION name ( SELECT ... )}
and sits inside the
+ * column-definition body of the DDL. This method removes every such block
including the optional
+ * trailing comma, while preserving string literals and respecting nested
parentheses.
+ *
+ * @param createSql raw {@code SHOW CREATE TABLE} output
+ * @return the DDL with all PROJECTION blocks removed, or the original
string if none are found
+ */
+ @VisibleForTesting
+ String stripProjections(String createSql) {
Review Comment:
Could we avoid parsing projection definitions here? ClickHouse already
provides the table sort expression in `system.tables.sorting_key`. This class
also reads `partition_key` from `system.tables`, and `getTableProperties()`
already queries the same table. Reusing `sorting_key` would let us remove this
long parser and avoid more ClickHouse syntax edge cases.
##########
catalogs-contrib/catalog-jdbc-clickhouse/src/main/java/org/apache/gravitino/catalog/clickhouse/operations/ClickHouseTableOperations.java:
##########
@@ -1376,23 +1376,174 @@ Transform[] parsePartitioning(String partitionKey) {
return ClickHouseTableSqlUtils.parsePartitioning(partitionKey);
}
+ /**
+ * Strips PROJECTION definition blocks from a {@code SHOW CREATE TABLE} DDL
string so that
+ * internal {@code ORDER BY} / {@code PARTITION BY} clauses inside
projection bodies are not
+ * mistaken for the table-level sort key or partitioning expression.
+ *
+ * <p>A projection block has the form {@code PROJECTION name ( SELECT ... )}
and sits inside the
+ * column-definition body of the DDL. This method removes every such block
including the optional
+ * trailing comma, while preserving string literals and respecting nested
parentheses.
+ *
+ * @param createSql raw {@code SHOW CREATE TABLE} output
+ * @return the DDL with all PROJECTION blocks removed, or the original
string if none are found
+ */
+ @VisibleForTesting
+ String stripProjections(String createSql) {
+ if (StringUtils.isBlank(createSql)) {
+ return createSql;
+ }
+
+ StringBuilder result = new StringBuilder(createSql.length());
+ int i = 0;
+ int len = createSql.length();
+
+ while (i < len) {
+ char ch = createSql.charAt(i);
+
+ // ----- skip single-quoted string literals (preserve as-is) -----
+ if (ch == '\'') {
+ result.append(ch);
+ i++;
+ while (i < len) {
+ char c = createSql.charAt(i);
+ result.append(c);
+ if (c == '\'') {
+ // escaped single quote: ''
+ if (i + 1 < len && createSql.charAt(i + 1) == '\'') {
+ result.append('\'');
+ i += 2;
+ continue;
+ }
+ i++;
+ break;
+ }
+ i++;
+ }
+ continue;
+ }
+
+ // ----- skip backtick-quoted identifiers (preserve as-is) -----
+ // NOTE: does not handle ClickHouse double-backtick escaping
(``col``name`).
+ // This is safe because SHOW CREATE TABLE output uses only simple ASCII
+ // identifiers where escaping is never necessary, and PROJECTION is
always
+ // a keyword (never backtick-quoted).
+ if (ch == '`') {
+ result.append(ch);
+ i++;
+ while (i < len && createSql.charAt(i) != '`') {
+ result.append(createSql.charAt(i));
+ i++;
+ }
+ if (i < len) {
+ result.append(createSql.charAt(i)); // closing backtick
+ i++;
+ }
+ continue;
+ }
+
+ // ----- detect PROJECTION keyword -----
+ if (i + "PROJECTION".length() <= len) {
+ String candidate = createSql.substring(i, i + "PROJECTION".length());
+ if ("PROJECTION".equalsIgnoreCase(candidate)) {
+ // word boundary before
+ boolean boundaryBefore =
+ i == 0 || !Character.isJavaIdentifierPart(createSql.charAt(i -
1));
+ int afterKw = i + "PROJECTION".length();
+ // word boundary after (or end-of-string)
+ boolean boundaryAfter =
+ afterKw >= len ||
!Character.isJavaIdentifierPart(createSql.charAt(afterKw));
+ if (boundaryBefore && boundaryAfter) {
+ // Skip PROJECTION keyword and whitespace
+ i = afterKw;
+ while (i < len && Character.isWhitespace(createSql.charAt(i))) {
+ i++;
+ }
+ // Skip projection name (backtick-quoted or simple identifier)
+ if (i < len && createSql.charAt(i) == '`') {
+ i++;
+ while (i < len && createSql.charAt(i) != '`') {
+ i++;
+ }
+ if (i < len) i++; // closing backtick
+ } else {
+ while (i < len
+ && (Character.isJavaIdentifierPart(createSql.charAt(i))
+ || createSql.charAt(i) == '_')) {
+ i++;
+ }
+ }
+ // Skip whitespace to reach '('
+ while (i < len && Character.isWhitespace(createSql.charAt(i))) {
+ i++;
+ }
+ // Skip the projection body — bracket-counting aware
+ if (i < len && createSql.charAt(i) == '(') {
+ int depth = 1;
+ i++;
+ while (i < len && depth > 0) {
+ char bodyCh = createSql.charAt(i);
+ if (bodyCh == '\'') {
+ // skip string literal inside projection body
+ i++;
+ while (i < len) {
+ if (createSql.charAt(i) == '\'') {
+ if (i + 1 < len && createSql.charAt(i + 1) == '\'') {
+ i += 2;
+ continue;
+ }
+ i++;
+ break;
+ }
+ i++;
+ }
+ } else {
+ if (bodyCh == '(') depth++;
+ else if (bodyCh == ')') depth--;
+ i++;
+ }
+ }
+ // Skip trailing whitespace and optional comma
Review Comment:
Newer ClickHouse versions allow `PROJECTION p (...) WITH SETTINGS (...)`.
This code stops after the projection body and leaves `WITH SETTINGS (...)` in
`cleanedSql`. Then `SETTINGS_PATTERN.find()` may read projection settings as
table settings.
Could we remove the full projection declaration, including `WITH SETTINGS`,
or avoid parsing projection syntax here? See the current ClickHouse parser:
https://github.com/ClickHouse/ClickHouse/blob/master/src/Parsers/ParserCreateQuery.cpp#L267-L334
--
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]
