shangxinli commented on PR #776: URL: https://github.com/apache/iceberg-cpp/pull/776#issuecomment-4784890387
Force-pushed again in response to a third codex pass and to address the test request. **Codex P0/P2 (new, post-redesign):** keying \`replaces_full_table_\` on a staged file's spec being unpartitioned was wrong for multi-spec tables (partition evolution could leave a historical unpartitioned spec on a now-partitioned table). Codex also flagged that \`Validate()\` with no files staged would fall through to a full-table \`AlwaysTrue\` conflict check. **Fix:** drop \`replaces_full_table_\` entirely. \`DropPartition(spec_id, partition)\` already does the right thing — for an unpartitioned spec the partition value is empty and the filter manager matches every file in that spec without needing a separate \`AlwaysTrue\` path. \`Validate()\` now skips when no partitions were staged and uses the partition-scoped \`PartitionSet\` overloads otherwise. Net result: simpler code, both new findings resolved, behavior matches Java's \`BaseReplacePartitions\` more closely. **Tests added (\`replace_partitions_test.cc\`, parameterized over format v2/v3 × main/feature branch — 20 cases + 2 fixtures, all passing locally):** - \`AddNullFile\` / \`AddFileWithoutSpecIdRejected\` — input validation - \`OperationIsOverwrite\` — operation marker - \`EmptyTableReplaceWritesOverwriteSnapshot\` — first commit on empty table, snapshot has \`operation=overwrite\` and \`replace-partitions=true\` - \`ReplaceOverwritesOnlyTouchedPartitions\` — **the key correctness test**: seed three files across two partitions, replace only partition 1, verify partition 2's file survives, partition 1's two files are marked deleted, new file is added (covers the original P0) - \`ValidateAppendOnlyFailsWhenPartitionHasData\` / \`ValidateAppendOnlyAllowsAddToEmptyPartition\` — append-only enforcement PR scope expanded slightly to wire \`Table::NewReplacePartitions()\` and \`Transaction::NewReplacePartitions()\` so the tests can drive the operation end-to-end — this collapses the original PR1/PR2 split into a single shippable change. -- 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]
