New logical plan for unnest struct type

[duongcongtoai]

I'm implementing unnest for struct type column. For now we only support unnest for list type in this executor
While unnest on list will increases the number of rows of the input, unnest on struct will keep the output rows the same, while flatten the fields of the struct into new columns.
To me i think we should have this difference be aware at logical plan layer, meaning we add a new option called UnnestStruct to this enum.
Reference from polar: https://github.com/pola-rs/polars/blob/eb7f9391cb7f2fb6984b6a72581168c5425abcfe/crates/polars-plan/src/logical_plan/functions/schema.rs#L53
they have separate logical node for explode (unnest on list) and unnest (unnest on struct)

wdyt

[alamb]

What is the alternative to a new LogicalPlan? Potentially add a new field to https://docs.rs/datafusion/latest/datafusion/logical_expr/struct.Unnest.html ?

unnest on struct will keep the output rows the same, while flatten the fields of the struct into new columns.

Given this explanation, I wonder is there some reason we couldn't make unnest_struct a scalar function? That would be a lot simpler than adding a whole new LogicalPlan type 🤔

[duongcongtoai]

I notice in polar code they have something called unnest_pushdown: https://github.com/pola-rs/polars/blob/2feb8331fd8e61771f0e5e9e9f657d5a38aaf05f/polars/polars-lazy/polars-plan/src/logical_plan/optimizer/projection_pushdown/mod.rs#L261
If we consider this as a UDF it will hide away this optimization opportunity.
But i notice we recently moved some core function into UDF, could you give me the original discussion on why we did this?

[duongcongtoai]

and also the current trait for UDF only returns 1 column as output

    fn invoke(&self, _args: &[ColumnarValue]) -> Result<ColumnarValue>;

If we consider this as an UDF, we may have to update this trait

[sadboy]

make unnest_struct a scalar function

"unnest" is not a scalar function, it's more a table function. Most SQL engines don't allow unnest in the SELECT items, only in the FROM clauses.

[sadboy]

I don't see why we need to change anything in the logical plan for this -- any logical plan analyzer can already be fully aware of this difference by simply looking at the type of the Column to be unnested. I think this difference is much better suited to be modeled in the physical layer alone.

[jayzhan211]

cc @jonahgao I think you might be interested in

[duongcongtoai]

• The reason is i see that this operation on different types belong to different logic, and i wonder should we group them by how the parser call them, for example some query engine uses "explode" instead of unnest for explodin list values into multiple rows.
• And also, if we differentiate them at logical level, then some optimizor such as PushdownFilter can pushdown predicate on struct subfield if it sees there is a logical plan UnnestStruct there (but if we use only one logical node to group them i think we can still have this possibility, just with extra check on the column type)

D create table example(addr struct(host varchar, port int), id int);
D insert into example values(row('localhost',8000),2);
D insert into example values(row('localhost',80),1);
D explain select port from (select unnest(addr) from example) where host='localhost';

┌─────────────────────────────┐
│┌───────────────────────────┐│
││       Physical Plan       ││
│└───────────────────────────┘│
└─────────────────────────────┘
┌───────────────────────────┐
│         PROJECTION        │
│   ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─   │
│            port           │
└─────────────┬─────────────┘                             
┌─────────────┴─────────────┐
│         SEQ_SCAN          │
│   ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─   │
│          example          │
│   ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─   │
│            addr           │
│   ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─   │
│     Filters: addr.host    │
│=localhost AND addr.ho...  │
│   ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─   │
│           EC: 1           │
└───────────────────────────┘   

[duongcongtoai]

But actually i have one argument that goes against my inital proposal, is something like this: https://cloud.google.com/bigquery/docs/reference/standard-sql/query-syntax#unnest_and_structs
Where it is reasonable to have an operator handling both horizontal and vertical transformation at the same time

[jayzhan211]

some query engine uses "explode" instead of unnest

I think we can map different function name to one node if they are doing the same thing but just with different name

but if we use only one logical node to group them i think we can still have this possibility, just with extra check on the column type

Agree, we just need to check the type, I don't think the difference is large enough that we need another node to differentiate them

I think we can start with UnnestStructExec first (or even extend on UnnestExec if possible), and then introduce another logical node if we found practical use-case.

[sadboy]

some optimizor such as PushdownFilter can pushdown predicate

That's a fair point. But I think both predicate and projection pushdowns are actually benefitial to both array and struct unnests (e.g. array unnest can probably do a length computation instead of copying all the exploded arrays if it knows it's going to be projected out), so it in fact may be an argument for keeping them as a single logical plan node. What you need is to expand the Unnest struct with projection and filter fields (i.e. mirroring the corresponding fields in TableScan) so it can take advantage of the pushdown optimizers.

[duongcongtoai]

nice, thanks for the suggestions everyone