ARROW-16158: [C++][R] Rename ARROW_ENGINE to ARROW_SUBSTRAIT #12915
+216
−190
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
|
Member
nealrichardson
left a comment
There was a problem hiding this comment.
R changes make sense to me, just one comment on the cmake
cpp/CMakeLists.txt
Outdated
Member
There was a problem hiding this comment.
Does the substrait integration actually require all of these things?
Member
Author
There was a problem hiding this comment.
Yes. In the future we could presumably add conditional-feature flags. For example, if a Substrait file says the format is Parquet and the consumer was built without Parquet then reject the plan as invalid. I created ARROW-16274 to track this.
kou
reviewed
Apr 21, 2022
r/NAMESPACE
Outdated
Member
There was a problem hiding this comment.
Could you keep this list in alphabetical order?
Member
|
@github-actions crossbow submit -g nightly |
|
Revision: ec22d466b469202d27df76f5445809b77e978174 Submitted crossbow builds: ursacomputing/crossbow @ actions-1897
|
kou
approved these changes
Apr 23, 2022
Member
kou
left a comment
There was a problem hiding this comment.
+1
(I think that https://github.com/apache/arrow/pull/12915/files/ec22d466b469202d27df76f5445809b77e978174#diff-bae2e9b731f293cf61d77c971d4152d31b9d49b0354817931e5a51c6c27ecf19 should be fixed before we merge this.)
212c4bd to
17031e3
Compare
Member
|
I fixed the R namespace and rebased to resolve conflicts, this should be good to merge once CI finishes |
…e will have a standalone engine module but there is not enough to justify one in this release.
…lity since engine is not published yet. In the future we can move it back as visibility.h is not very public and no one should be using it anyways.
17031e3 to
3683962
Compare
Member
|
@westonpace I don't know why but C++ on s390x test on Travis CI is failed since this commit: https://app.travis-ci.com/github/apache/arrow/builds Could you confirm it? https://app.travis-ci.com/github/apache/arrow/jobs/568028126 |
Member
|
Benchmark runs are scheduled for baseline = 0b06870 and contender = d21f8b1. d21f8b1 is a master commit associated with this PR. Results will be available as each benchmark for each run completes. |
This was referenced Aug 29, 2023
loicalleyne
pushed a commit
to loicalleyne/arrow
that referenced
this pull request
Nov 13, 2023
…ache#37451) Documents the `ARROW_SUBSTRAIT` flag added in apache#31565 / apache#12915 and adds a missing seimcolon to the docs entry for `ARROW_WITH_RE2`. * Closes: apache#37447 Authored-by: Ian Cook <ianmcook@gmail.com> Signed-off-by: Sutou Kouhei <kou@clear-code.com>
dgreiss
pushed a commit
to dgreiss/arrow
that referenced
this pull request
Feb 19, 2024
…ache#37451) Documents the `ARROW_SUBSTRAIT` flag added in apache#31565 / apache#12915 and adds a missing seimcolon to the docs entry for `ARROW_WITH_RE2`. * Closes: apache#37447 Authored-by: Ian Cook <ianmcook@gmail.com> Signed-off-by: Sutou Kouhei <kou@clear-code.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
At the moment there is not enough non-Substrait functionality to justify a dedicated engine module. In the future there likely will be and so I have some of the general structure around in the C++ (e.g.
ARROW_ENGINE_EXPORT) but removed any external facing references.