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-6079: [Java] Implement/test UnionFixedSizeListWriter for FixedSizeListVector #4973

Closed
wants to merge 2 commits into from

Conversation

tianchen92
Copy link
Contributor

Related to ARROW-6079.

Now we have two list vectors: ListVector and FixedSizeListVector.

ListVector has already implemented UnionListWriter for writing data, however, FixedSizeListVector doesn't have this yet and seems the only way for users to write data is getting inner vector and set value manually.

Implement a writer for FixedSizeListVector is useful in some cases.

@codecov-io
Copy link

codecov-io commented Jul 31, 2019

Codecov Report

Merging #4973 into master will increase coverage by 2.15%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4973      +/-   ##
==========================================
+ Coverage   87.55%   89.71%   +2.15%     
==========================================
  Files        1000      670     -330     
  Lines      142303    99543   -42760     
  Branches     1418        0    -1418     
==========================================
- Hits       124589    89301   -35288     
+ Misses      17352    10242    -7110     
+ Partials      362        0     -362
Impacted Files Coverage Δ
cpp/src/parquet/file_writer.h 81.81% <0%> (-18.19%) ⬇️
cpp/src/arrow/extension_type.h 85.71% <0%> (-14.29%) ⬇️
cpp/src/gandiva/function_registry_test.cc 87.23% <0%> (-12.77%) ⬇️
cpp/src/parquet/arrow/reader.h 80% <0%> (-11.67%) ⬇️
cpp/src/parquet/arrow/reader.cc 84.86% <0%> (-6.65%) ⬇️
cpp/src/arrow/array/builder_nested.h 97.61% <0%> (-2.39%) ⬇️
cpp/src/gandiva/precompiled/time.cc 94.92% <0%> (-2.28%) ⬇️
cpp/src/gandiva/precompiled/arithmetic_ops.cc 36.11% <0%> (-2.13%) ⬇️
cpp/src/arrow/array/builder_nested.cc 90.55% <0%> (-2.12%) ⬇️
python/pyarrow/tests/test_convert_builtin.py 97.24% <0%> (-2.03%) ⬇️
... and 456 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 803dd89...ad18d1c. Read the comment docs.

writeListVector(writer1, new int[] {7, 8, 9});
writer1.setValueCount(3);

assertEquals(3, vector1.getValueCount());
Copy link
Contributor

Choose a reason for hiding this comment

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

is there a way to avoid string comparison?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, fixed now.

@emkornfield
Copy link
Contributor

Seems mostly OK to me, not an expert on this area @praveenbingo do you have time to a take a look?

@praveenbingo
Copy link
Contributor

@tianchen92 - could you please re-trigger the tests, looks unrelated but just to be double sure.

@tianchen92 tianchen92 closed this Aug 9, 2019
@tianchen92 tianchen92 reopened this Aug 9, 2019
@tianchen92
Copy link
Contributor Author

@tianchen92 - could you please re-trigger the tests, looks unrelated but just to be double sure.

Sure, thank you. I also think it's unrelated since I only change the tests.

@tianchen92
Copy link
Contributor Author

@praveenbingo build passed :)

@emkornfield
Copy link
Contributor

+1, merging. Thank you @tianchen92

pprudhvi pushed a commit to pprudhvi/arrow that referenced this pull request Aug 11, 2019
…izeListVector

Related to [ARROW-6079](https://issues.apache.org/jira/browse/ARROW-6079).

Now we have two list vectors: ListVector and FixedSizeListVector.

ListVector has already implemented UnionListWriter for writing data, however, FixedSizeListVector doesn't have this yet and seems the only way for users to write data is getting inner vector and set value manually.

Implement a writer for FixedSizeListVector is useful in some cases.

Closes apache#4973 from tianchen92/ARROW-6079 and squashes the following commits:

ad18d1c <tianchen> fix test to avoid string comparision
d3338c7 <tianchen> ARROW-6079:  Implement/test UnionFixedSizeListWriter for FixedSizeListVector

Authored-by: tianchen <niki.lj@alibaba-inc.com>
Signed-off-by: Micah Kornfield <emkornfield@gmail.com>
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