Flow the WITH ORDINALITY struct as a record constructor value - #4651
g31pranjal wants to merge 2 commits into
Conversation
18b6d61 to
1af4396
Compare
730f88a to
7493cd5
Compare
1af4396 to
fb7c601
Compare
7493cd5 to
88b0d18
Compare
fb7c601 to
ef81c06
Compare
88b0d18 to
f3ab945
Compare
#4651 made a `WITH ORDINALITY` explode flow its `(element, ordinal)` struct as a record constructor, but a plain explode still flowed the bare element. That left the two shapes incomparable: a plain explode on the query side could not be related to a `WITH ORDINALITY` explode on the candidate side, because `MaxMatchMap` descends into a record constructor but not into an opaque value, so there was nothing below the element to match. Every explode now flows a record -- `(element)` or `(element, ordinal)` -- and the element is reachable as `_0` on either side. The flag #4650 introduced is simply set for both variants. Everything that reads what an explode flows follows from that. `FieldValue.ofFieldNamesAndFuseIfPossible` fuses a field access onto an existing field value instead of nesting one inside the other, and the key expression expansion uses it. The pull-up rules require the child of the value being pulled up to be the quantified object value itself, so a nested access could not be pulled through the explode's quantifier at all. An in-join and an in-union bind an element of the collection under `__corr_«alias»`, not the struct the explode flows, so the plans they put underneath have to be translated: `ExplodeExpression.elementBindingTranslationMap` makes each such alias stand for the element alone, which composes `«alias»._0` away to a plain reference to the binding. Neither rule can take an explode with ordinality as a source, since there is no place to put the ordinal. On the SQL side, a quantifier over an unnesting stands for the struct, so the element is reached through `_0`: `LogicalOperator` unwraps it when it builds the unnesting's attributes, `QuantifierValues` sees through it when resolving an index definition down to the base record, and a star expansion over an unnesting expands the element's fields rather than the struct's. The `AT` ordinal is a property of the unnesting rather than a column of the row, so it is ephemeral: nameable, but not part of a star. `ExplodePlanTest` follows the same way: a plain explode flows a record, so the skip-and-limit tests read the element out of it, and the plans that have to flow the element itself ask for that shape explicitly. The hash of a plain explode moves accordingly; the hash a plan that flows the element itself has always had is pinned alongside it.
ef81c06 to
d46370d
Compare
f3ab945 to
d28361e
Compare
An `EXPLODE WITH ORDINALS`'s value is a single opaque `QueriedValue`, even though it flows 2 pieces of information - element and its position. Nothing inside it is reachable, which is fine for evaluation but it leaves no way to relate the element to anything. ### Why `RecordConstructorValue` is needed: An index utilizing an `UnnestedRecordType` requires that the `SyntheticRecordType` is expanded into a candidate. This expansion in turn requires unnesting with ordinality, because the proto descriptor of the `UnnestedRecordType`, as well as its primary key, carries the position of the element. A query that unnests the same collection but asks for no ordinals must be matched against this candidate, by relating the query's element to the candidate's element: `MaxMatchMap` has to find the query's value among the candidate's reachable sub-values. `MaxMatchMap` descends only into record constructors. An opaque `QueriedValue` of the struct type cannot be descended into. Having `RecordConstructorValue`, makes the element a reachable sub-value and the match findable structurally. This matching happens in #4619; the shape it needs is turned on in #4651. ### Why the shape is recorded rather than inferred The value a plan flows is part of the plan's identity: `RecordQueryExplodePlan` hashes it, so changing the shape changes the plan hash of every `WITH ORDINALITY` plan. A plan serialized by an earlier version flows the opaque value and was hashed as such, and it has to keep doing both when a newer version reads it back. Deciding the shape from `withOrdinality` alone would hence not be backward-compatible. So the shape becomes state the plan records and serializes. This PR adds that state and leaves it at the value every existing plan flows; #4651 changes what newly planned explodes ask for. ### What this PR does `ExplodeExpression` and `RecordQueryExplodePlan` gain a `flowsRecordConstructorValue` flag. What the flag decides differs between the two variants. With ordinality the struct is the result type either way, so only the shape of the value changes: an older version that cannot see the flag still executes such a plan identically. Plainly, the struct is the flag's doing — the result type is the element type without it and a one-column struct with it — so the plan produces that struct rather than the bare element. That is why each variant needs its own PR to turn the shape on: #4651 for the ordinality one, and #4623 for the plain one, which reaches into everything that reads what an explode flows. **Nothing asks for new shape yet.** That will be enabled in later PRs ### Compatibility The flag is serialized in porto. Absent means the opaque value, which is precisely what a plan serialized before the field existed flows, so such a plan keeps both its shape and its hash.
42c4b1f to
9890e1d
Compare
c0e7a3a to
f456ca6
Compare
An index defined on an `UnnestedRecordType` had no match candidate, so the Cascades planner could never use it. Build one from the graph #4626 expands the type into, and teach the relational layer to declare such a type from SQL: a synthetic table whose constituents come from unnesting a repeated field, the metadata to serialize it, and the DDL to define an index over it. The query side is a plain explode of the same repeated field, and it matches the candidate's explode with ordinality through #4619, whose mapping reaches the element because the candidate flows it as a column of a record constructor. `unnested-record-type-indexes.yamsql` covers the whole path end to end, from declaring the type in a schema template to a covering scan of the index. Squashed from the pre-split branch and re-rooted onto #4619, so that #4626 -> #4651 -> #4619 -> #4641 is linear.
#4650 gave an explode the flag that decides whether it flows the element and the ordinal as a `RecordConstructorValue` or as one opaque value of the struct type, but left every explode flowing the opaque value. Ask for the record constructor where the SQL layer builds an `AT` unnesting, which is the only place that produces an explode with ordinality. The record-layer constructors keep defaulting to the opaque value, so nothing else changes shape. Both shapes stand for the same data and evaluate identically -- the plan builds its struct from the protobuf descriptor its declared type names -- but the element and the ordinal are now reachable sub-values, which is what lets anything be related to them. The expected plans of the five `AT` queries that also carry an `IN` move, since an `IN` over a value read out of a record is costed differently than one over an opaque value. They move in both directions: `array-join-at`'s `WHERE "at" IN (1, 2)` drops its join over the `IN` list for a single scan with a residual filter, two queries with an `IN` on the primary key keep their operators but move the unnesting across the in-join boundary, and `in-predicate`'s two queries with an `IN` on the unnested value start scanning the table once per `IN` element, which is #4621. No index match is lost, and all results still verify. A plan serialized before the flag existed still deserializes to the opaque value, so it keeps the shape, and the hash, it was planned with. A release that predates #4650 drops the flag instead, so this cannot be released before #4650 is.
3fa86c3 to
109de81
Compare
f456ca6 to
609becf
Compare
An index defined on an `UnnestedRecordType` had no match candidate, so the Cascades planner could never use it. Build one from the graph #4626 expands the type into, and teach the relational layer to declare such a type from SQL: a synthetic table whose constituents come from unnesting a repeated field, the metadata to serialize it, and the DDL to define an index over it. The query side is a plain explode of the same repeated field, and it matches the candidate's explode with ordinality through #4619, whose mapping reaches the element because the candidate flows it as a column of a record constructor. `unnested-record-type-indexes.yamsql` covers the whole path end to end, from declaring the type in a schema template to a covering scan of the index. Squashed from the pre-split branch and re-rooted onto #4619, so that #4626 -> #4651 -> #4619 -> #4641 is linear.
📊 Metrics Diff Analysis ReportSummary
ℹ️ About this analysisThis automated analysis compares query planner metrics between the base branch and this PR. It categorizes changes into:
The last category in particular may indicate planner regressions that should be investigated. Plan and Metrics ChangedThese queries experienced both plan and metrics changes. This generally indicates that there was some planner change Total: 2 queries Statistical Summary (Plan and Metrics Changed)
Significant Regressions (Plan and Metrics Changed)There were 2 outliers detected. Outlier queries have a significant regression in at least one field. Statistically, this represents either an increase of more than two standard deviations above the mean or a large absolute increase (e.g., 100).
|
#4650, now merged, added the
flowsRecordConstructorValueflag but left every explode flowing the opaque value. This PR turns it on, and only from the SQL layer:LogicalOperator.generateCorrelatedFieldAccess, where anATunnesting is built, asks for the record constructor. That is the only place that produces an explode with ordinality, and the record-layer constructors keep defaulting to the opaque value, so nothing else changes shape. Wrapping the element of a plain explode in a struct of its own is #4623.Both shapes stand for the same data and evaluate identically — the plan builds its struct from the protobuf descriptor its declared type names — but the element and the ordinal are now reachable sub-values, which is what lets anything be related to them. The matching that relies on it is #4619; why the shape matters at all is covered in #4650.
What changes in plans
Nothing in
fdb-record-layer-corechanges, and no core test needed updating. What moves is the expected plan of the fiveATqueries that also carry anIN: anINover a value read out of a record is costed differently than one over an opaque value. The movement goes in both directions and is worth a look:array-join-at:317,WHERE "at" IN (1, 2): was an explode of theINlist joined with a scan, and is now a single scan with a residualFILTER _._1 + 1 IN @c19. Cheaper.array-join-at:305andin-predicate:402, bothINon the primary key: same operators, but the unnesting moves across the in-join boundary — into the in-join in the first, out of it in the second.in-predicate:382andin-predicate:410, bothINon the unnested value: theINlist explode becomes the outer loop with the table scan inside it, i.e. one full scan perINelement. This is Cost model scores a nested data access the same as an unnested one, so an IN-join can be chosen over a single scan #4621 — the cost model scores a data access nested under an in-join the same as an unnested one.No index match is lost, and all results still verify. The full
:yaml-tests:quickTesthas no other plan mismatches.Compatibility
A plan serialized before the flag existed has it unset, deserializes to the opaque value, and so keeps the shape, and the hash, it was planned with.
The other direction is what gates this PR: a release that predates #4650 does not know the field, drops it, reads the plan as flowing the opaque value, and rejects the continuation.
mixedModeTestagainst 4.15.1.0 — the newest release, cut before #4650 merged — fails all four force-continuation variants ofarrayJoinAtandinPredicatewithcannot continue query due to mismatch between serialized and actual plan hashfor exactly that reason. The same suites pass 12/12 against an external server built from #4650. So this should land only once a release containing #4650 exists.