Plan with structure not flagged for having too many names

[EpsilonPrime]

The following plan is not flagged by the validator as having too many root names (it should have just one). Without the two extra names the following plan executes perfectly in DuckDB.

{ "relations": [ { "root": { "input": { "read": { "common": { "direct": {} }, "baseSchema": { "names": [ "r", "a", "b" ], "struct": { "types": [ { "struct": { "types": [ { "i64": { "nullability": "NULLABILITY_REQUIRED" } }, { "string": { "nullability": "NULLABILITY_REQUIRED" } } ], "nullability": "NULLABILITY_NULLABLE" } } ], "nullability": "NULLABILITY_REQUIRED" } }, "namedTable": { "names": [ "mytesttable" ] } } }, "names": [ "r", "a", "b" ] } } ], "version": { "minorNumber": 42, "producer": "mytool" } }

[Blizzara]

Unless I'm mistaken, the plan has a single struct column with two fields in it - so that makes three names in total, no? Maybe DuckDB just doesn't implement struct field name handling correctly?

(I've been staring so much at those "names" fields lately...)

[jvanstraten]

I haven't been active in substrait for a little under two years and no one's really maintained the validator since, so I would expect neither me nor the validator to be an authoritative source, but it doesn't look like this has changed. See the comment at https://github.com/substrait-io/substrait/blob/main/proto/substrait/algebra.proto#L422-L423. The root relation should have three names in this case, because it names all fields, including nested struct fields, in depth-first order. Note that there is actually a positive test case in the validator precisely to test this here.

DuckDB probably treats the root names as toplevel field or column names rather than nested field names. I personally don't think that's all that weird of an interpretation, either, so I'm not surprised DuckDB's implementation disagrees with that comment in the protobuf spec. I'd say it's more of a spec issue, so I'm guessing you'll get more eyes on this if you post in the main substrait repository. Or at DuckDB, of course.

[EpsilonPrime]

It does indeed look like the DuckDB behavior is at fault. I'll take it up over there.