Skip to content

Commit

Permalink
opt: fix star expansion of un-labeled tuples
Browse files Browse the repository at this point in the history
Prior to this commit, there was a bug with star expansion of
un-labeled tuples in which only the first column of the tuple
was projected. This was happening because the name "?column?"
was being passed to NewTypedColumnAccessExpr for all columns in
the tuple, which made it impossible to determine that they were
actually different expressions. This commit fixes the issue by
ensuring that an empty column name is always passed to
NewTypedColumnAccessExpr when the tuple elements are unlabeled.
This signals that the tuple index should be used instead of the
name.

Fixes cockroachdb#48179

Release note (bug fix): Fixed a bug in which (tuple).* was only
expanded to the first column in the tuple and the remaining
elements were dropped.
  • Loading branch information
rytaft committed May 1, 2020
1 parent 193281a commit 1d339a8
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 9 deletions.
4 changes: 2 additions & 2 deletions pkg/sql/logictest/testdata/logic_test/srfs
Original file line number Diff line number Diff line change
Expand Up @@ -1087,8 +1087,8 @@ query II colnames
SELECT (unnest(ARRAY[(1,2),(3,4)])).*
----
?column? ?column?
1 1
3 3
1 2
3 4

query T colnames
SELECT * FROM unnest(ARRAY[(1,2),(3,4)])
Expand Down
14 changes: 14 additions & 0 deletions pkg/sql/opt/optbuilder/testdata/project
Original file line number Diff line number Diff line change
Expand Up @@ -229,3 +229,17 @@ project
│ └── columns: k:1!null v:2
└── projections
└── ((v:2, v:2, v:2) AS a, b, c) [as=x:3]

# Regression test for #48179. Star expansion of un-labeled tuple must project
# all columns from the tuple.
build
SELECT (b).* FROM (VALUES (((1, 2)))) as a(b)
----
project
├── columns: "?column?":2 "?column?":3
├── values
│ ├── columns: column1:1
│ └── ((1, 2),)
└── projections
├── (column1:1).@1 [as="?column?":2]
└── (column1:1).@2 [as="?column?":3]
17 changes: 10 additions & 7 deletions pkg/sql/opt/optbuilder/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,29 +81,32 @@ func (b *Builder) expandStar(
tTuple, isTuple := texpr.(*tree.Tuple)

aliases = typ.TupleLabels()
for i := len(aliases); i < len(typ.TupleContents()); i++ {
// Add aliases for all the non-named columns in the tuple.
aliases = append(aliases, "?column?")
}
exprs = make([]tree.TypedExpr, len(typ.TupleContents()))
for i := range typ.TupleContents() {
if isTuple {
// De-tuplify: ((a,b,c)).* -> a, b, c
exprs[i] = tTuple.Exprs[i].(tree.TypedExpr)
} else {
// Can't de-tuplify:
// either (Expr).* -> (Expr).a, (Expr).b, (Expr).c if there are enough labels,
// or (Expr).* -> (Expr).@1, (Expr).@2, (Expr).@3 if labels are missing.
// either (Expr).* -> (Expr).a, (Expr).b, (Expr).c if there are enough
// labels, or (Expr).* -> (Expr).@1, (Expr).@2, (Expr).@3 if labels are
// missing.
//
// We keep the labels if available so that the column name
// generation still produces column label "x" for, e.g. (E).x.
colName := ""
if i < len(typ.TupleContents()) {
if i < len(aliases) {
colName = aliases[i]
}
// NewTypedColumnAccessExpr expects colName to be empty if the tuple
// should be accessed by index.
exprs[i] = tree.NewTypedColumnAccessExpr(texpr, colName, i)
}
}
for i := len(aliases); i < len(typ.TupleContents()); i++ {
// Add aliases for all the non-named columns in the tuple.
aliases = append(aliases, "?column?")
}

case *tree.AllColumnsSelector:
src, srcMeta, err := t.Resolve(b.ctx, inScope)
Expand Down

0 comments on commit 1d339a8

Please sign in to comment.