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

Convert Expr to a parsable representation #7165

Open
Blajda opened this issue Aug 1, 2023 · 6 comments
Open

Convert Expr to a parsable representation #7165

Blajda opened this issue Aug 1, 2023 · 6 comments
Labels
enhancement New feature or request

Comments

@Blajda
Copy link

Blajda commented Aug 1, 2023

Is your feature request related to a problem or challenge?

In the delta-rs project we support operations such as delete, update, and merge where users can supply predicate as either a string or a DataFusion Expr. String predicates go through sql-parser to obtain an Expr and are evaluated. At the end of each operation the expression must be converted back to a string to store in the transaction log for conflict resolution.

The implementations for create_name and canonical_name almost fit this need but scalar values are surrounded by their type which cannot be parsed by sql-parser.

E.G col1 = 1 becomes col1 = Int32(1)

Describe the solution you'd like

Given an Expr one should be able to obtain it's string representation that can be parsed by sql parser.

Describe alternatives you've considered

No response

Additional context

No response

@Blajda Blajda added the enhancement New feature or request label Aug 1, 2023
@parkma99
Copy link
Contributor

parkma99 commented Aug 1, 2023

I made a PR #6708 about printing Expr two weeks ago. Could you give more infomation? Thanks , and I will work it this night.

@Blajda
Copy link
Author

Blajda commented Aug 1, 2023

Thanks @parkma99

On delta-rs we this method to convert an sql expression to Expr.
We can almost convert it back to a sql string using canonical_name however it fails to parse using the above code.
This is because the method uses Debug format for scalar values which results in output like Int32(1). The parser thinks Int32(1) is a function.

I would like to have a function like canonical_name except it format scalar values as their display value.

@parkma99
Copy link
Contributor

parkma99 commented Aug 1, 2023

cc @alamb. Do we need a new function to Convert Expr to a parsable representation? Or we just change the canonical_name function. Thanks 😊

@alamb
Copy link
Contributor

alamb commented Aug 3, 2023

We could potentially propose changing canonical_name - however as I recall that function is used to create the column names so it might have a large change.

Another possibility, which is somewhat of a hack, might be to add a expr rewrite pass parsing stuff like c = Int32(1) that rewrites Function: int32(arg) --> ScalarValue::Int32

@parkma99
Copy link
Contributor

parkma99 commented Aug 4, 2023

Another possibility, which is somewhat of a hack, might be to add a expr rewrite pass parsing stuff like c = Int32(1) that rewrites Function: int32(arg) --> ScalarValue::Int32

just like does??
https://github.com/apache/arrow-datafusion/blob/02da0445f8c94019c1526d6b1759492fda266cdf/datafusion-examples/examples/rewrite_expr.rs#L163-L187

@alamb
Copy link
Contributor

alamb commented Aug 4, 2023

just like does??

That is a good example of using the rewriter ! Though the actual rewrite rule is a little different

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants