[DO NOT MERGE] [Cherry-pick to branch-1.3] [#12761] fix(clickhouse): Propagate clustered table rename (#12763) - #12790
Closed
github-actions[bot] wants to merge 1 commit into
Closed
Conversation
### What changes were proposed in this pull request? - Overrides `ClickHouseTableOperations.rename()` so tables with trusted Gravitino cluster metadata use `RENAME TABLE ... ON CLUSTER ...`, while local and unmarked external tables keep the existing local rename behavior. - Reads the old table metadata from `system.tables` with an exact current-database and table-name predicate, and rejects a present but blank cluster marker before executing DDL. - Preserves the existing ClickHouse exception mapping and identifier quoting contract without changing the common JDBC rename path. - Expands the repository ClickHouse cluster fixture from one self-referencing node to three independently addressable nodes. - Adds focused unit coverage, local SQL regression coverage, and a three-node lifecycle test that verifies the initiating query, all-node old/new state, comment metadata preservation, and cleanup after drop. ### Why are the changes needed? Fix: #12761 The common JDBC rename path generates a local `RENAME TABLE old_name TO new_name` statement. For ClickHouse tables created through Gravitino with `ON CLUSTER`, that statement renames only the JDBC connection node and leaves the old name on the other nodes, silently splitting cluster metadata and making later DDL inconsistent. ### Does this PR introduce _any_ user-facing change? Yes. Renaming a Gravitino-created ClickHouse table with trusted cluster metadata now propagates to every configured cluster node. Local tables and unmarked external tables retain local rename behavior. This PR does not add or change catalog-facing APIs or property keys. ### 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 with no skipped, failed, or errored tests. - `./gradlew :catalogs-contrib:catalog-jdbc-clickhouse:test --tests "org.apache.gravitino.catalog.clickhouse.integration.test.CatalogClickHouseIT.testAlterAndDropClickhouseTable" -PskipDockerTests=false` — passed with `tests=1 skipped=0 failures=0 errors=0`. - `./gradlew :catalogs-contrib:catalog-jdbc-clickhouse:test --tests "org.apache.gravitino.catalog.clickhouse.integration.test.CatalogClickHouseClusterIT" -PskipDockerTests=false` — passed on three ClickHouse 24.8.14 nodes with `tests=19 skipped=0 failures=0 errors=0`. - `./gradlew :catalogs-contrib:catalog-jdbc-clickhouse:build -x test` — passed. - `python3 ~/GitHub/bin/gravitino-pr-precheck.py --worktree ~/GitHub/workspace/gravitino-fix-clickhouse-clustered-table-rename` — all checks passed. Signed-off-by: jiangxt2 <jiangxt2@vip.qq.com> # Conflicts: # catalogs-contrib/catalog-jdbc-clickhouse/src/test/java/org/apache/gravitino/catalog/clickhouse/operations/TestClickHouseTableOperationsUnit.java
github-actions
Bot
added
cherry-pick
Contributor
|
@jiangxt2 can you raise a new PR to fix the merge conflicts? |
Contributor
Contributor
|
Close it as #12832 has replaced this one. |
jerryshao
pushed a commit
that referenced
this pull request
Sep 2, 2026
…ered table rename (#12763) (#12832) ### What changes were proposed in this pull request? This PR manually backports #12763 to `branch-1.3` and supersedes the unresolved automated cherry-pick PR #12790. - Override the ClickHouse table rename path so tables with trusted Gravitino cluster metadata use `RENAME TABLE ... ON CLUSTER ...`, while unmarked external tables keep local rename behavior. - Read the existing table metadata from `system.tables` with an exact current-database and table-name predicate, and reject a present but blank cluster marker before executing DDL. - Preserve the existing ClickHouse exception mapping and identifier quoting contract without changing the common JDBC rename path. - Expand the ClickHouse cluster fixture to three independently addressable nodes and add lifecycle coverage for rename propagation, query text, all-node state, comment metadata, and cleanup. - Resolve the branch-1.3 unit-test conflict by retaining the target branch tests and only the rename-related tests from #12763; source-branch-only index parameter tests are not included. ### Why are the changes needed? The common JDBC rename path generates a local `RENAME TABLE old_name TO new_name` statement. For ClickHouse tables created through Gravitino with `ON CLUSTER`, that statement renames only the JDBC connection node and leaves the old name on the other nodes, silently splitting cluster metadata and making later DDL inconsistent. ClickHouse does not include `ON CLUSTER` in `SHOW CREATE TABLE`, so the connector must use the trusted cluster marker embedded in the stored comment when generating the rename statement. The automated backport PR #12790 contains unresolved conflict markers in the ClickHouse unit test. This PR resolves that conflict against the current `branch-1.3` baseline without carrying unrelated source-branch changes. The source-branch-only `StringIdentifier.ID_KEY` property assertions are not carried into this `branch-1.3` backport because the existing branch-1.3 ClickHouse flow does not expose that derived property for this test path. The cluster integration test still verifies that the raw StringIdentifier and cluster marker remain in `system.tables.comment` on every node. Fix: #12761 ### Does this PR introduce _any_ user-facing change? Yes. Renaming a Gravitino-created ClickHouse table with trusted cluster metadata now propagates the rename to every configured cluster node. Local tables and unmarked external tables retain local rename behavior. This PR does not add, remove, or change catalog-facing APIs or property keys. ### How was this patch tested? - `./gradlew :catalogs-contrib:catalog-jdbc-clickhouse:spotlessCheck` — passed. - `./gradlew rat` — passed. - `./gradlew :catalogs-contrib:catalog-jdbc-clickhouse:test -PskipITs -PskipDockerTests=true` — passed. - `./gradlew :catalogs-contrib:catalog-jdbc-clickhouse:test --tests 'org.apache.gravitino.catalog.clickhouse.integration.test.CatalogClickHouseIT' -PskipDockerTests=false` — passed. - `./gradlew :catalogs-contrib:catalog-jdbc-clickhouse:test --tests 'org.apache.gravitino.catalog.clickhouse.integration.test.CatalogClickHouseClusterIT' -PskipDockerTests=false` — passed with 18 tests, 0 skipped, 0 failures, and 0 errors. - `./gradlew :catalogs-contrib:catalog-jdbc-clickhouse:build -x test` — passed. 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
3 participants
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.
Cherry-pick Information:
branch-1.3Do not merge until conflict markers are resolved and the
cherry-pick-conflictlabel is removed.Please review and resolve the conflicts before merging.