zeroshade commented on code in PR #1438:
URL: https://github.com/apache/iceberg-go/pull/1438#discussion_r3566743302
##########
catalog/internal/utils.go:
##########
@@ -202,15 +202,17 @@ func UpdateAndStageTable(ctx context.Context, catprops
iceberg.Properties, curre
)
if current != nil {
- for _, r := range reqs {
- if err := r.Validate(current.Metadata()); err != nil {
- return nil, err
- }
- }
-
baseMeta = current.Metadata()
metadataLoc = current.MetadataLocation()
- } else {
+ }
+
+ for _, r := range reqs {
+ if err := r.Validate(baseMeta); err != nil {
Review Comment:
This correctly validates every requirement against `baseMeta` even when the
table is missing (baseMeta nil) — 👍.
However, `catalog/hadoop/hadoop.go`'s `CommitTable` doesn't use
`UpdateAndStageTable`; it still validates requirements only inside `if current
!= nil` (around `hadoop.go:614`). So a commit carrying e.g. `AssertTableUUID`
against a missing table bypasses validation there and proceeds as a create —
the same bug this PR fixes elsewhere. Please apply the same ordering in the
Hadoop path (validate all reqs against `baseMeta`, which may be nil, before
creating empty base metadata) and add a Hadoop regression test for a non-create
requirement on a missing table.
--
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]
