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

feat: add extension for logical_plan_builder #12293

Closed
wants to merge 1 commit into from

Conversation

zhuliquan
Copy link
Contributor

Which issue does this PR close?

Closes #.

Rationale for this change

Support build extension for builder

What changes are included in this PR?

add function extension for LogicalPlanBuilder, which is aimed to build extension logical plan.

Are these changes tested?

yes, add test case plan_builder_from_extension

Are there any user-facing changes?

yes, user can build logical plan with extension node.

@github-actions github-actions bot added the logical-expr Logical plan and expressions label Sep 2, 2024
@jayzhan211
Copy link
Contributor

jayzhan211 commented Sep 2, 2024

Why do we need extension? Is there any example that shows we need this? I would be nice if there is end-to-end query in slt file

@zhuliquan
Copy link
Contributor Author

Why do we need extension? Is there any example that shows we need this? I would be nice if there is end-to-end query in slt file

I think building a logical plan from SQL is a high-level API behavior exposed to user, and we also need to build a logical plan from a low-level API like building blocks, just like Flink I can build a stream through Flink-SQL, and we can also build a flow through basic operators. Because SQL is completely limiting to how LogicalPlan is built. This allows us to build logicalPlans not only from SQL, but also from user-defined DSL. If there are features that are not sufficient with the existing LogicalPlan, we need to extend it. Meanwhile we need a extension func to build it

@edmondop
Copy link
Contributor

edmondop commented Sep 5, 2024

Why do we need extension? Is there any example that shows we need this? I would be nice if there is end-to-end query in slt file

I think building a logical plan from SQL is a high-level API behavior exposed to user, and we also need to build a logical plan from a low-level API like building blocks, just like Flink I can build a stream through Flink-SQL, and we can also build a flow through basic operators. Because SQL is completely limiting to how LogicalPlan is built. This allows us to build logicalPlans not only from SQL, but also from user-defined DSL. If there are features that are not sufficient with the existing LogicalPlan, we need to extend it. Meanwhile we need a extension func to build it

Following up on this, can we file an issue with this new feature @zhuliquan ? I think it will make easier to understand the context/scenario you have in mind

@alamb
Copy link
Contributor

alamb commented Sep 11, 2024

In general I think this PR is trying to make it easier to create LogicalPlan::Extension (aka user defined logical nodes)

I agree in general about the benefit of a ticket

What I would personally suggest is starting with a new example in datafusion-examples with a LogicalPlan::Extension node

perhaps we could show how to use that to implement some sort of non-relational operation. Maybe based on the following thing from influxdata:

https://github.com/influxdata/influxdb3_core/blob/1eaa4ed5ea147bc24db98d9686e457c124dfd5b7/iox_query/src/exec/schema_pivot.rs#L1-L20

@zhuliquan if you agree this is what you are trying to do, I would be happy to file the ticket to try and help this project along

@zhuliquan zhuliquan closed this Sep 27, 2024
@zhuliquan zhuliquan deleted the feature-extension_builder branch September 27, 2024 09:08
@alamb
Copy link
Contributor

alamb commented Sep 29, 2024

Thanks @zhuliquan -- I know adding/improving examples is a bigger as than just an API

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
logical-expr Logical plan and expressions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants