Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ARROW-5111: [Go] implement reading list arrays from Arrow file #4157

Closed
wants to merge 1 commit into from

Conversation

sbinet
Copy link
Contributor

@sbinet sbinet commented Apr 16, 2019

No description provided.

@sbinet
Copy link
Contributor Author

sbinet commented Apr 16, 2019

needs #4155

@codecov-io
Copy link

Codecov Report

Merging #4157 into master will decrease coverage by 27.23%.
The diff coverage is 0%.

Impacted file tree graph

@@             Coverage Diff             @@
##           master    #4157       +/-   ##
===========================================
- Coverage   87.89%   60.65%   -27.24%     
===========================================
  Files         758       67      -691     
  Lines       92163     4883    -87280     
  Branches     1251        0     -1251     
===========================================
- Hits        81006     2962    -78044     
+ Misses      11040     1812     -9228     
+ Partials      117      109        -8
Impacted Files Coverage Δ
go/arrow/ipc/file_reader.go 0% <0%> (ø) ⬆️
go/arrow/ipc/metadata.go 0% <0%> (ø) ⬆️
python/pyarrow/ipc.pxi
cpp/src/arrow/csv/chunker-test.cc
cpp/src/parquet/column_page.h
cpp/src/parquet/bloom_filter-test.cc
cpp/src/arrow/array/builder_decimal.cc
cpp/src/plasma/client.cc
cpp/src/arrow/io/test-common.h
cpp/src/arrow/util/int-util-test.cc
... and 683 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1c0930d...dda12fd. Read the comment docs.

@@ -409,6 +425,29 @@ func (ctx *arrayLoaderContext) loadPrimitive(dt arrow.DataType) array.Interface
return array.MakeFromData(data)
}

func (ctx *arrayLoaderContext) loadBinary(dt arrow.DataType) array.Interface {
field, buffers := ctx.loadCommon(3)
Copy link
Contributor

Choose a reason for hiding this comment

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

Hi @sbinet, I'm working on the equivalent for Rust. Hope you don't mind me asking a few questions.

Thanks

Copy link
Member

Choose a reason for hiding this comment

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

A binary array always has 3 buffers, but the first buffer may be null or empty if the null count is 0.

This means buffer #1 should always be the offsets array and buffer #2 the values array.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks @pitrou, is it safe for me to presume that the buffer numbers for primitives, list and struct will behave in the same way, where the null buffer may be null or empty?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, it's part of the Arrow format specification.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@sbinet
Copy link
Contributor Author

sbinet commented Apr 26, 2019

PTAL @stuartcarnie @alexandreyc @nickpoorman

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.

None yet

5 participants