feat: plan-time SQL expression simplifying #19311

theirix · 2025-12-13T19:18:36Z

Which issue does this PR close?

Closes feat: plan-time SQL expression simplifying #19312.

Rationale for this change

Introduce a module for parsing and simplification of an SQL expression to a literal of a given type.

This module provides functionality to parse and simplify static SQL expressions
used in SQL constructs like FROM TABLE SAMPLE (10 + 50 * 2). If they are required
in a planning (not an execution) phase, they need to be reduced to literals of a given type.

What changes are included in this PR?

New module with documentation and unit tests
A table sample example is switched to use this module for SQL parsing
A small fix-up to run
cargo run --package datafusion-examples --example relation_planner
instead of
cargo run --package datafusion-examples --example relation_planner -- all

Are these changes tested?

Unit tests
Table sample example runs

Are there any user-facing changes?

A new API is provided

alamb

Thanks @theirix

alamb · 2025-12-14T13:14:01Z

datafusion/optimizer/src/simplify_expressions/simplify_sql_literal.rs

+where
+    T: TryFrom<ScalarValue, Error = DataFusionError>,
+{
+    match context.sql_to_expr(expr.clone(), &Arc::clone(schema)) {


it is a lot of ceremony to do this process

However, it also feels like this function is non trivial to use itself (it has 4 parameters and a type parameter)

I wonder if we added a method like SessionContext::simplify to invoke the simplifier more easily

Then extracting an expr as a particular type might look like

let expr = ...; // Expr to simplity // cast to target type and simplify let cast_expr = SessionContext::simplify(expr.cast(target_type)?); // Check if it simplified to a type let Expr::Literal(scalar) = cast_expr else { // error parsing }; // convert

That's true. Its purpose is to simplify expression and convert to a native type at the same time, hence the five parameters and the things behind the scenes (simplify, coerce, arrow cast and arrow-to-native cast) to shift it from client code.

For other usages of the optimiser's method - I agree that simplification could be done more easily in the client code via facade methods in context, as you suggested.

I do wonder if there's a way to compact this signature; for example, having both T and target_type seem redundant since they'd need to be compatible anyway, but off the top of my head I can't think of a way to combine them 🤔

Agree.

I've found a way to use an ArrowPrimitiveType and derive a native type from this trait. Now it looks much more compact with three parameters.

alamb · 2025-12-14T13:15:55Z

datafusion-examples/examples/relation_planner/table_sample.rs

    }
 }

-/// Parse a SQL expression as a numeric value (supports basic arithmetic).


@geoffreyclaude I wonder what your thoughts on this PR and the approach of parse_sql_literal?

adriangb · 2025-12-14T22:23:24Z

Don't we have this already at the logical and physical expression layers?

theirix · 2025-12-19T12:19:37Z

Don't we have this already at the logical and physical expression layers?

@adriangb please correct me if I'm wrong, but since we are accessing SQL ast, we cannot have it at a higher level, especially physical planning. The proposed function won't be used in higher-level layers, but only for planning.

Jefffrey · 2025-12-24T00:57:32Z

Don't we have this already at the logical and physical expression layers?

@adriangb please correct me if I'm wrong, but since we are accessing SQL ast, we cannot have it at a higher level, especially physical planning. The proposed function won't be used in higher-level layers, but only for planning.

I think the question is if we already have an optimizer rule to simplify expressions (including evaluating literals), what benefit does this PR bring to do it at the SQL planning level?

adriangb · 2025-12-24T01:08:42Z

Don't we have this already at the logical and physical expression layers?

@adriangb please correct me if I'm wrong, but since we are accessing SQL ast, we cannot have it at a higher level, especially physical planning. The proposed function won't be used in higher-level layers, but only for planning.

I think the question is if we already have an optimizer rule to simplify expressions (including evaluating literals), what benefit does this PR bring to do it at the SQL planning level?

Yes that was my thought as well

