Conversation
There was a problem hiding this comment.
Pull request overview
Updates the project to a newer DataFusion (DF) revision and adjusts an optimizer-plan assertion to match DF 52.4’s changed logical plan output around COALESCE/casts/union coercion.
Changes:
- Bump all
datafusion*git dependencies inCargo.tomlto a new pinned revision. - Update a logical-plan expectation in
src/materialized/dependencies.rstests to reflect DF 52.4 optimizer output (CSE extraction andCOALESCEexpansion).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| Cargo.toml | Updates pinned DataFusion git revisions across all datafusion-* crates. |
| src/materialized/dependencies.rs | Updates expected EXPLAIN plan output for a test case and documents why DF 52.4 changes the plan. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "| | Projection: CASE WHEN __common_expr_2 IS NOT NULL THEN __common_expr_2 ELSE t2.year END AS year |", | ||
| "| | Projection: CAST(t1.year AS Utf8View) AS __common_expr_2, t2.year |", | ||
| "| | Full Join: Using CAST(t1.year AS Utf8View) = t2.year |", |
There was a problem hiding this comment.
The updated expected plan asserts the internal CSE-generated alias __common_expr_2. That alias index is not a stable part of the API and can change with minor optimizer rule reordering, leading to brittle/failing tests without any functional regression. Consider normalizing the explain output before comparison (e.g., replace __common_expr_\d+ with a placeholder), or weakening this assertion to check for key operators/expressions without depending on the exact common-expr name/number.
No description provided.