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

[C++] rename ARROW_ENGINE to ARROW_SUBSTRAIT #31565

Closed
asfimport opened this issue Apr 8, 2022 · 5 comments
Closed

[C++] rename ARROW_ENGINE to ARROW_SUBSTRAIT #31565

asfimport opened this issue Apr 8, 2022 · 5 comments
Assignees
Labels
Milestone

Comments

@asfimport
Copy link

When we introduced substrait we reused the cmake + feature ARROW_ENGINE to mean compute+a few other things as well as the substrait consumer functionality. In general, right now, we don't yet need (or want) to build substrait in our packages (e.g. the R package) since many places don't yet take advantage of it. But the naming of the cmake or feature is now confusing: it effectively is only substrait if you separately enable compute, etc. but it makes it sound like the query engine we have been building since 6.0.0 is disabled.

We should rename ARROW_ENGINE to ARROW_SUBSTRAIT now and then we can add an ARROW_ENGINE later if we need to encompass a larger set of engine functionality (e.g. compute+spillover+scheduler+memory limits) if that's needed.

Reporter: Jonathan Keane / @jonkeane
Assignee: Weston Pace / @westonpace

PRs and other links:

Note: This issue was originally created as ARROW-16158. Please see the migration documentation for further details.

@asfimport
Copy link
Author

Jonathan Keane / @jonkeane:
I've made this a blocker for 8.0.0 not because the name is super super important, but rather because we have not yet released with this new meaning of ARROW_ENGINE so if we do this before the release there's no change anyone is depending on it (in a released pacakge)

@asfimport
Copy link
Author

Antoine Pitrou / @pitrou:
I'm a bit surprised about this request, because ARROW_ENGINE originally meant the execution engine. Perhaps we want a new separate ARROW_SUBSTRAIT build flag?

cc @westonpace  

@asfimport
Copy link
Author

Weston Pace / @westonpace:
Yes, we will eventually want both ARROW_ENGINE and ARROW_SUBSTRAIT it sounds like. We had originally named it ARROW_ENGINE thinking that more components would move into here over time. However, those components haven't moved yet. So currently, once you take away the Substrait components there is not much left. I think the idea is to rename to ARROW_SUBSTRAIT and then later introduce ARROW_ENGINE once we have more engine components.

@asfimport
Copy link
Author

Antoine Pitrou / @pitrou:
I agree with renaming to ARROW_SUBSTRAIT as it's a more explicit and informative name for the component anyway.

@asfimport
Copy link
Author

Krisztian Szucs / @kszucs:
Issue resolved by pull request 12915
#12915

kou pushed a commit that referenced this issue Aug 29, 2023
Documents the `ARROW_SUBSTRAIT` flag added in #31565 / #12915 and adds a missing seimcolon to the docs entry for `ARROW_WITH_RE2`.
* Closes: #37447

Authored-by: Ian Cook <ianmcook@gmail.com>
Signed-off-by: Sutou Kouhei <kou@clear-code.com>
loicalleyne pushed a commit to loicalleyne/arrow that referenced this issue 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 issue 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
Labels
Projects
None yet
Development

No branches or pull requests

2 participants