theirix · 2025-12-24T11:12:56Z

Don't we have this already at the logical and physical expression layers?

@adriangb please correct me if I'm wrong, but since we are accessing SQL ast, we cannot have it at a higher level, especially physical planning. The proposed function won't be used in higher-level layers, but only for planning.

I think the question is if we already have an optimizer rule to simplify expressions (including evaluating literals), what benefit does this PR bring to do it at the SQL planning level?

Does it only work if the optimizer is fed with an expression datafusion_expr::Expr?

These lines with logic for AST expression rely on having a numeric as a physical value - check if it is in 0..1 range, float or int, etc. We don't pass it as an expression for the optimizer to process later.

For other cases, of course, optimizer rules can properly work with expressions, and this function won't be needed.

Jefffrey

I can understand the use case for this now, though the plumbing required does seem a bit gnarly 🤔

Jefffrey · 2025-12-24T14:50:19Z

datafusion-examples/examples/relation_planner/main.rs

    let example: ExampleKind = std::env::args()
        .nth(1)
-        .ok_or_else(|| DataFusionError::Execution(format!("Missing argument. {usage}")))?
+        .unwrap_or_else(|| ExampleKind::All.to_string())


This doesn't seem consistent with the other example groups

I agree, this change shouldn't belong to this PR. I wanted to keep the original behaviour of running tests via classic cargo run --example relation_planner compared to cargo run --example relation_planner -- all. If it makes sense, I can make a consistent change for all examples in a separate PR.

A separate issue/PR for this sounds good

Jefffrey · 2025-12-24T15:09:51Z

datafusion/optimizer/src/simplify_expressions/simplify_sql_literal.rs

+where
+    T: TryFrom<ScalarValue, Error = DataFusionError>,
+{
+    match context.sql_to_expr(expr.clone(), &Arc::clone(schema)) {


I do wonder if there's a way to compact this signature; for example, having both T and target_type seem redundant since they'd need to be compatible anyway, but off the top of my head I can't think of a way to combine them 🤔

theirix added 5 commits December 13, 2025 18:58

Allow to run datafusion-examples relation_planner without parameters

d9b39a3

datafusion-sql: expose relation module

3421187

Add SqlToRelRelationContext constructor

690e167

Add parse_sql_literal module

5b5a838

Switch table sample example to lib sql parsing

b2bc6cf

github-actions bot added sql SQL Planner optimizer Optimizer rules labels Dec 13, 2025

theirix added 7 commits December 13, 2025 19:25

Reformat

a46e040

Use unwrap_or_else

1cb87c1

Ignore doctest

e5a95d4

Gate parse_sql_literal with sql feature

93ba06e

Enable optimizer feature sql

90f7a4b

Enable sql feature for datafusion in examples

964a326

Set sql feature for optimizer crate

df70b24

github-actions bot added the core Core DataFusion crate label Dec 13, 2025

theirix marked this pull request as ready for review December 13, 2025 23:30

alamb reviewed Dec 14, 2025

View reviewed changes

Jefffrey reviewed Dec 24, 2025

View reviewed changes

theirix added 2 commits December 24, 2025 15:28

Omit example launcher change

78e2dc1

Simplify signature by deriving a primitive type from ArrowPrimitiveType

7aa15c8

feat: plan-time SQL expression simplifying #19311

Are you sure you want to change the base?

feat: plan-time SQL expression simplifying #19311

Conversation

theirix commented Dec 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Which issue does this PR close?

Rationale for this change

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

Uh oh!

alamb left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adriangb commented Dec 14, 2025

Uh oh!

theirix commented Dec 19, 2025

Uh oh!

Jefffrey commented Dec 24, 2025

Uh oh!

adriangb commented Dec 24, 2025

Uh oh!

theirix commented Dec 24, 2025

Uh oh!

Jefffrey left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

theirix commented Dec 13, 2025 •

edited

Loading