jiangxt2 opened a new pull request, #12918: URL: https://github.com/apache/gravitino/pull/12918
### What changes were proposed in this pull request? This pull request fixes ClickHouse table-level SETTINGS readback when a scalar literal contains commas. - Replace the unconditional comma split in `ClickHouseTableOperations.parseSettingsClause()` with top-level delimiter parsing that ignores commas and equals signs inside quoted text and nested parentheses. - Preserve the complete server-returned scalar-literal text and the existing `settings.*` property representation, including the current last-value-wins behavior for duplicate keys. - Reuse the existing ClickHouse quote and parenthesis scanning behavior for backslash escapes, doubled quote delimiters, and quoted identifiers. - Reject structurally malformed SETTINGS metadata with a setting-specific error that does not include the raw value. - Add focused unit coverage and a real ClickHouse native-load and load/recreate regression test. No public API, property key, dependency, documentation, ALTER behavior, or general SQL parsing capability is changed. ### Why are the changes needed? The ClickHouse catalog reads table-level settings from `system.tables.engine_full`, but the current parser splits the SETTINGS clause at every comma. A valid String setting such as `merge_workload = 'gravitino,quoted,comma'` is therefore loaded as only `'gravitino`, even though ClickHouse preserves the complete literal. Recreating a table from the loaded property cannot retain the original value. The existing write path already treats each `settings.*` value as one complete ClickHouse scalar literal, so this change restores read/write symmetry without changing the property contract. Fix: #12913 ### Does this PR introduce _any_ user-facing change? Yes. Loading a ClickHouse table now preserves complete table-level setting values when quoted text or nested function-style values contain commas. Existing simple settings and `settings.*` property keys are unchanged. Structurally malformed SETTINGS metadata now fails explicitly instead of being silently ignored or converted into truncated properties. ### How was this patch tested? - `./gradlew :catalogs-contrib:catalog-jdbc-clickhouse:spotlessCheck` — passed. - `./gradlew rat` — passed. - `./gradlew :catalogs-contrib:catalog-jdbc-clickhouse:test -PskipITs` — passed, including quoted-comma, escaping, doubled-quote, nested-parenthesis, duplicate-key, malformed-metadata, engine-parameter, and Graphite regression coverage. - `./gradlew :catalogs-contrib:catalog-jdbc-clickhouse:build -x test` — passed. - `./gradlew :catalogs-contrib:catalog-jdbc-clickhouse:test --tests 'org.apache.gravitino.catalog.clickhouse.integration.test.CatalogClickHouseIT.testLoadAndRecreateTableWithQuotedCommaSetting' -PskipDockerTests=false` — passed against the project ClickHouse 24.8.14 image with `tests=1`, `skipped=0`, `failures=0`, and `errors=0`. - `./gradlew :catalogs-contrib:catalog-jdbc-clickhouse:test --tests 'org.apache.gravitino.catalog.clickhouse.integration.test.CatalogClickHouseClusterIT' -PskipDockerTests=false` — passed against the project three-node ClickHouse 24.8.14 cluster fixture with `tests=19`, `skipped=0`, `failures=0`, and `errors=0`. - A local supplemental Gravitino static precheck completed with no errors. -- 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]
