singhpk234 commented on code in PR #3360:
URL: https://github.com/apache/polaris/pull/3360#discussion_r2894368333
##########
integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationBase.java:
##########
@@ -1189,6 +1189,101 @@ public void
testMultipleConflictingCommitsToSingleTableInTransaction() {
assertThat(latestCommittedSchema.asStruct()).isEqualTo(originalSchema.asStruct());
}
+ @Test
+ public void testCoalescedConflictOnOneTableRollsBackEntireTransaction() {
+ Namespace namespace = Namespace.of("coalescingAtomicNs");
+ TableIdentifier goodId = TableIdentifier.of(namespace, "goodTable");
+ TableIdentifier conflictId = TableIdentifier.of(namespace,
"conflictTable");
+
+ if (requiresNamespaceCreate()) {
+ catalog().createNamespace(namespace);
+ }
+
+ catalog().createTable(goodId, SCHEMA);
+ catalog().createTable(conflictId, SCHEMA);
+
+ Schema originalGoodSchema = catalog().loadTable(goodId).schema();
+ Schema originalConflictSchema = catalog().loadTable(conflictId).schema();
+
+ // goodTable: a single non-conflicting schema change
+ Transaction goodTx = catalog().loadTable(goodId).newTransaction();
+ goodTx.updateSchema().addColumn("new_col", Types.LongType.get()).commit();
+
+ // conflictTable: two independent transactions that both rename the same
column,
+ // producing two TableCommits for the same table. The first rename
succeeds,
+ // but the second conflicts because the schema has already changed.
+ Table conflictTable = catalog().loadTable(conflictId);
+ Transaction conflictTx1 = conflictTable.newTransaction();
+ Transaction conflictTx2 = conflictTable.newTransaction();
+ conflictTx1.updateSchema().renameColumn("data", "renamed-col1").commit();
+ conflictTx2.updateSchema().renameColumn("data", "renamed-col2").commit();
+
+ TableCommit goodCommit =
+ TableCommit.create(
+ goodId,
+ ((BaseTransaction) goodTx).startMetadata(),
+ ((BaseTransaction) goodTx).currentMetadata());
+ TableCommit conflictCommit1 =
+ TableCommit.create(
+ conflictId,
+ ((BaseTransaction) conflictTx1).startMetadata(),
+ ((BaseTransaction) conflictTx1).currentMetadata());
+ TableCommit conflictCommit2 =
+ TableCommit.create(
+ conflictId,
+ ((BaseTransaction) conflictTx2).startMetadata(),
+ ((BaseTransaction) conflictTx2).currentMetadata());
+
+ // The coalescing logic groups conflictCommit1 and conflictCommit2
together.
+ // The first rename succeeds, but the second fails requirement validation
+ // (schema ID changed). This should fail the entire transaction atomically.
+ assertThatThrownBy(
+ () -> restCatalog.commitTransaction(goodCommit, conflictCommit1,
conflictCommit2))
+ .isInstanceOf(CommitFailedException.class);
+
+ // Verify atomicity: neither table should have changed.
+ assertThat(catalog().loadTable(goodId).schema().asStruct())
+ .isEqualTo(originalGoodSchema.asStruct());
+ assertThat(catalog().loadTable(conflictId).schema().asStruct())
+ .isEqualTo(originalConflictSchema.asStruct());
+ }
+
+ @Test
+ public void
testMultipleNonConflictingUpdatesToSameTableWithSchemaAndProperties() {
+ Namespace namespace = Namespace.of("coalescingMixedNs");
+ TableIdentifier identifier = TableIdentifier.of(namespace,
"coalescingMixedTable");
+
+ if (requiresNamespaceCreate()) {
+ catalog().createNamespace(namespace);
+ }
+
+ // Use a single Iceberg transaction that performs both a schema change and
a property update.
+ // This produces a single TableCommit containing multiple metadata
updates, verifying
+ // that the server correctly handles multiple update types within a single
commit request.
+ Table table = catalog().buildTable(identifier, SCHEMA).create();
+ Transaction transaction = table.newTransaction();
+
+ UpdateSchema updateSchema =
+ transaction.updateSchema().addColumn("new_col", Types.LongType.get());
+ Schema expectedSchema = updateSchema.apply();
+ updateSchema.commit();
+
+ transaction.updateProperties().set("prop-key", "prop-val").commit();
Review Comment:
yes they are (atleast in java impl)
https://github.com/apache/iceberg/blob/main/core/src/main/java/org/apache/iceberg/rest/RESTSessionCatalog.java#L1320
The serializer and deserializer both respect the order :
https://github.com/apache/iceberg/blob/main/core/src/main/java/org/apache/iceberg/rest/requests/CommitTransactionRequestParser.java#L42
https://github.com/apache/iceberg/blob/main/core/src/main/java/org/apache/iceberg/rest/requests/CommitTransactionRequestParser.java#L62
an E2E tests for this
https://github.com/apache/iceberg/blob/main/core/src/test/java/org/apache/iceberg/rest/requests/TestCommitTransactionRequestParser.java#L162
--
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]
