Skip to content

Add VariantExprAdapterFactory to read files with a different shredding layout - #149

Merged
sdf-jkl merged 20 commits into
datafusion-contrib:mainfrom
raghuvanshraj:variant-expr-adapter
Nov 10, 2026
Merged

sdf-jkl merged 20 commits into
datafusion-contrib:mainfrom
raghuvanshraj:variant-expr-adapter

Conversation

@raghuvanshraj

Copy link
Copy Markdown
Contributor

Adds VariantExprAdapterFactory, a PhysicalExprAdapterFactory that wraps DataFusion's default adapter. When a Variant column's layout in a Parquet file differs from the merged table layout, the default adapter casts the file struct to the table struct and null-fills the missing typed_value children, which the shredding spec reads as "not an object"; every shredded field of that file then returns NULL with no error. The new adapter instead reads the file column as is and, per batch, unshreds it and shreds it again to the table's typed_value schema. Files whose layout already matches the table are left to the default adapter.

Closes #148.

Installed per table with ListingTableConfig::with_expr_adapter_factory(Arc::new(VariantExprAdapterFactory::default())); the README has the full snippet. Wiring it in automatically for the extension type is a core question, raised on apache/datafusion#27089.

Two limits worth knowing: files that need the rewrite read the whole Variant column, so they don't get leaf-level projection; and only whole-column reads of the Variant are rewritten, so direct struct access like event['typed_value'] on a mismatched file still sees the null-filled cast.

Tests write real Parquet files and read them through a ListingTable: a shredded plus an unshredded file (3 of 6 rows NULL without the adapter, 6 of 6 with it), two files shredded on different fields, a whole-column read, and same-layout files showing no rewrite in the plan.

Validation: cargo test (new and existing tests, SLTs, README examples), cargo fmt --check, cargo clippy --all-targets -- -D warnings.

@alamb alamb left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thank you for this @raghuvanshraj

Could you please add .slt tests showing the usecase ?

This morning we merged the ability to create shredded variants directly in SQL

So it should now be straightforward to setup your usecase for the expression adapter with slt

Here are some docs that explain how shredding works in more detail

Comment thread README.md Outdated
+---------+-----------+
```

#### Files with different shredding layouts

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Should we suggest people install this adapter for all tables that read variant, even for those that aren't shredded, right?

Also, it would be really nice to be able to set this up fr all tables automatically somehow in register_all -- maybe that needs a new datafusion API 🤔

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Should we suggest people install this adapter for all tables that read variant, even for those that aren't shredded, right?

Yes @alamb agreed on this. Anyway an unshredded file never pays anything since the adapter never enters rewrite for those.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Also, it would be really nice to be able to set this up fr all tables automatically somehow in register_all -- maybe that needs a new datafusion API 🤔

For CREATE EXTERNAL TABLE actually this might work without a new API: register_all can replace the session's PARQUET table factory with one that wraps ListingTableFactory and rebuilds the ListingTable with the adapter attached. Every Parquet table created by SQL in that session then gets it.

@alamb thoughts?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think it is a great idea -- I actually tried it locally and it works great. I pushed to this PR and am going to push a few more commits too

@raghuvanshraj

Copy link
Copy Markdown
Contributor Author

Thank you for this @raghuvanshraj

Could you please add .slt tests showing the usecase ?

This morning we merged the ability to create shredded variants directly in SQL

* #145

So it should now be straightforward to setup your usecase for the expression adapter with slt

Here are some docs that explain how shredding works in more detail

* #152

@alamb I've added tests/test_files/expr_adapter.slt. It writes the two layouts with COPY ... OPTIONS ('variant.shredding' ...) from #145, reads them back with the default adapter (count(service) = 3, NULLs for the unshredded file) and through the new VariantListingTableFactory (6, all fields present), plus the different-keys case.

@alamb alamb left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks @raghuvanshraj -- this is great. I am going to spend some time pushing some comment cleanups, etc and hopefully we'll have this one ready to go

Comment thread src/lib.rs

/// Register all Variant scalar, aggregate, and table functions, cast rewrites,
/// `:` field access, SQL `VARIANT` type syntax, and JSON display of Variant values.
use datafusion::error::Result;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

this is a drive by cleanup

@alamb alamb left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thank you @raghuvanshraj -- I think this is a great step and works as advertised

I went through this PR carefully, and took the liberty of reworking a bunch of the examples, tests and comments in this PR so they are easier to understand.

I am going to go through the tests one more time

After this, I think the next step would be to make a benchmark:

We also now have enough pieces to benchmark reasonably and I have already found at least one problem

@sdf-jkl

sdf-jkl commented Nov 10, 2026

Copy link
Copy Markdown
Collaborator

I'd like to take a pass too

@sdf-jkl

sdf-jkl commented Nov 10, 2026

Copy link
Copy Markdown
Collaborator

Then again we can merge and iterate

@alamb

alamb commented Nov 10, 2026

Copy link
Copy Markdown
Collaborator

I also filed a follow on issue here

@alamb

alamb commented Nov 10, 2026

Copy link
Copy Markdown
Collaborator

I'd like to take a pass too

Sounds great. I will let you merge it then. I don't think there is any rush.

I have to work on some other stuff now today, but I think I filed a bunch of tickets and we are pushing this along well

@sdf-jkl sdf-jkl left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks @raghuvanshraj @alamb -- this is epic 🔥

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.

Reading Parquet files with different Variant shredding layouts null-fills typed_value; shredded fields read as NULL

3 participants