Skip to content

[#12913] fix(clickhouse): preserve quoted commas in table settings - #12918

Draft
jiangxt2 wants to merge 1 commit into
apache:mainfrom
jiangxt2:fix/clickhouse-quoted-comma-settings
Draft

[#12913] fix(clickhouse): preserve quoted commas in table settings#12918
jiangxt2 wants to merge 1 commit into
apache:mainfrom
jiangxt2:fix/clickhouse-quoted-comma-settings

Conversation

@jiangxt2

@jiangxt2 jiangxt2 commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug report] ClickHouse catalog truncates quoted-comma table SETTINGS during load

1 participant