You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2022/10/07 19:35:27 UTC
[GitHub] [arrow] rtpsw opened a new pull request, #14348: ARROW-17965: [C++] ExecBatch support for ChunkedArray values
rtpsw opened a new pull request, #14348:
URL: https://github.com/apache/arrow/pull/14348
See https://issues.apache.org/jira/browse/ARROW-17965
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] ursabot commented on pull request #14348: ARROW-17965: [C++] ExecBatch support for ChunkedArray values
Posted by GitBox <gi...@apache.org>.
ursabot commented on PR #14348:
URL: https://github.com/apache/arrow/pull/14348#issuecomment-1281397930
Benchmark runs are scheduled for baseline = 289e0c92d7a14c279b15a22fca5ce4839b404116 and contender = d809c285080e9c1b18a66f57ba483e88ed7eb4ee. d809c285080e9c1b18a66f57ba483e88ed7eb4ee is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Finished :arrow_down:0.0% :arrow_up:0.0%] [ec2-t3-xlarge-us-east-2](https://conbench.ursa.dev/compare/runs/5e69ed462eab4f8b8f978934d49d5d02...437ff3c9499c4b7da082b0bd2f299c1c/)
[Failed :arrow_down:0.0% :arrow_up:0.0%] [test-mac-arm](https://conbench.ursa.dev/compare/runs/ed96a88c0d85410298f9eb410a558321...9af99c0a14774d6a92dad06aa2d01dbe/)
[Finished :arrow_down:0.0% :arrow_up:0.0%] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/787a8574c420464aaf14b58941658ef0...f4b3e4427d6f4362ab6c457949dc1f3d/)
[Finished :arrow_down:0.96% :arrow_up:0.04%] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/1a3bd980efc843ccb8b2406812344e50...6eecfc18378244bfb07ad077b4f51215/)
Buildkite builds:
[Finished] [`d809c285` ec2-t3-xlarge-us-east-2](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ec2-t3-xlarge-us-east-2/builds/1721)
[Failed] [`d809c285` test-mac-arm](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-test-mac-arm/builds/1740)
[Finished] [`d809c285` ursa-i9-9960x](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-i9-9960x/builds/1723)
[Finished] [`d809c285` ursa-thinkcentre-m75q](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-thinkcentre-m75q/builds/1734)
[Finished] [`289e0c92` ec2-t3-xlarge-us-east-2](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ec2-t3-xlarge-us-east-2/builds/1720)
[Failed] [`289e0c92` test-mac-arm](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-test-mac-arm/builds/1739)
[Finished] [`289e0c92` ursa-i9-9960x](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-i9-9960x/builds/1722)
[Finished] [`289e0c92` ursa-thinkcentre-m75q](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-thinkcentre-m75q/builds/1733)
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
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] lidavidm commented on pull request #14348: ARROW-17965: [C++] ExecBatch support for ChunkedArray values
Posted by GitBox <gi...@apache.org>.
lidavidm commented on PR #14348:
URL: https://github.com/apache/arrow/pull/14348#issuecomment-1279106199
Oh sure, as long as the conflict is solved
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] kou commented on a diff in pull request #14348: ARROW-17965: [C++] ExecBatch support for ChunkedArray values
Posted by GitBox <gi...@apache.org>.
kou commented on code in PR #14348:
URL: https://github.com/apache/arrow/pull/14348#discussion_r990515005
##########
cpp/src/arrow/compute/exec.cc:
##########
@@ -90,14 +90,29 @@ void PrintTo(const ExecBatch& batch, std::ostream* os) {
if (value.is_scalar()) {
*os << "Scalar[" << value.scalar()->ToString() << "]\n";
continue;
+ } else if (value.is_array() || value.is_chunked_array()) {
+ PrettyPrintOptions options;
+ options.skip_new_lines = true;
+ if (value.is_array()) {
+ auto array = value.make_array();
+ *os << "Array";
+ ARROW_CHECK_OK(PrettyPrint(*array, options, os));
+ } else {
+ auto array = value.chunked_array();
+ *os << "Chunked Array";
+ ARROW_CHECK_OK(PrettyPrint(*array, options, os));
+ }
+ *os << "\n";
+ } else if (value.is_chunked_array()) {
Review Comment:
Duplicated condition?
##########
cpp/src/arrow/compute/exec.cc:
##########
@@ -118,8 +133,15 @@ std::string ExecBatch::ToString() const {
ExecBatch ExecBatch::Slice(int64_t offset, int64_t length) const {
ExecBatch out = *this;
for (auto& value : out.values) {
- if (value.is_scalar()) continue;
- value = value.array()->Slice(offset, length);
+ if (value.is_scalar()) {
+ continue;
Review Comment:
Should we replace this with `// Do nothing` or something?
##########
cpp/src/arrow/compute/exec.cc:
##########
@@ -90,14 +90,29 @@ void PrintTo(const ExecBatch& batch, std::ostream* os) {
if (value.is_scalar()) {
*os << "Scalar[" << value.scalar()->ToString() << "]\n";
continue;
Review Comment:
Should we remove this?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] rtpsw commented on pull request #14348: ARROW-17965: [C++] ExecBatch support for ChunkedArray values
Posted by GitBox <gi...@apache.org>.
rtpsw commented on PR #14348:
URL: https://github.com/apache/arrow/pull/14348#issuecomment-1279100478
@lidavidm, it would be easier for me to resolve-conflict directly. OK with you?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] kou commented on pull request #14348: ARROW-17965: [C++] ExecBatch support for ChunkedArray values
Posted by GitBox <gi...@apache.org>.
kou commented on PR #14348:
URL: https://github.com/apache/arrow/pull/14348#issuecomment-1278367075
@lidavidm What do you think this approach?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] rtpsw commented on a diff in pull request #14348: ARROW-17965: [C++] ExecBatch support for ChunkedArray values
Posted by GitBox <gi...@apache.org>.
rtpsw commented on code in PR #14348:
URL: https://github.com/apache/arrow/pull/14348#discussion_r990597511
##########
cpp/src/arrow/compute/exec.cc:
##########
@@ -90,14 +90,29 @@ void PrintTo(const ExecBatch& batch, std::ostream* os) {
if (value.is_scalar()) {
*os << "Scalar[" << value.scalar()->ToString() << "]\n";
continue;
+ } else if (value.is_array() || value.is_chunked_array()) {
+ PrettyPrintOptions options;
+ options.skip_new_lines = true;
+ if (value.is_array()) {
+ auto array = value.make_array();
+ *os << "Array";
+ ARROW_CHECK_OK(PrettyPrint(*array, options, os));
+ } else {
+ auto array = value.chunked_array();
+ *os << "Chunked Array";
+ ARROW_CHECK_OK(PrettyPrint(*array, options, os));
+ }
+ *os << "\n";
+ } else if (value.is_chunked_array()) {
Review Comment:
Yes, this is an oversight. Will remove.
##########
cpp/src/arrow/compute/exec.cc:
##########
@@ -90,14 +90,29 @@ void PrintTo(const ExecBatch& batch, std::ostream* os) {
if (value.is_scalar()) {
*os << "Scalar[" << value.scalar()->ToString() << "]\n";
continue;
Review Comment:
You mean remove the `continue` statement? I suppose we could.
##########
cpp/src/arrow/compute/exec.cc:
##########
@@ -118,8 +133,15 @@ std::string ExecBatch::ToString() const {
ExecBatch ExecBatch::Slice(int64_t offset, int64_t length) const {
ExecBatch out = *this;
for (auto& value : out.values) {
- if (value.is_scalar()) continue;
- value = value.array()->Slice(offset, length);
+ if (value.is_scalar()) {
+ continue;
Review Comment:
You mean remove the `continue` statement? I suppose we could.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] rtpsw commented on pull request #14348: ARROW-17965: [C++] ExecBatch support for ChunkedArray values
Posted by GitBox <gi...@apache.org>.
rtpsw commented on PR #14348:
URL: https://github.com/apache/arrow/pull/14348#issuecomment-1277710091
@kou, what remains for this PR to go through?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #14348: ARROW-17965: [C++] ExecBatch support for ChunkedArray values
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #14348:
URL: https://github.com/apache/arrow/pull/14348#issuecomment-1272023416
https://issues.apache.org/jira/browse/ARROW-17965
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] lidavidm merged pull request #14348: ARROW-17965: [C++] ExecBatch support for ChunkedArray values
Posted by GitBox <gi...@apache.org>.
lidavidm merged PR #14348:
URL: https://github.com/apache/arrow/pull/14348
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] ursabot commented on pull request #14348: ARROW-17965: [C++] ExecBatch support for ChunkedArray values
Posted by GitBox <gi...@apache.org>.
ursabot commented on PR #14348:
URL: https://github.com/apache/arrow/pull/14348#issuecomment-1281398236
['Python', 'R'] benchmarks have high level of regressions.
[test-mac-arm](https://conbench.ursa.dev/compare/runs/ed96a88c0d85410298f9eb410a558321...9af99c0a14774d6a92dad06aa2d01dbe/)
[ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/787a8574c420464aaf14b58941658ef0...f4b3e4427d6f4362ab6c457949dc1f3d/)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org