GH-34056: [C++] Add Utility function to simplify converting any row-based structure into an `arrow::RecordBatchReader` or an `arrow::Table` #34057

gringasalpastor · 2023-02-06T18:27:11Z

Are these changes tested?

The following tests are provided:

basic usage
const ranges
custom struct accessor
usage with std::variant

Closes: [C++] Add Utility function to simplify converting any row-based structure into an arrow::RecordBatchReader or an arrow::Table #34056

github-actions · 2023-02-06T18:27:33Z

Closes: [C++] Add Utility function to simplify converting any row-based structure into an arrow::RecordBatchReader or an arrow::Table #34056

github-actions · 2023-02-06T18:27:36Z

⚠️ GitHub issue #34056 has been automatically assigned in GitHub to PR creator.

gringasalpastor · 2023-02-06T18:46:25Z

@wjones127 If you could take a look, thanks.

wjones127

Thanks for submitting this! It would be nice to have at least one example with nested row data.

I'd suggest either adding another test, or to be more complete it would be interesting to see how the rapidjson example would be changed to use this.

cpp/src/arrow/util/rows_to_batches.h

Co-authored-by: Will Jones <willjones127@gmail.com>

…nce_wrapper

cpp/src/arrow/util/rows_to_batches_test.cc

wjones127

Just two minor changes, then I think this is ready to go.

cpp/src/arrow/util/rows_to_batches.h

cpp/src/arrow/util/rows_to_batches_test.cc

Co-authored-by: Will Jones <willjones127@gmail.com>

gringasalpastor · 2023-02-17T18:49:49Z

Just two minor changes, then I think this is ready to go.

Committed the last 2 suggested changes, let me know if there is anything else you would like to see updated @wjones127

wjones127 · 2023-02-17T23:04:08Z

@gringasalpastor There seems to be one CI failure that is legitimate. Could you address that?

/arrow/cpp/src/arrow/util/rows_to_batches.h:107: error: The following parameters of arrow::util::RowsToBatches(const std::shared_ptr< Schema > &schema, const Range &rows, DataPointConvertor &&data_point_convertor, RowAccessor &&row_accessor=detail::MakeDefaultRowAccessor(), MemoryPool *pool=default_memory_pool(), const std::size_t batch_size=1024) are not documented:
  parameter 'pool'
  parameter 'batch_size' (warning treated as error, aborting now)
1

gringasalpastor · 2023-02-21T16:11:33Z

@gringasalpastor There seems to be one CI failure that is legitimate. Could you address that?

/arrow/cpp/src/arrow/util/rows_to_batches.h:107: error: The following parameters of arrow::util::RowsToBatches(const std::shared_ptr< Schema > &schema, const Range &rows, DataPointConvertor &&data_point_convertor, RowAccessor &&row_accessor=detail::MakeDefaultRowAccessor(), MemoryPool *pool=default_memory_pool(), const std::size_t batch_size=1024) are not documented:
  parameter 'pool'
  parameter 'batch_size' (warning treated as error, aborting now)
1

@wjones127 Thanks, I just pushed a new commit to add documentation for these.

wjones127

Thanks @gringasalpastor!

ursabot · 2023-02-22T08:10:36Z

Benchmark runs are scheduled for baseline = 92d91f5 and contender = 7828165. 7828165 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Finished ⬇️0.0% ⬆️0.0%] ec2-t3-xlarge-us-east-2
[Failed ⬇️0.79% ⬆️0.0%] test-mac-arm
[Finished ⬇️0.0% ⬆️0.0%] ursa-i9-9960x
[Finished ⬇️0.83% ⬆️0.06%] ursa-thinkcentre-m75q
Buildkite builds:
[Finished] 7828165f ec2-t3-xlarge-us-east-2
[Failed] 7828165f test-mac-arm
[Finished] 7828165f ursa-i9-9960x
[Finished] 7828165f ursa-thinkcentre-m75q
[Finished] 92d91f53 ec2-t3-xlarge-us-east-2
[Finished] 92d91f53 test-mac-arm
[Finished] 92d91f53 ursa-i9-9960x
[Finished] 92d91f53 ursa-thinkcentre-m75q
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python, R. Runs only benchmarks with cloud = True
test-mac-arm: Supported benchmark langs: C++, Python, R
ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java

… row-based structure into an `arrow::RecordBatchReader` or an `arrow::Table` (apache#34057) *Are these changes tested?* The following tests are provided: - basic usage - const ranges - custom struct accessor - usage with `std::variant` * Closes: apache#34056 Lead-authored-by: Mike Hancock <mhancock34@bloomberg.net> Co-authored-by: Michael Hancock <javaiscoolmike@gmail.com> Co-authored-by: Will Jones <willjones127@gmail.com> Signed-off-by: Will Jones <willjones127@gmail.com>

Mike Hancock added 6 commits January 25, 2023 15:11

Initial code for rows_to_batches utility function

e5497e4

Update rows_to_batches to follow style guide

1b85fbb

Updates rows_to_batches to follow style guide

e213096

Updates rows_to_batches to follow style guide

0ac4216

Add more unit tests for rows_to_batches

f96cc4e

Use ChunkedArrayFromJSON() in rows_to_batches unit tests.

6bf33d2

github-actions bot added the Component: C++ label Feb 6, 2023

gringasalpastor closed this Feb 6, 2023

gringasalpastor reopened this Feb 6, 2023

wjones127 self-requested a review February 6, 2023 19:08

wjones127 requested changes Feb 6, 2023

View reviewed changes

gringasalpastor and others added 9 commits February 9, 2023 10:29

Update cpp/src/arrow/util/rows_to_batches.h

d2a650f

Co-authored-by: Will Jones <willjones127@gmail.com>

Update cpp/src/arrow/util/rows_to_batches.h

c2925ab

Co-authored-by: Will Jones <willjones127@gmail.com>

make batch_size an arg with default value

5c5a22f

Update cpp/src/arrow/util/rows_to_batches.h

3461b0b

Co-authored-by: Will Jones <willjones127@gmail.com>

Update cpp/src/arrow/util/rows_to_batches.h

91ebfdc

Co-authored-by: Will Jones <willjones127@gmail.com>

Update cpp/src/arrow/util/rows_to_batches.h

ba9e735

Co-authored-by: Will Jones <willjones127@gmail.com>

fix typo

ac7a079

Use instead of

cb53bd5

Support row accessors that return by value in addition to std::refere…

33a5787

…nce_wrapper

wjones127 reviewed Feb 16, 2023

View reviewed changes

cpp/src/arrow/util/rows_to_batches_test.cc Outdated Show resolved Hide resolved

Update cpp/src/arrow/util/rows_to_batches_test.cc

0ca631a

wjones127 self-requested a review February 17, 2023 17:16

wjones127 requested changes Feb 17, 2023

View reviewed changes

cpp/src/arrow/util/rows_to_batches.h Outdated Show resolved Hide resolved

cpp/src/arrow/util/rows_to_batches_test.cc Outdated Show resolved Hide resolved

gringasalpastor and others added 3 commits February 17, 2023 13:09

Update cpp/src/arrow/util/rows_to_batches.h

ab402c5

Co-authored-by: Will Jones <willjones127@gmail.com>

Update cpp/src/arrow/util/rows_to_batches_test.cc

6f0c006

Co-authored-by: Will Jones <willjones127@gmail.com>

Merge branch 'apache:main' into rows_to_batches

40611d8

github-actions bot removed Component: Python Component: JavaScript Component: R Component: MATLAB Component: Go Component: C++ - Gandiva Component: Ruby Component: C# Component: Parquet Component: Java labels Feb 17, 2023

trxcllnt removed their request for review February 17, 2023 19:23

westonpace removed their request for review February 17, 2023 20:16

Add documentation for pool and batch_size

cf615b7

gringasalpastor requested review from wjones127 and removed request for assignUser, kou, raulcd, paleolimbot, thisisnic, AlenkaF, domoritz, lidavidm and zeroshade February 21, 2023 19:12

wjones127 approved these changes Feb 21, 2023

View reviewed changes

wjones127 merged commit 7828165 into apache:main Feb 21, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

GH-34056: [C++] Add Utility function to simplify converting any row-based structure into an `arrow::RecordBatchReader` or an `arrow::Table` #34057

GH-34056: [C++] Add Utility function to simplify converting any row-based structure into an `arrow::RecordBatchReader` or an `arrow::Table` #34057

gringasalpastor commented Feb 6, 2023 •

edited by github-actions bot

Loading

github-actions bot commented Feb 6, 2023

github-actions bot commented Feb 6, 2023

gringasalpastor commented Feb 6, 2023

wjones127 left a comment

wjones127 left a comment

gringasalpastor commented Feb 17, 2023

wjones127 commented Feb 17, 2023

gringasalpastor commented Feb 21, 2023

wjones127 left a comment

ursabot commented Feb 22, 2023

GH-34056: [C++] Add Utility function to simplify converting any row-based structure into an arrow::RecordBatchReader or an arrow::Table #34057

GH-34056: [C++] Add Utility function to simplify converting any row-based structure into an arrow::RecordBatchReader or an arrow::Table #34057

Conversation

gringasalpastor commented Feb 6, 2023 • edited by github-actions bot Loading

github-actions bot commented Feb 6, 2023

github-actions bot commented Feb 6, 2023

gringasalpastor commented Feb 6, 2023

wjones127 left a comment

Choose a reason for hiding this comment

wjones127 left a comment

Choose a reason for hiding this comment

gringasalpastor commented Feb 17, 2023

wjones127 commented Feb 17, 2023

gringasalpastor commented Feb 21, 2023

wjones127 left a comment

Choose a reason for hiding this comment

ursabot commented Feb 22, 2023

GH-34056: [C++] Add Utility function to simplify converting any row-based structure into an `arrow::RecordBatchReader` or an `arrow::Table` #34057

GH-34056: [C++] Add Utility function to simplify converting any row-based structure into an `arrow::RecordBatchReader` or an `arrow::Table` #34057

gringasalpastor commented Feb 6, 2023 •

edited by github-actions bot

Loading