Skip to content

fix(array): compare sparse union slices at relative offsets - #1096

Merged
zeroshade merged 1 commit into
apache:mainfrom
fallintoplace:fix/sparse-union-approx-slice-offset
Aug 6, 2026
Merged

fix(array): compare sparse union slices at relative offsets#1096
zeroshade merged 1 commit into
apache:mainfrom
fallintoplace:fix/sparse-union-approx-slice-offset

Conversation

@fallintoplace

@fallintoplace fallintoplace commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Rationale for this change

SparseUnion.setData already slices each child using the parent union offset. ApproxEqual adds that offset again, so sliced unions can compare the wrong child values or hit invalid slice bounds.

What changes are included in this PR?

Compare each selected child at its relative i:i+1 position, matching exact equality and the representation used by sparse unions.

Are these changes tested?

  • go test ./arrow/array -run TestSparseUnionApproxEqualSlicesWithDifferentOffsets -count=1

Are there any user-facing changes?

No API changes. This corrects the reported behavior while preserving the existing ownership and compatibility contracts.

@fallintoplace fallintoplace changed the title fix(array): compare sliced sparse union children correctly fix(array): compare sliced sparse union children at relative offsets Aug 5, 2026
@fallintoplace fallintoplace changed the title fix(array): compare sliced sparse union children at relative offsets fix(array): compare sparse union slices at relative offsets Aug 6, 2026

@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. The root cause is confirmed: union.setData already slices each sparse-union child by the parent offset (union.go L137-143), and RawTypeCodes() returns typecodes[offset:], so children and type codes are both already offset-adjusted. Adding data.offset again in arraySparseUnionApproxEqual double-counted the offset. The fix brings the approx-equal path in line with the already-correct exact-equality path (arraySparseUnionEqual, L454), and dense unions are correctly left untouched.

Verified locally:

  • New test passes; full ./arrow/array -run Union suite passes.
  • Regression proof: reverting only the union.go change (keeping the test) makes the test fail, confirming it genuinely catches the bug.
  • The test uses distinct offsets on each side over different underlying values, so it faithfully reproduces the reported issue.

@zeroshade
zeroshade merged commit 588ca34 into apache:main Aug 6, 2026
23 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