Skip to content

Conversation

@chenkovsky
Copy link
Contributor

@chenkovsky chenkovsky commented Mar 26, 2025

Which issue does this PR close?

Rationale for this change

Some LogicalPlans don't support to_variant now.

we are using datafusion in parsing sql into logical plan, analysis and do some transformation on logical plan, then unparsing the logical plan into sql. But I found that it will throw error when the logical plan is not supported. so I added the missing to_variant support.

What changes are included in this PR?

add python bindings for these classes.

Are there any user-facing changes?

No

@timsaucer
Copy link
Member

timsaucer commented Mar 30, 2025

This is a really big PR and it's not immediately obvious to me what problem it's trying to solve. Can you expand the description or at least link it to an issue describing the problem?

@chenkovsky
Copy link
Contributor Author

This is a really big PR and it's not immediately obvious to me what problem it's trying to solve. Can you expand the description or at least link it to an issue describing the problem?

@timsaucer
I updated the description. we are analysing logical plan with datafusion. that's why we need complete to_variant support.

@timsaucer
Copy link
Member

I'm sorry it took me so long to get around to reviewing this. Thank you for the contribution!

@timsaucer timsaucer mentioned this pull request Apr 27, 2025
4 tasks
@timsaucer timsaucer merged commit 15b96c4 into apache:main May 5, 2025
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants