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

Add PyExpr to_variant conversions #793

Merged
merged 13 commits into from
Aug 4, 2024

Conversation

Michael-J-Ward
Copy link
Contributor

@Michael-J-Ward Michael-J-Ward commented Jul 30, 2024

Which issue does this PR close?

Closes #781.

Rationale for this change

The user bug report filed

When a SQL query contains a InList Expr, I can't get the InList object through Expr.to_variant().

In fixing this bug, I found that upstream datafusion has added explicit structs for many of the variants, so I also updated the datafusion-python variants for those.

Lastly, datafusion-python was incorrectly returning the logical_plan::Sort instead of the expr::Sort as the variant for the Expr::Sort.

Changes

These Py-Variants now wrap the upstream variant structs.

  • PyAlias now wraps datafusion_expr::expr::Alias
  • PyExists now wraps datafusion_expr::expr::Exists
  • PyInList now wraps datafusion_expr::expr::PyInList
  • PyInSubquery now wraps datafusion_expr::expr::InSubquery
  • PyPlaceholder now wraps datafusion_expr::expr::Placeholder

Disambiguating datafusion_expr::logical_plan::* and datafusion_expr::expr::* variants

PyExpr::to_variant was incorrectly returning variants datafusion_expr::logical_plan::Sort and datafusion_expr::logical_plan::Unnest.

NOTE I chose PySortExpr because that is how datafusion_expr re-exports expr::Sort as SortExpr

PyExpr::to_variant additionally implemented for

  • SimilarTo
  • Between
  • Case
  • Cast
  • TryCast
  • InList
  • Exists
  • InSubquery
  • ScalarSubquery
  • GroupingSet
  • Placeholder

@Michael-J-Ward Michael-J-Ward marked this pull request as ready for review July 31, 2024 06:54
@Michael-J-Ward Michael-J-Ward changed the title WIP: Add PyExpr to_variant conversions Add PyExpr to_variant conversions Jul 31, 2024
Copy link
Member

@andygrove andygrove left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks @Michael-J-Ward

@andygrove andygrove merged commit 3eb198b into apache:main Aug 4, 2024
14 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.

Cannot convert Expr to InList
2 participants