You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@polaris.apache.org by "kotwal-itpro (via GitHub)" <gi...@apache.org> on 2026/10/08 06:42:32 UTC
[PR] Docs and test: promote changes across tables with one multi-table commit [polaris]
kotwal-itpro opened a new pull request, #5740:
URL: https://github.com/apache/polaris/pull/5740
Related to #3469. Follows Dmitri's suggestion on the dev@ thread "[DISCUSS] Catalog-level branches for atomic multi-table promotion" to start with a test and a doc for option B.
Users who backfill or publish several related tables need readers to see either the old state of all of them or the new state of all of them. Polaris can already do this with Iceberg branches and the multi-table commit endpoint: create the branch on every table in one commit, write to it, then move `main` of every table to its branch head in one commit that asserts `main` hasn't moved. This PR documents that recipe and adds a test that pins the behavior it relies on.
**Doc:** `site/content/in-dev/unreleased/multi-table-promotion.md` walks through the steps with request bodies, says what the recipe doesn't cover (schema changes are table-wide on Iceberg branches, only existing tables can be branched, the client tracks membership), and notes the privileges a multi-table commit needs (`TABLE_WRITE_PROPERTIES` and `TABLE_CREATE` on every table) and that federated and static-facade catalogs aren't supported.
**Test:** `BranchPromotionRecipeTest` runs the recipe through the REST handler with three tables:
- `promotesEveryTableAtOnce`: branch created in one commit, one snapshot written to each branch, promotion moves `main` of all three tables to their branch heads.
- `promotesNothingWhenOneTableMovedOnMain`: after branching, another writer moves `main` of one table; the promotion commit is rejected and none of the three tables change.
The test uses the in-memory metastore of the service tests. On the multi-entity consistency discussion ([thread](https://lists.apache.org/thread/622d0sn4wzhw993dr5yhmz5zhy4228hp)): the recipe only updates the refs of the tables it commits, and each of those is written with a check against the version that was read, so it doesn't rely on validations that depend on other entities. Happy to add the same test against the NoSQL persistence if that's useful.
No code changes outside the test.
I used an AI coding assistant while preparing this change; I reviewed and tested all of it and am responsible for it.
## Checklist
- [x] ๐ก๏ธ Don't disclose security issues! (contact security@apache.org)
- [x] ๐ Clearly explained why the changes are needed, or linked related issues: Related to #3469
- [x] ๐งช Added/updated tests with good coverage, or manually tested (and explained how)
- [x] ๐ก Added comments for complex logic
- [ ] ๐งพ Updated `CHANGELOG.md` (if needed) โ not needed, docs and test only
- [x] ๐ Updated documentation in `site/content/in-dev/unreleased` (if needed)
--
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: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Docs and test: promote changes across tables with one multi-table commit [polaris]
Posted by "orca-review-bot (via GitHub)" <gi...@apache.org>.
orca-review-bot commented on code in PR #5740:
URL: https://github.com/apache/polaris/pull/5740#discussion_r4395466945
##########
runtime/service/src/test/java/org/apache/polaris/service/catalog/iceberg/BranchPromotionRecipeTest.java:
##########
@@ -0,0 +1,277 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.polaris.service.catalog.iceberg;
+
+import static org.apache.polaris.service.admin.PolarisAuthzTestBase.SCHEMA;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+
+import jakarta.ws.rs.core.Response;
+import java.nio.file.Path;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+import java.util.UUID;
+import org.apache.iceberg.MetadataUpdate;
+import org.apache.iceberg.Snapshot;
+import org.apache.iceberg.SnapshotParser;
+import org.apache.iceberg.SnapshotRefType;
+import org.apache.iceberg.TableMetadata;
+import org.apache.iceberg.UpdateRequirement;
+import org.apache.iceberg.catalog.Namespace;
+import org.apache.iceberg.catalog.TableIdentifier;
+import org.apache.iceberg.rest.requests.CommitTransactionRequest;
+import org.apache.iceberg.rest.requests.CreateNamespaceRequest;
+import org.apache.iceberg.rest.requests.CreateTableRequest;
+import org.apache.iceberg.rest.requests.UpdateTableRequest;
+import org.apache.iceberg.rest.responses.LoadTableResponse;
+import org.apache.polaris.core.admin.model.Catalog;
+import org.apache.polaris.core.admin.model.CatalogProperties;
+import org.apache.polaris.core.admin.model.CreateCatalogRequest;
+import org.apache.polaris.core.admin.model.FileStorageConfigInfo;
+import org.apache.polaris.core.admin.model.StorageConfigInfo;
+import org.apache.polaris.service.TestServices;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.io.TempDir;
+
+/**
+ * Atomic promotion of a branch across several tables with nothing but the Iceberg REST multi-table
+ * commit: create the branch on every table in one commit, write to it, then move every table's main
+ * to its branch head in one commit that asserts main hasn't moved.
+ */
+public class BranchPromotionRecipeTest {
+ private static final String CATALOG = "lake";
+ private static final String NAMESPACE = "sales";
+ private static final List<String> TABLES = List.of("orders", "items", "customers");
+ private static final String BRANCH = "backfill";
+
+ private String catalogLocation;
+ private TestServices services;
+ private long nextSnapshotId = 1000;
+
+ @BeforeEach
+ void setUp(@TempDir Path tempDir) {
+ catalogLocation = tempDir.toAbsolutePath().toUri().toString().replaceAll("/$", "");
+ services =
+ TestServices.builder()
+ .config(
+ Map.of(
+ "ALLOW_INSECURE_STORAGE_TYPES",
+ "true",
+ "SUPPORTED_CATALOG_STORAGE_TYPES",
+ List.of("FILE")))
+ .build();
+ createCatalogNamespaceAndTables();
+ }
+
+ @Test
+ void promotesEveryTableAtOnce() {
+ List<Long> bases = commitToAll("main", null);
+ createBranchOnAll(bases);
+ List<Long> heads = commitToAll(BRANCH, bases);
+
+ promote(bases, heads);
+
+ for (int i = 0; i < TABLES.size(); i++) {
+ TableMetadata metadata = load(TABLES.get(i));
+ assertThat(metadata.ref("main").snapshotId()).isEqualTo(heads.get(i));
+ }
+ }
+
+ @Test
+ void promotesNothingWhenOneTableMovedOnMain() {
+ List<Long> bases = commitToAll("main", null);
+ createBranchOnAll(bases);
+ List<Long> heads = commitToAll(BRANCH, bases);
+
+ // Someone else writes to main of one table after the branch was created.
+ String moved = TABLES.get(1);
+ long concurrent = nextSnapshotId++;
+ commit(
+ List.of(
+ UpdateTableRequest.create(
+ TableIdentifier.of(NAMESPACE, moved),
+ List.of(),
+ List.of(
+ new MetadataUpdate.AddSnapshot(snapshot(moved, concurrent, bases.get(1))),
+ setRef("main", concurrent)))));
+
+ assertThatThrownBy(() -> promote(bases, heads)).isNotNull();
Review Comment:
This passes on any exception. A bad snapshot JSON or a 400 from setup would look like a successful rejection. The failed ref assertion throws `CommitFailedException` (the 409 the doc promises), so can we assert `.isInstanceOf(CommitFailedException.class)`?
<sub>๐ค AI-generated review comment โ agents make mistakes, so please verify before acting on it.</sub>
##########
runtime/service/src/test/java/org/apache/polaris/service/catalog/iceberg/BranchPromotionRecipeTest.java:
##########
@@ -0,0 +1,277 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.polaris.service.catalog.iceberg;
+
+import static org.apache.polaris.service.admin.PolarisAuthzTestBase.SCHEMA;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+
+import jakarta.ws.rs.core.Response;
+import java.nio.file.Path;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+import java.util.UUID;
+import org.apache.iceberg.MetadataUpdate;
+import org.apache.iceberg.Snapshot;
+import org.apache.iceberg.SnapshotParser;
+import org.apache.iceberg.SnapshotRefType;
+import org.apache.iceberg.TableMetadata;
+import org.apache.iceberg.UpdateRequirement;
+import org.apache.iceberg.catalog.Namespace;
+import org.apache.iceberg.catalog.TableIdentifier;
+import org.apache.iceberg.rest.requests.CommitTransactionRequest;
+import org.apache.iceberg.rest.requests.CreateNamespaceRequest;
+import org.apache.iceberg.rest.requests.CreateTableRequest;
+import org.apache.iceberg.rest.requests.UpdateTableRequest;
+import org.apache.iceberg.rest.responses.LoadTableResponse;
+import org.apache.polaris.core.admin.model.Catalog;
+import org.apache.polaris.core.admin.model.CatalogProperties;
+import org.apache.polaris.core.admin.model.CreateCatalogRequest;
+import org.apache.polaris.core.admin.model.FileStorageConfigInfo;
+import org.apache.polaris.core.admin.model.StorageConfigInfo;
+import org.apache.polaris.service.TestServices;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.io.TempDir;
+
+/**
+ * Atomic promotion of a branch across several tables with nothing but the Iceberg REST multi-table
+ * commit: create the branch on every table in one commit, write to it, then move every table's main
+ * to its branch head in one commit that asserts main hasn't moved.
+ */
+public class BranchPromotionRecipeTest {
+ private static final String CATALOG = "lake";
+ private static final String NAMESPACE = "sales";
+ private static final List<String> TABLES = List.of("orders", "items", "customers");
+ private static final String BRANCH = "backfill";
+
+ private String catalogLocation;
+ private TestServices services;
+ private long nextSnapshotId = 1000;
+
+ @BeforeEach
+ void setUp(@TempDir Path tempDir) {
+ catalogLocation = tempDir.toAbsolutePath().toUri().toString().replaceAll("/$", "");
+ services =
+ TestServices.builder()
+ .config(
+ Map.of(
+ "ALLOW_INSECURE_STORAGE_TYPES",
+ "true",
+ "SUPPORTED_CATALOG_STORAGE_TYPES",
+ List.of("FILE")))
+ .build();
+ createCatalogNamespaceAndTables();
+ }
+
+ @Test
+ void promotesEveryTableAtOnce() {
+ List<Long> bases = commitToAll("main", null);
+ createBranchOnAll(bases);
+ List<Long> heads = commitToAll(BRANCH, bases);
+
+ promote(bases, heads);
+
+ for (int i = 0; i < TABLES.size(); i++) {
+ TableMetadata metadata = load(TABLES.get(i));
+ assertThat(metadata.ref("main").snapshotId()).isEqualTo(heads.get(i));
+ }
+ }
+
+ @Test
+ void promotesNothingWhenOneTableMovedOnMain() {
+ List<Long> bases = commitToAll("main", null);
+ createBranchOnAll(bases);
+ List<Long> heads = commitToAll(BRANCH, bases);
+
+ // Someone else writes to main of one table after the branch was created.
+ String moved = TABLES.get(1);
+ long concurrent = nextSnapshotId++;
+ commit(
+ List.of(
+ UpdateTableRequest.create(
+ TableIdentifier.of(NAMESPACE, moved),
+ List.of(),
+ List.of(
+ new MetadataUpdate.AddSnapshot(snapshot(moved, concurrent, bases.get(1))),
+ setRef("main", concurrent)))));
+
+ assertThatThrownBy(() -> promote(bases, heads)).isNotNull();
+
+ assertThat(load(TABLES.get(0)).ref("main").snapshotId()).isEqualTo(bases.get(0));
+ assertThat(load(moved).ref("main").snapshotId()).isEqualTo(concurrent);
+ assertThat(load(TABLES.get(2)).ref("main").snapshotId()).isEqualTo(bases.get(2));
+ }
+
+ private void promote(List<Long> bases, List<Long> heads) {
+ List<UpdateTableRequest> changes = new ArrayList<>();
+ for (int i = 0; i < TABLES.size(); i++) {
+ changes.add(
+ UpdateTableRequest.create(
+ TableIdentifier.of(NAMESPACE, TABLES.get(i)),
+ List.of(new UpdateRequirement.AssertRefSnapshotID("main", bases.get(i))),
+ List.of(setRef("main", heads.get(i)))));
+ }
+ commit(changes);
+ }
+
+ private void createBranchOnAll(List<Long> bases) {
Review Comment:
Step 1 of the doc creates the branch with an `assert-ref-snapshot-id` on `main`, but this sends no requirements. Can we add `new UpdateRequirement.AssertRefSnapshotID("main", bases.get(i))` here so the test runs the request the doc shows?
<sub>๐ค AI-generated review comment โ agents make mistakes, so please verify before acting on it.</sub>
##########
site/content/in-dev/unreleased/multi-table-promotion.md:
##########
@@ -0,0 +1,158 @@
+---
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+#
+title: Promoting Changes Across Tables
+type: docs
+weight: 440
+---
+
+Some changes only make sense across several tables at once: a backfill that rewrites related
+fact tables, a migration that changes dimensions and facts together, or a daily load that is
+checked before anyone may read it (write-audit-publish). If the tables are updated one by one,
+readers can see a mix of old and new data, and a failure halfway through leaves the tables
+inconsistent.
+
+Polaris can make such a change visible on all tables at once, using Iceberg branches and the
+multi-table commit endpoint of the Iceberg REST API, `POST /v1/{prefix}/transactions/commit`. A
+multi-table commit applies the changes to every table in the request, or to none of them.
+
+No Polaris-specific feature is involved: the steps below use standard Iceberg table branches, and
+work with any client that can send a multi-table commit.
+
+## The steps
+
+The examples use a catalog `lake` with tables `sales.orders` and `sales.items`, and a branch named
+`backfill`.
+
+### 1. Create the branch on every table
+
+Load each table and note the snapshot `main` points to. Then create the branch on all tables in
+one commit. The `assert-ref-snapshot-id` requirement makes the commit fail if `main` of any table
+changed in the meantime.
+
+```json
+POST /api/catalog/v1/lake/transactions/commit
+
+{
+ "table-changes": [
+ {
+ "identifier": {"namespace": ["sales"], "name": "orders"},
+ "requirements": [{"type": "assert-ref-snapshot-id", "ref": "main", "snapshot-id": 1001}],
+ "updates": [{"action": "set-snapshot-ref", "ref-name": "backfill", "type": "branch", "snapshot-id": 1001}]
+ },
+ {
+ "identifier": {"namespace": ["sales"], "name": "items"},
+ "requirements": [{"type": "assert-ref-snapshot-id", "ref": "main", "snapshot-id": 2001}],
+ "updates": [{"action": "set-snapshot-ref", "ref-name": "backfill", "type": "branch", "snapshot-id": 2001}]
+ }
+ ]
+}
+```
+
+### 2. Write to the branch
+
+Engines write to an Iceberg branch as usual, and readers of `main` don't see those writes. For
+example, in Spark:
+
+```sql
+INSERT INTO lake.sales.orders.branch_backfill SELECT ...;
+```
+
+### 3. Review the change
+
+Before promoting, check what will change on each table. You already have both ends of the change:
+the snapshot `main` points to (from step 1) and the head of `backfill`. Load each table and note
+the head of the branch; these are the snapshots you will promote in the next step.
+
+* Query the branch directly, for example `lake.sales.orders.branch_backfill`, to validate the data.
+* The summary of each snapshot between the two (in the table's `snapshots` metadata table) shows
+ how many records and files were added and removed.
+* For row-level changes, Iceberg's Spark procedure `create_changelog_view` lists the rows inserted
+ and deleted between two snapshots. The start snapshot is exclusive and the end snapshot inclusive:
+
+```sql
+CALL lake.system.create_changelog_view(
+ table => 'sales.orders',
+ options => map('start-snapshot-id', '1001', 'end-snapshot-id', '1005'),
+ changelog_view => 'orders_backfill_changes'
+);
+```
+
+A person or an automated check can approve the change at this point. Promote the branch heads you
+reviewed, not whatever the branch points to later: the commit in the next step names each snapshot
+explicitly, so writes to the branch after the review are not promoted.
+
+### 4. Promote the branch
+
+Move `main` of every table to the branch head you reviewed in step 3, in one commit. The
+requirement asserts that `main` is still at the snapshot it pointed to in step 1:
+
+```json
+POST /api/catalog/v1/lake/transactions/commit
+
+{
+ "table-changes": [
+ {
+ "identifier": {"namespace": ["sales"], "name": "orders"},
+ "requirements": [{"type": "assert-ref-snapshot-id", "ref": "main", "snapshot-id": 1001}],
+ "updates": [{"action": "set-snapshot-ref", "ref-name": "main", "type": "branch", "snapshot-id": 1005}]
+ },
+ {
+ "identifier": {"namespace": ["sales"], "name": "items"},
+ "requirements": [{"type": "assert-ref-snapshot-id", "ref": "main", "snapshot-id": 2001}],
+ "updates": [{"action": "set-snapshot-ref", "ref-name": "main", "type": "branch", "snapshot-id": 2007}]
+ }
+ ]
+}
+```
+
+Readers now see the new state of every table. If another writer changed `main` of any of the
Review Comment:
The commit is all-or-nothing, but readers still load each table separately. A reader that loads `orders` before the promotion and `items` after it sees a mix. Can we soften this line? Can we also add a bullet under "What this does not cover" saying reads that span several tables aren't pinned to one point?
<sub>๐ค AI-generated review comment โ agents make mistakes, so please verify before acting on it.</sub>
--
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: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Docs and test: promote changes across tables with one multi-table commit [polaris]
Posted by "kotwal-itpro (via GitHub)" <gi...@apache.org>.
kotwal-itpro commented on PR #5740:
URL: https://github.com/apache/polaris/pull/5740#issuecomment-6453633710
This has been open since Oct 14 and is mergeable. It documents and tests promoting several tables at once with the existing multi-table commit, covering both the success case and the case where one table moved on main and nothing changes. Qiegang's transactions design (#5833) publishes with the same commit when main hasn't moved, so the test may also be useful there. @dimas-b @jbonofre, would one of you have time to review?
--
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: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] docs: promote changes across tables with one multi-table commit [polaris]
Posted by "kotwal-itpro (via GitHub)" <gi...@apache.org>.
kotwal-itpro commented on PR #5740:
URL: https://github.com/apache/polaris/pull/5740#issuecomment-6476765104
Thanks for the review. All addressed in d90d39742, rebased on current main:
- The conflict test now asserts `CommitFailedException`.
- The branch is created with `assert-ref-snapshot-id` on `main`, the request the doc shows.
- The doc no longer says readers see every table at once. A new item under "What this does not cover" says reads that span several tables aren't pinned to one point.
- The title now has the `docs:` prefix, and the description says the conflict test pins the requirement check, not a concurrent race.
--
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: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org