Skip to content

fix(dv): reject negative deletion-vector positions in DVWriter.Add#1364

Merged
zeroshade merged 1 commit into
apache:mainfrom
fallintoplace:fix/dvwriter-negative-positions
Jul 4, 2026
Merged

fix(dv): reject negative deletion-vector positions in DVWriter.Add#1364
zeroshade merged 1 commit into
apache:mainfrom
fallintoplace:fix/dvwriter-negative-positions

Conversation

@fallintoplace

Copy link
Copy Markdown
Contributor

Summary

  • Validate DVWriter.Add positions are non-negative.
  • Change DVWriter.Add to return an error on negative positions.
  • Plumb error handling where DVWriter.Add is called.
  • Add regression coverage for negative position rejection in dv_writer tests.

Testing

  • go test ./table/dv -run 'TestDVWriter'
  • go test ./table -run 'TestScanPruningWithDeletionVector|TestReadTaskDeletionVectorSupersedesPositionalDeletes|TestRewriteDataFilesPreservesSiblingDeletionVector'
  • go test ./table ./table/dv

@fallintoplace fallintoplace requested a review from zeroshade as a code owner July 3, 2026 23:16
@fallintoplace fallintoplace force-pushed the fix/dvwriter-negative-positions branch from f563da8 to aa004fd Compare July 3, 2026 23:19
@fallintoplace fallintoplace changed the title Reject negative deletion-vector positions in DVWriter.Add fix(dv): reject negative deletion-vector positions in DVWriter.Add Jul 3, 2026
@fallintoplace fallintoplace force-pushed the fix/dvwriter-negative-positions branch from aa004fd to 7feef1a Compare July 3, 2026 23:23

@zeroshade zeroshade left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Rejecting negative positions at the DVWriter.Add entry point before mutating writer state is correct, the production caller now propagates the error, and the negative/zero cases are well covered.

@zeroshade zeroshade merged commit d5e1770 into apache:main Jul 4, 2026
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants