chore: upgrade DataFusion to 54.1.0 - #1226
Conversation
|
@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(); |
There was a problem hiding this comment.
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}; |
There was a problem hiding this comment.
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" |
There was a problem hiding this comment.
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 --lockedThe 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.
…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>
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 big diff is mostly the lockfile for the examples.