Skip to content

Conversation

@petern48
Copy link
Contributor

@petern48 petern48 commented Nov 1, 2025

Which issue does this PR close?

Rationale for this change

Convenience method for when parsing has already been done, and we want to start from a an expr object instead of SQL string.

What changes are included in this PR?

Are these changes tested?

Added test

Are there any user-facing changes?

Yes, new public api.

@github-actions github-actions bot added the core Core DataFusion crate label Nov 1, 2025
@petern48 petern48 force-pushed the create_logical_expr_from_sql_expr branch from c718319 to 20f53ab Compare November 1, 2025 05:57
@petern48 petern48 marked this pull request as ready for review November 1, 2025 15:05
Copy link
Contributor

@rkrishn7 rkrishn7 left a comment

Choose a reason for hiding this comment

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

Thanks @petern48! Looks good to me, although will need someone who can merge to sign off

let query = SqlToRel::new_with_options(&provider, state.get_parser_options());

for sql in ["[1,2,3]", "a > 10", "SUM(a)"] {
let sql_expr = state.sql_to_expr(sql, &dialect).unwrap();
Copy link
Member

Choose a reason for hiding this comment

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

The api name is strange to me first, I think it wants to say "sql string to expr"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, it can be a bit ambiguous. Either way, sql_to_expr is an existing public API before this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core Core DataFusion crate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add SessionState::create_logical_expr_from_sql_expr

3 participants