Skip to content

Conversation

@emkornfield
Copy link
Contributor

Based on discussion on the mailing list.

@emkornfield emkornfield changed the title [Proposal] Add a padding for flatbuffer alignment [Format][Proposal] Add a padding for flatbuffer alignment Jul 26, 2019
@codecov-io
Copy link

codecov-io commented Jul 26, 2019

Codecov Report

Merging #4951 into master will decrease coverage by 22.59%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4951      +/-   ##
==========================================
- Coverage   87.33%   64.74%   -22.6%     
==========================================
  Files         895      491     -404     
  Lines      119946    65572   -54374     
  Branches     1418        0    -1418     
==========================================
- Hits       104751    42452   -62299     
- Misses      14833    23120    +8287     
+ Partials      362        0     -362
Impacted Files Coverage Δ
cpp/src/arrow/util/memory.h 0% <0%> (-100%) ⬇️
cpp/src/gandiva/date_utils.h 0% <0%> (-100%) ⬇️
cpp/src/arrow/util/memory.cc 0% <0%> (-100%) ⬇️
cpp/src/arrow/filesystem/util-internal.cc 0% <0%> (-100%) ⬇️
cpp/src/arrow/util/sse-util.h 0% <0%> (-100%) ⬇️
cpp/src/gandiva/decimal_type_util.h 0% <0%> (-100%) ⬇️
cpp/src/arrow/compute/logical_type.h 0% <0%> (-100%) ⬇️
cpp/src/parquet/hasher.h 0% <0%> (-100%) ⬇️
cpp/src/gandiva/basic_decimal_scalar.h 0% <0%> (-100%) ⬇️
cpp/src/arrow/compute/kernels/boolean.cc 0% <0%> (-100%) ⬇️
... and 886 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 9cfa7b3...254eaa0. Read the comment docs.

Copy link
Member

@pitrou pitrou left a comment

Choose a reason for hiding this comment

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

Just a suggestion.

@@ -24,13 +24,16 @@ Encapsulated message format
Data components in the stream and file formats are represented as encapsulated
*messages* consisting of:

* A 32-bit continuation indicator (0xFFFFFFFF). This allows flatbuffer bytes
to begin on an 8-byte boundary. This was added as of release 0.15.0.
* A length prefix indicating the metadata size
Copy link
Member

Choose a reason for hiding this comment

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

Change this to "A 32-bit length prefix"?
(also, where is endianness documented?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed. Also clarified it is little endian.

@wesm
Copy link
Member

wesm commented Aug 6, 2019

I added some additional clarification. I'm going to start a vote on the mailing list

@wesm wesm changed the title [Format][Proposal] Add a padding for flatbuffer alignment ARROW-6329: [Format] Add a padding for Flatbuffer alignment, use 8-byte EOS Aug 23, 2019
@wesm wesm closed this in 3dc3161 Aug 23, 2019
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.

4 participants