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

[GLUTEN-6834][CORE] Remove unused DDL plan that doesn't correspond to Substrait spec #6833

Merged
merged 1 commit into from
Aug 15, 2024

Conversation

EpsilonPrime
Copy link
Contributor

What changes were proposed in this pull request?

This is part of a larger effort to unfork Substrait.

How was this patch tested?

Tested utilizing the available unit tests.

@github-actions github-actions bot added the CORE works for Gluten Core label Aug 14, 2024
Copy link

Thanks for opening a pull request!

Could you open an issue for this pull request on Github Issues?

https://github.com/apache/incubator-gluten/issues

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

[GLUTEN-${ISSUES_ID}][COMPONENT]feat/fix: ${detailed message}

See also:

Copy link

Run Gluten Clickhouse CI

@EpsilonPrime EpsilonPrime changed the title feat: remove unused ddplan that doesn't correspond to Substrait spec [GLUTEN-6834][core] feat: remove unused ddplan that doesn't correspond to Substrait spec Aug 14, 2024
Copy link

#6834

@EpsilonPrime
Copy link
Contributor Author

It's worth noting that DDL operations are connected differently in Substrait so that this code would need to be updated before it could be used anyway.

@zhztheplayer zhztheplayer changed the title [GLUTEN-6834][core] feat: remove unused ddplan that doesn't correspond to Substrait spec [GLUTEN-6834][CORE] feat: remove unused ddplan that doesn't correspond to Substrait spec Aug 15, 2024
@zhztheplayer
Copy link
Member

Hi @rui-mo, where did the code come from, do you remember?

@rui-mo
Copy link
Contributor

rui-mo commented Aug 15, 2024

This was introduced by CH backend with 614543e. cc: @zzcclp

Copy link
Member

@zhztheplayer zhztheplayer left a comment

Choose a reason for hiding this comment

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

+1 if the code is unused

@zhztheplayer
Copy link
Member

zhztheplayer commented Aug 15, 2024

@EpsilonPrime

By the way I still doubt we can unfork Substrait with minimal effort, though it sounds good. Different native accelerators / libraries would have different ways to implement the same logical computation. It's may not be easy to have a single protocol to unify them at physical level.

@zhztheplayer zhztheplayer changed the title [GLUTEN-6834][CORE] feat: remove unused ddplan that doesn't correspond to Substrait spec [GLUTEN-6834][CORE] Remove unused DDL plan that doesn't correspond to Substrait spec Aug 15, 2024
@zhztheplayer zhztheplayer merged commit 9fb9899 into apache:main Aug 15, 2024
46 checks passed
@EpsilonPrime EpsilonPrime deleted the remove_ddl branch August 15, 2024 05:25
sharkdtu pushed a commit to sharkdtu/gluten that referenced this pull request Nov 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CORE works for Gluten Core
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants