hcrosse commented on code in PR #2350:
URL: https://github.com/apache/iceberg-go/pull/2350#discussion_r4431438800
##########
catalog/hadoop/tables.go:
##########
@@ -208,23 +208,23 @@ func (c *Catalog) CommitTable(ctx context.Context, ident
table.Identifier, reqs
} else {
baseMeta, err = table.NewMetadata(iceberg.NewSchema(0), nil,
table.UnsortedSortOrder, "", nil)
if err != nil {
- return nil, "", fmt.Errorf("hadoop catalog: failed to
create base metadata: %w", err)
+ return nil, "", fmt.Errorf("%w: hadoop catalog: failed
to create base metadata: %w", table.ErrCommitRejected, err)
Review Comment:
Thanks, I routed all four Hadoop rejection paths through
`internal.RejectUnlessConflict`. Requirement errors now include `hadoop
catalog:`, including conflicts, which still match `table.ErrCommitFailed`. The
other three error messages are unchanged.
##########
catalog/internal/update_stage_test.go:
##########
@@ -92,6 +92,69 @@ func
TestUpdateAndStageTableRejectsEmptyMetadataLocationForExistingTable(t *test
require.Contains(t, err.Error(), "invalid metadata location")
}
+func TestUpdateAndStageTableClassifiesFailures(t *testing.T) {
Review Comment:
I added both `CommitTable` tests, thanks for pointing out the gap. Each
covers a rejected staging failure with no catalog write and an `UpdateTable` or
`AlterTable` failure after the metadata write that is neither rejected nor a
conflict.
##########
catalog/sql/sql_test.go:
##########
@@ -1732,6 +1732,43 @@ func (s *SqliteCatalogTestSuite)
TestStaleSchemaUpdateIsRefusedOnRecommit() {
s.Equal(peerMeta.MetadataLocation(), current.MetadataLocation(),
"nothing may be committed")
}
+func (s *SqliteCatalogTestSuite)
TestRejectedUpdateLeavesTransactionResumable() {
Review Comment:
I added a SQLite `BEFORE UPDATE` trigger test. The failure after the
metadata write is neither rejected nor a conflict, and a second `Commit` fails
with `transaction has already been committed`.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
