Skip to content

MINOR: [Go] Normalize zero-length C Data buffers with regression coverage - #5

Closed
wangting0128 wants to merge 2 commits into
milvus-io:v17.0.0from
wangting0128:wt/fix-arrow-cdata-zero-length-tests
Closed

MINOR: [Go] Normalize zero-length C Data buffers with regression coverage#5
wangting0128 wants to merge 2 commits into
milvus-io:v17.0.0from
wangting0128:wt/fix-arrow-cdata-zero-length-tests

Conversation

@wangting0128

Copy link
Copy Markdown

Summary

  • merge the zero-length C Data buffer normalization from fix(arrow/cdata): avoid unsafe.Slice on zero-length pointers #4
  • make the regression test fail deterministically before the fix by checking that zero-length Go buffers do not retain the C sentinel pointer 0x1
  • cover String, Binary, LargeString, and LargeBinary with all-null and all-empty values
  • cover nested List<String> imports, empty StringView buffers, and the C stream -> record batch reader path used by Milvus

This targets milvus-io/milvus#52233, where compaction crashed in cimporter.importVariableValuesBuffer after a zero-length values buffer with address 0x1 was retained on a Go stack.

Verification

go test -tags test ./arrow/cdata
go test ./arrow/cdata
go test -tags test -run 'Test(StrArrayAllNulls|StrArrayEmptyValues|ListStringArrayAllNullChildValues|EmptyStringViewArbitraryZeroLengthPointer|RecordReaderStreamAllNullStrings)$' -count=100 ./arrow/cdata
go test -tags test -gcflags=all=-d=checkptr=2 -run 'Test(StrArrayAllNulls|StrArrayEmptyValues|ListStringArrayAllNullChildValues|EmptyStringViewArbitraryZeroLengthPointer|RecordReaderStreamAllNullStrings)$' -count=10 ./arrow/cdata

Negative-control verification was also performed: reverting only the sz == 0 guard makes the new tests fail consistently because the imported zero-length slice still has data pointer 0x1.

orlp and others added 2 commits August 10, 2026 12:11
…pache#513)

Slices from FFI may have an arbitrary pointer when the length is zero,
but this is not allowed in Go, where the pointer must always be valid. I
believe this fixes apache/arrow-go#28.

I changed the instances of `unsafe.Slice` in `arrow/cdata` I could find
to be robust when used with length zero.

No, I don't have a Go setup at all.

No.

---------

Co-authored-by: Matt Topol <zotthewizard@gmail.com>
Signed-off-by: jiaqizho <jiaqi.zhou@zilliz.com>
Signed-off-by: wangting0128 <ting.wang@zilliz.com>
@github-actions

Copy link
Copy Markdown

Thanks for opening a pull request!

This pull request has been automatically converted to a draft because its title doesn't match Arrow's required format.

If this is not a minor PR. Could you open an issue for this pull request on GitHub? https://github.com/apache/arrow/issues/new/choose

Opening GitHub issues ahead of time contributes to the Openness of the Apache Arrow project.

Then could you also rename the pull request title in the following format?

GH-${GITHUB_ISSUE_ID}: [${COMPONENT}] ${SUMMARY}

or

MINOR: [${COMPONENT}] ${SUMMARY}

After updating the title, you can mark the pull request as ready for review.

See also:

@wangting0128 wangting0128 changed the title fix(arrow/cdata): normalize zero-length C buffers with regression coverage MINOR: [Go] Normalize zero-length C Data buffers with regression coverage Aug 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants