opt: expand immediate stars in row constructors - #174773
Open
Alignyx wants to merge 1 commit into
Open
Conversation
ROW(table.*) previously wrapped the table's expanded fields in an additional tuple. For example, array_agg(ROW(table.*)) produced nested records where PostgreSQL produces an array of flat row values. Explicit nested row constructors acquired one additional level as well. Expand immediate table/composite star arguments in the enclosing row constructor before ordinary child traversal. Preserve deliberate nested rows, bare whole-row references, field order, visibility, and tuple metadata. Reuse the existing star resolver and leave ordinary function arguments unchanged. Add native optbuilder coverage for direct and aliased stars, mixed and repeated fields, composite-expression stars, intentional nesting, bare whole-row controls, and ordered array_agg. The same regression fixture fails before the fix and passes afterward. Fixes cockroachdb#115150 Epic: none Release note (bug fix): Fixed a bug where ROW(table.*) introduced an extra level of record nesting instead of expanding the table's fields. This could produce incorrectly nested array elements in expressions such as array_agg(ROW(table.*)). Explicit nested ROW expressions and whole-row references without a star retain their intended nesting.
|
Thank you for contributing to CockroachDB. Please ensure you have followed the guidelines for creating a PR. My owl senses detect your PR is good for review. Please keep an eye out for any test failures in CI. 🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
ROW(table.*)introduced an extra record layer, soarray_agg(ROW(table.*))returned arrays whose elements had the wrong tuple shape. This PR expands immediate star arguments into the enclosing row constructor's fields, matching PostgreSQL's row-constructor semantics.Fixes #115150.
Problem and reproduction
Before this change, the array contains one-field records whose sole field is another record:
After this change, it contains the two-field records requested by the query:
The aggregate's
ORDER BYmakes element order explicit for this reproduction; the defect concerns tuple shape, not ordering. The result should matcharray_agg(ROW(x, y) ORDER BY x)andarray_agg(xy.* ORDER BY x). Likewise,ROW(ROW(xy.*))should preserve its explicit outer row while expanding the inner row's star exactly once.Cause and change
During optbuilder's expression walk, a star encountered as an ordinary expression is converted into a tuple of its expanded columns. That is appropriate for a whole-row argument such as
array_agg(xy.*). Previously, the same handling ran for a star directly inside a row constructor: the child became a tuple, and the enclosingROW(...)then wrapped that tuple as one field. The extra nesting was already present in the optimizer expression before aggregate execution.scope.VisitPrenow handles*tree.Tuplebefore the generic child walk:AllColumnsSelectorandTupleStarchildren with the existingexpandStarresolver and append each expanded field to the enclosing expression list.Why this preserves the intended semantics
Expansion is restricted to syntax that directly requests the fields of a row. It does not flatten arbitrary tuple-valued expressions:
ROW(xy)still contains one whole-row field, and explicit nestedROW(...)layers remain. Non-star children are retained for normal name resolution. ReusingexpandStarpreserves the existing field ordering, visibility rules (including hidden columns), and composite-expression resolution. Copying the tuple preserves its labels and other metadata. Ordinary function-star arguments continue through their existing path.This fixes the shape at row construction, so aggregate execution and array formatting require no special cases or changes.
Validation
The branch is based directly on upstream
8812064a015d2faf99d3fc7e15880f94042954b0and contains only the product change and its native regression file.TestBuilder/row-star-expansion, including the orderedarray_agg(ROW(xy.*))reproduction, immediate stars, mixed/repeated fields, aliases, composite-expression stars, deliberate nesting, and bare whole-row controls.//pkg/sql/opt/optbuilder:optbuilder_testexecutable on this branch: 109 tests and subtests passed, with no failures or skips.vectorize=onandvectorize=off, twice per setting: 20/88 observations passed on the upstream baseline and 88/88 passed with the patch. Comparisons retain complete ordered array values, duplicates, NULLs, and tuple nesting. All reference values and previously correct results were unchanged.crlfmt -fast -tab 2andgit diff --checkpassed.The complete native package can be run with
./dev test pkg/sql/opt/optbuilder; the focused case isTestBuilder/row-star-expansion. In this environment the./dev generate,./dev lint --short, and./dev testwrappers refuse to run as root, so validation used directly built Bazel test executables with--norun_validations. Full-repository generation, lint, and tests are not claimed. SQL validation used real single-node test servers; no historical release or separate PostgreSQL binary was tested.The product fix and the initial regression inputs were generated with DeepSeek, then reviewed and tested locally; an additional direct aggregate regression was added during PR preparation.
Release note (bug fix): Fixed a bug where
ROW(table.*)introduced an extra level of record nesting instead of expanding the table's fields. This could produce incorrectly nested array elements in expressions such asarray_agg(ROW(table.*)). Explicit nestedROWexpressions and whole-row references without a star retain their intended nesting.