Skip to content

chore: upgrade DataFusion to 54.1.0 - #1226

Open
paleolimbot wants to merge 19 commits into
apache:mainfrom
paleolimbot:codex/datafusion-54
Open

chore: upgrade DataFusion to 54.1.0#1226
paleolimbot wants to merge 19 commits into
apache:mainfrom
paleolimbot:codex/datafusion-54

Conversation

@paleolimbot

@paleolimbot paleolimbot commented Sep 2, 2026

Copy link
Copy Markdown
Member

Upgrades DataFusion from 52.5.0 to 54.1.0 with the corresponding Arrow, Parquet, object_store, GeoArrow, and Zarr versions. Includes initial trait, downcast, execution-plan, and metadata-cache API migrations.

A few larger updates:

  • The Parquet reader now uses a "moreslizer" instead of an opener; however, conceptually this fits well with what we were doing already so the change is minimal.
  • Logical serde was updated, so our placeholder approach had to be tweaked.
  • A few metadata bugs were encountered, both worked around: a subquery getting optimized into a join borks on metadata (so I disabled/tracked) and one logical optimizer rule can't handle unnest + select (which is in our doctests for what you normally do with ST_Dump).

The big diff is mostly the lockfile for the examples.

@github-actions
github-actions Bot requested a review from prantogg September 2, 2026 22:32
@paleolimbot paleolimbot changed the title build(rust): upgrade DataFusion to 54.1.0 chore: upgrade DataFusion to 54.1.0 Sep 4, 2026
@paleolimbot
paleolimbot marked this pull request as ready for review September 4, 2026 20:37
@paleolimbot

Copy link
Copy Markdown
Member Author

@zhangfengcdt Can you take a look at this one...I think it's ready for 👀 !

let state_builder = SessionStateBuilder::new_with_default_features();
// DataFusion #22620 workaround tracked by
// https://github.com/apache/sedona-db/issues/1232.
let state_builder = register_vendored_optimizer_rules(state_builder).unwrap();

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Could this also be applied in new_from_context()? Passing a regular SessionContext still hits the Unnest bug. This test fails in push_down_leaf_projections; changing the constructor to SedonaContext::new() returns the expected two rows.

Repro: save as rust/sedona/tests/supplied_context_dump.rs, then run cargo test -p sedona --test supplied_context_dump.

#[tokio::test]
async fn supplied_context_dump() -> datafusion::error::Result<()> {
    use datafusion::{
        functions::core::expr_fn::get_field,
        prelude::{col, SessionContext},
    };
    use sedona::context::SedonaContext;

    let ctx = SedonaContext::new_from_context(SessionContext::new())?;
    let batches = ctx
        .sql("SELECT ST_Dump(ST_GeomFromText('MULTIPOINT (0 0, 1 1)')) AS dump")
        .await?
        .unnest_columns(&["dump"])?
        .select(vec![get_field(col("dump"), "geom").alias("geometry")])?
        .collect()
        .await?;
    assert_eq!(batches.iter().map(|b| b.num_rows()).sum::<usize>(), 2);
    Ok(())
}

use datafusion_pruning::PruningStatistics;
use las::Header;
use object_store::{path::Path, ObjectMeta, ObjectStore, PutPayload};
use object_store::{path::Path, ObjectMeta, ObjectStore, ObjectStoreExt, PutPayload};

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Could we add the zero-point guard to extract_chunk_stats() too? An empty LAZ file using point format 6 has a 477..477 chunk. Normal reading now works, but collect_statistics=true still fails with Requested range was invalid. The old object_store 0.12.4 accepted that empty range.

Repro: save as rust/sedona-pointcloud/tests/empty_laz_statistics.rs, then run cargo test -p sedona-pointcloud --test empty_laz_statistics. The final call fails; setting collect_statistics to false succeeds.

