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

Documented SimpleExtensionType and simplified ExtensionSet #1

Merged

Conversation

westonpace
Copy link

No description provided.

…s explaining the behavior and hid more of the underlying implementation which will help future-proof us in case we move to a map-based version in the future. Fixed a bug in execution plan creation where we were referencing the Substrait function name instead of the Arrow function name.
@github-actions
Copy link

github-actions bot commented Feb 9, 2022

Thanks for opening a pull request!

If this is not a minor PR. Could you open an issue for this pull request on JIRA? https://issues.apache.org/jira/browse/ARROW

Opening JIRAs ahead of time contributes to the Openness of the Apache Arrow project.

Then could you also rename pull request title in the following format?

ARROW-${JIRA_ID}: [${COMPONENT}] ${SUMMARY}

or

MINOR: [${COMPONENT}] ${SUMMARY}

See also:

Copy link
Owner

@jvanstraten jvanstraten left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great; in my opinion this is much easier to understand than it was. Just a few minor things. I can also merge this in and then make the changes in my branch; let me know.

cpp/src/arrow/engine/substrait/extension_set.h Outdated Show resolved Hide resolved
cpp/src/arrow/engine/substrait/extension_set.h Outdated Show resolved Hide resolved
cpp/src/arrow/engine/substrait/extension_set.h Outdated Show resolved Hide resolved
@westonpace westonpace force-pushed the substrait-consumer branch 2 times, most recently from 50ae330 to 8bd00a2 Compare February 10, 2022 02:03
westonpace and others added 2 commits February 10, 2022 20:12
Co-authored-by: Jeroen van Straten <jeroen.van.straten@gmail.com>
@westonpace
Copy link
Author

I can also merge this in and then make the changes in my branch; let me know.

Either way works for me. So if you see future changes and want to just make them then go for it.

@jvanstraten jvanstraten merged commit cde0b6f into jvanstraten:substrait-consumer Feb 11, 2022
jvanstraten pushed a commit that referenced this pull request Jun 29, 2022
TODOs:
Convert cheat sheet to PDF and hide slide #1.

Closes apache#12445 from pachadotdev/patch-4

Lead-authored-by: Stephanie Hazlitt <stephhazlitt@gmail.com>
Co-authored-by: Pachá <mvargas@dcc.uchile.cl>
Co-authored-by: Mauricio Vargas <mavargas11@uc.cl>
Co-authored-by: Pachá <mavargas11@uc.cl>
Signed-off-by: Jonathan Keane <jkeane@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants