Restore dropped Spice patch families (metadata columns, object versioning, version-cache invalidation, hash-join accumulator hooks)#162
Conversation
Upstream DF53.1 rewrote the file-metadata/statistics cache so that cache validity is checked purely on size + last_modified (CachedFileMetadata::is_valid_for and CachedFileMetadataEntry::is_valid_for in execution/src/cache/cache_manager.rs). For versioned object stores (e.g. S3 with object versioning), a new object version can share the same size and last-modified timestamp, so the cache would incorrectly serve stale metadata. This restores the Spice patch (origin spiceai-52.5 commit d8a28cf) by adding is_same_file_version() to cache_unit.rs (etag/version aware) and consulting it from both is_valid_for checks.
Restores the Spice object-versioning patch (origin spiceai-52.5 commits f32727f / #113) on the parquet reader factories so versioned parquet files can be read consistently from object stores that support ETag/version pinning. - DefaultParquetFileReaderFactory / CachedParquetFileReaderFactory gain an object_versioning_type field + with_object_versioning_type(), and now build the inner reader via ParquetObjectReader::new_with_meta(...).with_object_versioning_type(...) (the arrow-rs fork @ rev 0bb8862 provides ObjectVersionType + new_with_meta). - ParquetFormat::create_physical_plan wires conf.object_versioning_type into the cached factory. - datasource 'parquet' feature now enables parquet/object_store so the store-level ObjectVersionType / ParquetObjectReader types are in scope when the field is compiled. - Cargo.lock: refresh so the spiceai/arrow-rs fork patches (arrow + parquet 58.3.0) are actually adopted; the base lock left these [patch] entries unused in the crate graph, so ObjectVersionType was not visible. The FileScanConfig object_versioning_type field + builder are added in the metadata-columns commit.
Restores the Spice metadata-columns patch family (origin spiceai-52.5 commits adea254 / 633900a / 14c2cbe and follow-ups), re-wiring the orphaned datasource/src/metadata.rs (MetadataColumn = _location/_last_modified/_size) that survived the 53.1 reconcile but lost all of its call sites. Because DF53 replaced the 52.5 ExtendedColumnProjector (a single chokepoint in file_stream.rs) with a per-source projection model (TableSchema + SplitProjection + ProjectionOpener, with the bespoke ParquetOpener literal-injection path), metadata columns are threaded the DF53-native way instead of porting ExtendedColumnProjector verbatim: - TableSchema carries metadata_cols, appended to table_schema() after the partition columns (with_metadata_cols / metadata_cols()). - SplitProjection::new_with_table_schema classifies trailing metadata-column indices separately from partition columns; ProjectionOpener substitutes them with per-file literals from PartitionedFile.object_meta (inject_metadata_columns_into_projection). The legacy SplitProjection::new is preserved unchanged (no metadata cols). - ParquetOpener (which does not use ProjectionOpener) folds the metadata columns into its literal_columns map from the file's ObjectMeta. - FileSource gains with_metadata_cols() (default None; implemented for parquet, csv, json, avro, arrow by folding into the source's TableSchema and recomputing its default projection). - FileScanConfigBuilder::with_metadata_cols() folds them into the FileSource; FileScanConfig also gains the object_versioning_type field + builder used by the parquet object-versioning path. - ListingOptions gains metadata_cols + with_metadata_cols + validate_metadata_cols and object_versioning_type + with_object_versioning_type; ListingTable wires both into the scan (create_file_source folds metadata cols into TableSchema; the scan builder sets object_versioning_type). Public API names/signatures match the spiceai-52.5 surface so spice2 call sites (cayenne provider/table.rs with_metadata_cols, runtime metadata tests, listing connector with_object_versioning_type) compile unchanged.
There was a problem hiding this comment.
Pull request overview
This PR reintroduces several “Spice” extension patch families that were dropped during the DataFusion 53.1.0 reconcile, focusing on (1) per-file metadata columns, (2) parquet/object-store object versioning propagation, and (3) version/etag-aware cache invalidation. It also refreshes Cargo.lock to ensure the spiceai/arrow-rs forked Arrow/Parquet crates are actually used in the dependency graph.
Changes:
- Thread metadata columns (
_location,_size,_last_modified) throughTableSchema/SplitProjection/ProjectionOpenerand into file sources (parquet/csv/json/avro/arrow), including parquet literal injection inParquetOpener. - Add
ObjectVersionTypeplumbing fromListingOptions→FileScanConfig→ parquet reader factories (ParquetObjectReader::new_with_meta(...).with_object_versioning_type(...)). - Restore version/etag-aware cache validation by introducing
is_same_file_versionand consulting it from cache validity checks.
Reviewed changes
Copilot reviewed 17 out of 18 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| datafusion/execution/src/cache/cache_unit.rs | Adds is_same_file_version helper for version/etag-aware cache invalidation. |
| datafusion/execution/src/cache/cache_manager.rs | Incorporates is_same_file_version into cached-metadata validity checks. |
| datafusion/datasource/src/table_schema.rs | Extends TableSchema to append metadata columns after partition columns. |
| datafusion/datasource/src/projection.rs | Adds metadata-column classification/mapping and literal injection in ProjectionOpener / SplitProjection. |
| datafusion/datasource/src/file.rs | Extends FileSource with with_metadata_cols hook. |
| datafusion/datasource/src/file_scan_config.rs | Adds builder support for metadata columns and object_versioning_type plumbing. |
| datafusion/datasource/Cargo.toml | Enables parquet/object_store under the datasource parquet feature. |
| datafusion/datasource-parquet/src/source.rs | Implements FileSource::with_metadata_cols for parquet and recomputes default projection. |
| datafusion/datasource-parquet/src/reader.rs | Adds object_versioning_type to parquet reader factories and uses new_with_meta. |
| datafusion/datasource-parquet/src/opener.rs | Injects metadata columns into parquet literal substitution path. |
| datafusion/datasource-parquet/src/file_format.rs | Wires scan config object_versioning_type into cached parquet reader factory. |
| datafusion/datasource-json/src/source.rs | Implements metadata columns and updates projection pushdown to use table-schema-aware split projection. |
| datafusion/datasource-csv/src/source.rs | Implements metadata columns and updates projection pushdown to use table-schema-aware split projection. |
| datafusion/datasource-avro/src/source.rs | Implements metadata columns and updates projection pushdown to use table-schema-aware split projection. |
| datafusion/datasource-arrow/src/source.rs | Implements metadata columns and updates projection pushdown to use table-schema-aware split projection. |
| datafusion/catalog-listing/src/table.rs | Wires listing options into scan config and appends metadata columns to the scan TableSchema. |
| datafusion/catalog-listing/src/options.rs | Adds metadata_cols, object_versioning_type, and metadata validation helper APIs. |
| Cargo.lock | Refreshes lockfile so Arrow/Parquet resolve to the intended fork versions and updates transitive deps. |
Comments suppressed due to low confidence (1)
datafusion/datasource/src/table_schema.rs:233
TableSchema::table_schemadoc comment is now outdated: the returned schema includes metadata columns as well (appended after partition columns), not justfile schema + partition columns.
/// Get the full table schema (file schema + partition columns).
///
/// This is the complete schema that will be seen by queries, combining
/// both the columns from the files and the partition columns.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Decision on the un-ported family 4 (hash-join custom accumulator): we are adopting DF53.1's native dynamic-filter pushdown ( |
- is_same_file_version: make object `version` authoritative when present on both sides (compare versions; only fall back to e_tag when version can't decide). e_tag presence/value differences no longer force invalidation when both versions match. Preserves empty-string normalization and the both-absent -> same-file rule. - Add focused unit tests for is_same_file_version edge cases (both-none, empty-string normalization, version match with e_tag on one side only, version mismatch, version presence differs, e_tag-only match/mismatch). - MetadataColumnIndex::in_remainder_projection: fix doc bound. Metadata and partition columns share a contiguous remainder range starting at num_file_columns (projected file cols), so with no projected partition columns a metadata column can sit at num_file_columns (not num_file_columns + num_partition_columns). - ListingTable::try_new: append metadata columns to table_schema after the partition columns so ListingTable::schema() exposes _location/_size/ _last_modified (restores spiceai-52.5 parity; SQL can reference them and the provider schema matches the scan's TableSchema). Also restores the validate_metadata_cols guard.
Summary
Restores Spice patch families that existed on the old
spiceai-52.5line (539e6963) but were functionally deleted by the 53.1.0 reconcile merge289b74a64("Carry Spice patches forward to DataFusion 53.1.0"). The commits remained reachable by SHA, but their wiring was gone from the tip tree ofspiceai-53-patches(0c648eb06).Three of the four audited families are restored here. The fourth (HashJoin custom left-side accumulator) is not restored because 53.1 deliberately replaced the underlying mechanism upstream — details below.
Built and tested against the spiceai/arrow-rs fork (
rev 0bb8862, arrow/parquet 58.3.0), which already carriesObjectVersionType+ParquetObjectReader::new_with_meta.Families restored
1. Metadata columns (
_location/_last_modified/_size)datasource/src/metadata.rs(MetadataColumn) survived the reconcile but was orphaned —with_metadata_cols/ExtendedColumnProjectorhad 0 references. Upstream 53.1 has noMetadataColumn, and it replaced the 52.5ExtendedColumnProjector(a single chokepoint infile_stream.rs) with a per-source projection model:TableSchema+SplitProjection+ProjectionOpener, plus the bespokeParquetOpenerliteral-injection path.Rather than port
ExtendedColumnProjectorverbatim (its host code no longer exists), metadata columns are threaded the DF53-native way:TableSchemacarriesmetadata_cols, appended totable_schema()after the partition columns.SplitProjection::new_with_table_schemaclassifies trailing metadata-column indices separately from partition columns;ProjectionOpenersubstitutes them with per-file literals fromPartitionedFile.object_meta(inject_metadata_columns_into_projection). The legacySplitProjection::newis unchanged.ParquetOpener(which does not useProjectionOpener) folds metadata columns into itsliteral_columnsmap from the file'sObjectMeta.FileSourcegainswith_metadata_cols()(defaultNone; implemented for parquet/csv/json/avro/arrow).FileScanConfigBuilder::with_metadata_cols()folds them into theFileSource.ListingOptionsgainsmetadata_cols+with_metadata_cols+validate_metadata_cols;ListingTablewires them into the scan.Public API names/signatures match the 52.5 surface so spice2 call sites (
cayenneprovider/table.rs, runtime metadata tests) compile unchanged.ExtendedColumnProjectoritself is not reintroduced — it is purely internal and has no spice2 call sites; its behavior is provided by the DF53-native injection above.2. Object versioning (
ObjectVersionType)Restores
object_versioning_typeend-to-end (originf32727fbb/ #113):DefaultParquetFileReaderFactory/CachedParquetFileReaderFactorygainobject_versioning_type+with_object_versioning_type(), and build the reader viaParquetObjectReader::new_with_meta(...).with_object_versioning_type(...).FileScanConfig/FileScanConfigBuildergainobject_versioning_type(cfgparquet);ParquetFormat::create_physical_planwires it into the cached factory.ListingOptionsgainsobject_versioning_type+with_object_versioning_type, wired intoListingTable's scan.datasource'sparquetfeature now enablesparquet/object_storeso the store-level types are in scope.[patch]entries unused in the crate graph (parquet resolved to crates.io 58.0.0), soObjectVersionTypewas not visible at all.3. Object-version cache invalidation (
is_same_file_version)Upstream 53.1 rewrote the metadata/statistics cache to validate purely on
size+last_modified(CachedFileMetadata::is_valid_for/CachedFileMetadataEntry::is_valid_for). Restores the etag/version-awareis_same_file_version(origind8a28cf63/ #113) incache_unit.rsand consults it from bothis_valid_forchecks, so versioned objects sharing size + mtime are correctly invalidated.Family NOT restored: HashJoin custom left-side accumulator
The 52.5 patch (origin
38b6c042c/463193add/ #117) madeHashJoinExecgeneric over aCollectLeftAccumulatortrait (HashJoinExec<A = MinMaxLeftAccumulator>), addedrecreate_with_accumulator::<T>(), and aColumnBoundstrait whosephysical_expr()produced a custom per-partition dynamic-filter expression (spice2'sExactLeftAccumulatorproduces an exact InList with a byte-budget fallback to range + bloom).DF53.1 deliberately replaced this mechanism upstream:
exec.rswas rewritten (+1643/-424 vs the 52.5 base),ColumnBoundsis now a concretepub(crate)min/max struct (not a trait), and the dynamic filter is built centrally inshared_bounds.rs::create_membership_predicatevia aPushdownStrategy(InList for small build sides, hash-table lookup for large) driven bySharedBuildAccumulator+inlist_builder.rs. There is no accumulator-injection seam, no generic parameter, and nophysical_expr()delegation.Restoring the spice2 API contract would mean reverting that upstream redesign across a near-rewritten module with incompatible
ColumnBoundssemantics (struct→trait, generic-param threading throughHashJoinExecBuilder/Debug/DisplayAs/ExecutionPlan+ ~43HashJoinExecreference sites), which is a re-architecture rather than a restore. Flagging explicitly rather than silently skipping. Re-enabling this needs a separate decision: either (a) fork 53.1's centralized dynamic-filter pipeline back to the trait-delegated model, or (b) adapt cayenne to 53.1's native InList/hash-table pushdown + a byte-budget config (spice2-side, out of scope here).Build / test
cargo check -p datafusion -p datafusion-datasource -p datafusion-catalog-listing -p datafusion-execution -p datafusion-physical-plan— clean (EXIT0).cargo test -p datafusion-datasource --lib— 107 passed, 0 failed (coversprojection/table_schema).cargo test -p datafusion-execution --lib cache— (see PR thread).Audit context
spiceai-52.5@539e6963.289b74a64("Carry Spice patches forward to DataFusion 53.1.0").spiceai-53-patches@0c648eb06.