[#11880] fix(clickhouse): preserve SETTINGS clause on table round-trip#11885
Open
jiangxt2 wants to merge 1 commit into
Open
[#11880] fix(clickhouse): preserve SETTINGS clause on table round-trip#11885jiangxt2 wants to merge 1 commit into
jiangxt2 wants to merge 1 commit into
Conversation
…nd-trip - Add SETTINGS_PATTERN regex to parse SETTINGS from SHOW CREATE TABLE output - Add settings field to ShowCreateTableMetadata inner class - Parse SETTINGS key=value pairs with settings. prefix in parseSettingsClause() - Merge parsed settings into table properties in load() path Signed-off-by: jiangxt2 <jiangxt2@vip.qq.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changes were proposed in this pull request?
Fix SETTINGS clause lost on table round-trip in the ClickHouse catalog.
The write path (
appendTableProperties) correctly generatesSETTINGS key = valuefromtable properties with
settings.prefix, but the read path never parsed SETTINGS back intotable properties. This caused SETTINGS to be silently dropped on round-trip.
Changes:
SETTINGS_PATTERNregex to extract the SETTINGS clause fromSHOW CREATE TABLEoutputShowCreateTableMetadatawith asettingsfieldparseCreateStatement()now parses SETTINGS content (in addition to existing ORDER BY / PARTITION BY parsing)load()merges parsed SETTINGS into table properties withsettings.prefixWhy are the changes needed?
Fix: #11880
Tables with custom SETTINGS (e.g.
index_granularity = 4096) lost those settings when loadedthrough Gravitino, because the read path only extracted COMMENT, ENGINE, and cluster metadata
from
system.tables.Does this PR introduce any user-facing change?
Yes. Table properties now include
settings.*keys for tables with custom SETTINGS, enablingcorrect round-trip of ClickHouse table settings.
How was this patch tested?
testParseSettingsFromCreateSqlinTestClickHouseTableOperations(4 test cases: empty,single, multiple settings, settings with COMMENT). Note: this class has
@Tag("gravitino-docker-test")at class level, so the test runs under
-PskipDockerTests=false(not-PskipITs), though thetest logic itself is pure string parsing with no container dependency.
./gradlew :catalogs-contrib:catalog-jdbc-clickhouse:test -PskipDockerTests=false --tests "TestClickHouseTableOperations"