#[tokio::test]
async fn empty_laz_statistics() {
    use las::{point::Format, Builder, Writer};
    use object_store::{local::LocalFileSystem, path::Path, ObjectStoreExt};
    use sedona_pointcloud::las::{metadata::LasMetadataReader, options::LasOptions};

    let dir = tempfile::tempdir().unwrap();
    let path = dir.path().join("empty.laz");
    let mut builder = Builder::from((1, 4));
    builder.point_format = Format::new(6).unwrap();
    builder.point_format.is_compressed = true;
    let mut writer = Writer::from_path(&path, builder.into_header().unwrap()).unwrap();
    writer.close().unwrap();

    let store = LocalFileSystem::new();
    let location = Path::from_filesystem_path(&path).unwrap();
    let object = store.head(&location).await.unwrap();
    let metadata = LasMetadataReader::new(&store, &object)
        .fetch_metadata().await.unwrap();
    assert_eq!(metadata.header.number_of_points(), 0);

    let mut options = LasOptions::default();
    options.collect_statistics = true;
    LasMetadataReader::new(&store, &object)
        .with_options(options)
        .fetch_metadata()
        .await
        .unwrap();
}

version = "0.4.0"
source = "git+https://github.com/apache/sedona-db.git#9d6bcfa178a341e23f40d58a7ec6244f7b5dca25"
version = "0.5.0"
source = "git+https://github.com/apache/sedona-db.git#7b2864a996cd14edde6bb2c8d451ca5df52756fe"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This revision still uses DataFusion 52.5, while the example now depends on 54.1. Could we regenerate the lockfile against a Sedona revision containing the upgrade?

From this PR checkout:

cd examples/sedonadb-rust
cargo tree --locked --depth 1 -p sedonadb-rust-example
cargo tree --locked --depth 1 -p sedona
cargo check --locked

The first tree shows DataFusion 54.1; the second shows 52.5. The example then passes a 54.1 Expr from col("name") to sort_by, which expects a 52.5 Expr (E0308). The ? calls also have incompatible DataFusionError types. CI rewrites the Sedona dependency to the PR head before building, so it does not test this committed lockfile.

pull Bot pushed a commit to TCeason/arrow-datafusion that referenced this pull request Sep 7, 2026
…ache#24939)

## Which issue does this PR close?

- Closes apache#24933

Related:
[apache/sedona-db#1231](apache/sedona-db#1231),
[apache/sedona-db#1226](apache/sedona-db#1226).
This is the uncorrelated physical-plan counterpart of the earlier
outer-reference metadata fix in apache#17524 / apache#17422.

## Rationale for this change

UDFs that distinguish Arrow extension types from their storage types
(for example spatial predicates such as `ST_Intersects`) need
`ARROW:extension:name` on every argument. Uncorrelated scalar subqueries
kept that metadata in the logical plan, but physical planning built a
`ScalarSubqueryExpr` from only the data type and nullability. The
synthesized physical field had empty metadata, so queries like `WHERE
udf(col, (SELECT geometry FROM t WHERE id = 1))` failed even though the
equivalent join form worked.

## What changes are included in this PR?

- `ScalarSubqueryExpr` now stores the output `FieldRef` (name
`scalar_subquery`, original type/nullability, and metadata) via
`new_with_metadata`. The existing `new` constructor is unchanged and
still produces a field with empty metadata.
- Physical lowering copies metadata from the logical subquery output
field while still using `Expr::nullable` so zero-row subqueries remain
nullable.
- Protobuf encoding adds an additive `metadata` map on
`PhysicalScalarSubqueryExprNode` so plan round-trips keep extension
metadata.

## What is the testing strategy for this PR?

- Unit test `scalar_subquery_preserves_output_field_metadata` in
`planner.rs` reproduces the drop during physical lowering.
- Unit test `return_field_preserves_extension_metadata` and an updated
proto round-trip in `scalar_subquery.rs`.
- End-to-end regression
`test_extension_metadata_preserve_in_uncorrelated_scalar_subquery` in
`user_defined_scalar_functions.rs`, based on the issue reproducer. The
existing EXISTS-subquery metadata test still passes.

## Are there any user-facing changes?

Additive only: `ScalarSubqueryExpr::new_with_metadata` and an optional
protobuf `metadata` map (older payloads decode as empty metadata).
Existing `new(data_type, nullable, ...)` keeps working. Queries whose
UDFs inspect argument field metadata now see the subquery's original
extension metadata in the physical plan.

---------

Co-authored-by: Marcelo Tesla <9055877+M-Tesla@users.noreply.github.com>
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.

3 participants