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 2021/04/26 19:31:47 UTC
[GitHub] [arrow] bkietz opened a new pull request #10166: ARROW-11929: [C++][Dataset][Compute] Promote expression to the compute namespace
bkietz opened a new pull request #10166:
URL: https://github.com/apache/arrow/pull/10166
Moves Expression and its test and benchmark into the compute/exec/ directory. I haven't introduced an exec namespace.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] bkietz commented on pull request #10166: ARROW-11929: [C++][Dataset][Compute] Promote expression to the compute namespace
Posted by GitBox <gi...@apache.org>.
bkietz commented on pull request #10166:
URL: https://github.com/apache/arrow/pull/10166#issuecomment-827786780
TODO: update bindings
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] lidavidm commented on a change in pull request #10166: ARROW-11929: [C++][Dataset][Compute] Promote expression to the compute namespace
Posted by GitBox <gi...@apache.org>.
lidavidm commented on a change in pull request #10166:
URL: https://github.com/apache/arrow/pull/10166#discussion_r620591026
##########
File path: cpp/examples/arrow/dataset_documentation_example.cc
##########
@@ -20,9 +20,9 @@
#include <arrow/api.h>
#include <arrow/compute/cast.h>
+#include <arrow/compute/exec/expression.h>
Review comment:
I'll follow up and adjust the line numbers in the corresponding reST file (and see if I can figure out a better way to excerpt code snippets than hardcoding line numbers).
##########
File path: cpp/src/arrow/dataset/file_parquet_test.cc
##########
@@ -130,6 +130,10 @@ class TestParquetFileFormat : public FileFormatFixtureMixin<ParquetFormatHelper>
return Batches(std::move(scan_task_it));
}
+ void SetFilter(compute::Expression filter) {
+ ASSERT_OK_AND_ASSIGN(opts_->filter, filter.Bind(*opts_->dataset_schema));
+ }
+
Review comment:
This is already in test_util.h.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #10166: ARROW-11929: [C++][Dataset][Compute] Promote expression to the compute namespace
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #10166:
URL: https://github.com/apache/arrow/pull/10166#issuecomment-827090334
https://issues.apache.org/jira/browse/ARROW-11929
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] ianmcook commented on pull request #10166: ARROW-11929: [C++][Dataset][Compute] Promote expression to the compute namespace
Posted by GitBox <gi...@apache.org>.
ianmcook commented on pull request #10166:
URL: https://github.com/apache/arrow/pull/10166#issuecomment-830094225
@bkietz This caused a failure in the `test-r-minimal-build` nightly test:
https://dev.azure.com/ursacomputing/crossbow/_build/results?buildId=4423&view=logs&j=0da5d1d9-276d-5173-c4c4-9d4d4ed14fdb&t=d9b15392-e4ce-5e4c-0c8c-b69645229181&l=952
Do you know off the top of your head how to resolve this? Thanks
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] lidavidm commented on pull request #10166: ARROW-11929: [C++][Dataset][Compute] Promote expression to the compute namespace
Posted by GitBox <gi...@apache.org>.
lidavidm commented on pull request #10166:
URL: https://github.com/apache/arrow/pull/10166#issuecomment-827098268
Ah, looks like clang-format/cmake-format are unhappy, and the CMake magic that makes the new header gets installed is missing.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] ianmcook edited a comment on pull request #10166: ARROW-11929: [C++][Dataset][Compute] Promote expression to the compute namespace
Posted by GitBox <gi...@apache.org>.
ianmcook edited a comment on pull request #10166:
URL: https://github.com/apache/arrow/pull/10166#issuecomment-830094225
@bkietz This caused a failure in the `test-r-minimal-build` nightly test (that's the test where we build the C++ library and the R package with Dataset and Parquet switched off):
https://dev.azure.com/ursacomputing/crossbow/_build/results?buildId=4423&view=logs&j=0da5d1d9-276d-5173-c4c4-9d4d4ed14fdb&t=d9b15392-e4ce-5e4c-0c8c-b69645229181&l=952
Do you know off the top of your head how to resolve this? Thanks
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] nealrichardson commented on pull request #10166: ARROW-11929: [C++][Dataset][Compute] Promote expression to the compute namespace
Posted by GitBox <gi...@apache.org>.
nealrichardson commented on pull request #10166:
URL: https://github.com/apache/arrow/pull/10166#issuecomment-827733626
This just moves the code? There's no change in how or where expressions are evaluated yet, correct? I.e. they're still only for datasets here, they're just in a different namespace?
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] lidavidm commented on pull request #10166: ARROW-11929: [C++][Dataset][Compute] Promote expression to the compute namespace
Posted by GitBox <gi...@apache.org>.
lidavidm commented on pull request #10166:
URL: https://github.com/apache/arrow/pull/10166#issuecomment-830095028
@ianmcook I think we just need to add Expression to compute/type_fwd.h. I'll do this now.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] bkietz commented on a change in pull request #10166: ARROW-11929: [C++][Dataset][Compute] Promote expression to the compute namespace
Posted by GitBox <gi...@apache.org>.
bkietz commented on a change in pull request #10166:
URL: https://github.com/apache/arrow/pull/10166#discussion_r623400197
##########
File path: cpp/examples/arrow/dataset_documentation_example.cc
##########
@@ -20,9 +20,9 @@
#include <arrow/api.h>
#include <arrow/compute/cast.h>
+#include <arrow/compute/exec/expression.h>
Review comment:
https://issues.apache.org/jira/browse/ARROW-12605
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] lidavidm closed pull request #10166: ARROW-11929: [C++][Dataset][Compute] Promote expression to the compute namespace
Posted by GitBox <gi...@apache.org>.
lidavidm closed pull request #10166:
URL: https://github.com/apache/arrow/pull/10166
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org