From bee5bf066bf0ac5670c6a3aafa92e45624e9d4f5 Mon Sep 17 00:00:00 2001 From: rharding6373 Date: Thu, 12 Jan 2023 11:29:27 -0800 Subject: [PATCH] sql: support `*` in udf bodies This change allows `*` usage in UDF bodies. We rewrite UDF ASTs in place to expand `*`s into the columns they reference. Informs: #90080 Epic: CRDB-19496 Release note (sql change): Allow `*` expressions in UDFs. --- pkg/ccl/backupccl/backup_test.go | 3 +- .../tests/3node-tenant/generated_test.go | 7 + pkg/sql/logictest/testdata/logic_test/udf | 18 --- .../logictest/testdata/logic_test/udf_star | 141 ++++++++++++++++++ .../tests/fakedist-disk/generated_test.go | 7 + .../tests/fakedist-vec-off/generated_test.go | 7 + .../tests/fakedist/generated_test.go | 7 + .../generated_test.go | 7 + .../tests/local-vec-off/generated_test.go | 7 + .../logictest/tests/local/generated_test.go | 7 + pkg/sql/opt/optbuilder/delete.go | 4 +- pkg/sql/opt/optbuilder/insert.go | 8 +- pkg/sql/opt/optbuilder/join.go | 4 +- pkg/sql/opt/optbuilder/mutation_builder.go | 2 +- pkg/sql/opt/optbuilder/project.go | 35 ++++- pkg/sql/opt/optbuilder/select.go | 17 +-- .../opt/optbuilder/testdata/create_function | 20 ++- pkg/sql/opt/optbuilder/testdata/udf | 32 ++++ pkg/sql/opt/optbuilder/update.go | 4 +- pkg/sql/opt/optbuilder/util.go | 3 - 20 files changed, 283 insertions(+), 57 deletions(-) create mode 100644 pkg/sql/logictest/testdata/logic_test/udf_star diff --git a/pkg/ccl/backupccl/backup_test.go b/pkg/ccl/backupccl/backup_test.go index f0e0e480d94a..16da3509e2dc 100644 --- a/pkg/ccl/backupccl/backup_test.go +++ b/pkg/ccl/backupccl/backup_test.go @@ -10627,7 +10627,6 @@ CREATE FUNCTION sc1.f1(a sc1.enum1) RETURNS INT LANGUAGE SQL AS $$ SELECT nextval('sc1.sq1'); $$; `) - rows := srcSQLDB.QueryStr(t, `SELECT function_id FROM crdb_internal.create_function_statements WHERE function_name = 'f1'`) require.Equal(t, 1, len(rows)) require.Equal(t, 1, len(rows[0])) @@ -10674,7 +10673,7 @@ $$; }) require.NoError(t, err) - // Bakcup the whole src cluster. + // Backup the whole src cluster. srcSQLDB.Exec(t, `BACKUP INTO $1`, localFoo) // Restore into target cluster. diff --git a/pkg/ccl/logictestccl/tests/3node-tenant/generated_test.go b/pkg/ccl/logictestccl/tests/3node-tenant/generated_test.go index eed192f6d298..fa92cd05e1a8 100644 --- a/pkg/ccl/logictestccl/tests/3node-tenant/generated_test.go +++ b/pkg/ccl/logictestccl/tests/3node-tenant/generated_test.go @@ -2019,6 +2019,13 @@ func TestTenantLogic_udf( runLogicTest(t, "udf") } +func TestTenantLogic_udf_star( + t *testing.T, +) { + defer leaktest.AfterTest(t)() + runLogicTest(t, "udf_star") +} + func TestTenantLogic_union( t *testing.T, ) { diff --git a/pkg/sql/logictest/testdata/logic_test/udf b/pkg/sql/logictest/testdata/logic_test/udf index efcb3f6882f6..a2badff7f389 100644 --- a/pkg/sql/logictest/testdata/logic_test/udf +++ b/pkg/sql/logictest/testdata/logic_test/udf @@ -52,24 +52,6 @@ CREATE FUNCTION err(i INT) RETURNS INT LANGUAGE SQL AS 'SELECT j' statement error pgcode 42703 column \"j\" does not exist CREATE FUNCTION err(i INT) RETURNS INT LANGUAGE SQL AS 'SELECT a FROM ab WHERE a = j' -statement error pgcode 0A000 functions do not currently support \* expressions -CREATE FUNCTION err(i INT) RETURNS ab LANGUAGE SQL AS 'SELECT * FROM ab' - -statement error pgcode 0A000 functions do not currently support \* expressions -CREATE FUNCTION err(i INT) RETURNS ab LANGUAGE SQL AS 'SELECT ab.* FROM ab' - -statement error pgcode 0A000 functions do not currently support \* expressions -CREATE FUNCTION err(i INT) RETURNS ab LANGUAGE SQL AS $$ - SELECT 1; - SELECT * FROM ab; -$$ - -statement error pgcode 0A000 functions do not currently support \* expressions -CREATE FUNCTION err(i INT) RETURNS INT LANGUAGE SQL AS $$ - SELECT * FROM ab; - SELECT 1; -$$ - statement ok CREATE FUNCTION d(i INT2) RETURNS INT4 LANGUAGE SQL AS 'SELECT i' diff --git a/pkg/sql/logictest/testdata/logic_test/udf_star b/pkg/sql/logictest/testdata/logic_test/udf_star new file mode 100644 index 000000000000..1aef22c17a7f --- /dev/null +++ b/pkg/sql/logictest/testdata/logic_test/udf_star @@ -0,0 +1,141 @@ +statement ok +CREATE TABLE t_onecol (a INT); +INSERT INTO t_onecol VALUES (1) + +statement ok +CREATE TABLE t_twocol (a INT, b INT); +INSERT INTO t_twocol VALUES (1,2) + +statement ok +CREATE FUNCTION f_unqualified_onecol() RETURNS INT AS +$$ + SELECT * FROM t_onecol; +$$ LANGUAGE SQL; + +statement ok +CREATE FUNCTION f_subquery() RETURNS INT AS +$$ + SELECT * FROM (SELECT a FROM (SELECT * FROM t_onecol) AS foo) AS bar; +$$ LANGUAGE SQL; + +statement ok +CREATE FUNCTION f_unqualified_twocol() RETURNS t_twocol AS +$$ + SELECT * FROM t_twocol; +$$ LANGUAGE SQL; + +statement ok +CREATE FUNCTION f_allcolsel() RETURNS t_twocol AS +$$ + SELECT t_twocol.* FROM t_twocol; +$$ LANGUAGE SQL; + +statement ok +CREATE FUNCTION f_allcolsel_alias() RETURNS t_twocol AS +$$ + SELECT t1.* FROM t_twocol AS t1, t_twocol AS t2 WHERE t1.a = t2.a; +$$ LANGUAGE SQL; + +statement ok +CREATE FUNCTION f_tuplestar() RETURNS t_twocol AS +$$ + SELECT (t_twocol.*).* FROM t_twocol; +$$ LANGUAGE SQL; + +statement ok +CREATE FUNCTION f_unqualified_multicol() RETURNS INT AS +$$ + SELECT *, a FROM t_onecol; + SELECT a FROM t_onecol; +$$ LANGUAGE SQL; + +statement ok +CREATE FUNCTION f_unqualified_doublestar() RETURNS INT AS +$$ + SELECT *, * FROM t_onecol; + SELECT a FROM t_onecol; +$$ LANGUAGE SQL; + +query TTT +SELECT oid, proname, prosrc +FROM pg_catalog.pg_proc WHERE proname LIKE 'f\_%' ORDER BY oid; +---- +100108 f_unqualified_onecol SELECT t_onecol.a FROM test.public.t_onecol; +100109 f_subquery SELECT bar.a FROM (SELECT a FROM (SELECT t_onecol.a FROM test.public.t_onecol) AS foo) AS bar; +100110 f_unqualified_twocol SELECT t_twocol.a, t_twocol.b FROM test.public.t_twocol; +100111 f_allcolsel SELECT t_twocol.a, t_twocol.b FROM test.public.t_twocol; +100112 f_allcolsel_alias SELECT t1.a, t1.b FROM test.public.t_twocol AS t1, test.public.t_twocol AS t2 WHERE t1.a = t2.a; +100113 f_tuplestar SELECT t_twocol.a, t_twocol.b FROM test.public.t_twocol; +100114 f_unqualified_multicol SELECT t_onecol.a, a FROM test.public.t_onecol; + SELECT a FROM test.public.t_onecol; +100115 f_unqualified_doublestar SELECT t_onecol.a, t_onecol.a FROM test.public.t_onecol; + SELECT a FROM test.public.t_onecol; + +query I +SELECT f_unqualified_onecol() +---- +1 + +query I +SELECT f_subquery() +---- +1 + +statement ok +ALTER TABLE t_onecol ADD COLUMN b INT DEFAULT 5; + +query I +SELECT f_unqualified_onecol() +---- +1 + +query I +SELECT f_subquery() +---- +1 + +# It's ok to drop a column that was not used by the original UDF. +statement ok +ALTER TABLE t_onecol DROP COLUMN b; + +query T +SELECT f_unqualified_twocol() +---- +(1,2) + +query T +SELECT f_allcolsel() +---- +(1,2) + +query T +SELECT f_allcolsel_alias() +---- +(1,2) + +statement ok +ALTER TABLE t_twocol ADD COLUMN c INT DEFAULT 5; + +# TODO(#95558): With early binding, postgres returns an error after adding a +# column when the table is used as the return type. Note that this behavior is +# ok for late binding. +query T +SELECT f_unqualified_twocol() +---- +(1,2) + +# Altering a column type is not allowed in postgres or CRDB. +statement error pq: cannot alter type of column "b" because function "f_unqualified_twocol" depends on it +ALTER TABLE t_twocol ALTER b TYPE FLOAT; + +# TODO(harding): Postgres allows column renaming when only referenced by UDFs. +statement error pq: cannot rename column "a" because function "f_unqualified_twocol" depends on it +ALTER TABLE t_twocol RENAME COLUMN a TO d; + +# TODO(harding): Postgres allows table renaming when only referenced by UDFs. +statement error pq: cannot rename relation "t_twocol" because function "f_unqualified_twocol" depends on it +ALTER TABLE t_twocol RENAME TO t_twocol_prime; + +# Dropping a column a UDF depends on is not allowed. +statement error pq: cannot drop column "b" because function "f_unqualified_twocol" depends on it +ALTER TABLE t_twocol DROP COLUMN b; diff --git a/pkg/sql/logictest/tests/fakedist-disk/generated_test.go b/pkg/sql/logictest/tests/fakedist-disk/generated_test.go index f063f7c1018e..1d7d2263459a 100644 --- a/pkg/sql/logictest/tests/fakedist-disk/generated_test.go +++ b/pkg/sql/logictest/tests/fakedist-disk/generated_test.go @@ -1990,6 +1990,13 @@ func TestLogic_udf( runLogicTest(t, "udf") } +func TestLogic_udf_star( + t *testing.T, +) { + defer leaktest.AfterTest(t)() + runLogicTest(t, "udf_star") +} + func TestLogic_union( t *testing.T, ) { diff --git a/pkg/sql/logictest/tests/fakedist-vec-off/generated_test.go b/pkg/sql/logictest/tests/fakedist-vec-off/generated_test.go index 1ed95c3e9141..bb58d21dbe4e 100644 --- a/pkg/sql/logictest/tests/fakedist-vec-off/generated_test.go +++ b/pkg/sql/logictest/tests/fakedist-vec-off/generated_test.go @@ -1997,6 +1997,13 @@ func TestLogic_udf( runLogicTest(t, "udf") } +func TestLogic_udf_star( + t *testing.T, +) { + defer leaktest.AfterTest(t)() + runLogicTest(t, "udf_star") +} + func TestLogic_union( t *testing.T, ) { diff --git a/pkg/sql/logictest/tests/fakedist/generated_test.go b/pkg/sql/logictest/tests/fakedist/generated_test.go index aabbee91bed6..0580b4334d2a 100644 --- a/pkg/sql/logictest/tests/fakedist/generated_test.go +++ b/pkg/sql/logictest/tests/fakedist/generated_test.go @@ -2011,6 +2011,13 @@ func TestLogic_udf( runLogicTest(t, "udf") } +func TestLogic_udf_star( + t *testing.T, +) { + defer leaktest.AfterTest(t)() + runLogicTest(t, "udf_star") +} + func TestLogic_union( t *testing.T, ) { diff --git a/pkg/sql/logictest/tests/local-legacy-schema-changer/generated_test.go b/pkg/sql/logictest/tests/local-legacy-schema-changer/generated_test.go index aa2ce22a5fb9..f6e62fb6e38f 100644 --- a/pkg/sql/logictest/tests/local-legacy-schema-changer/generated_test.go +++ b/pkg/sql/logictest/tests/local-legacy-schema-changer/generated_test.go @@ -1976,6 +1976,13 @@ func TestLogic_udf( runLogicTest(t, "udf") } +func TestLogic_udf_star( + t *testing.T, +) { + defer leaktest.AfterTest(t)() + runLogicTest(t, "udf_star") +} + func TestLogic_union( t *testing.T, ) { diff --git a/pkg/sql/logictest/tests/local-vec-off/generated_test.go b/pkg/sql/logictest/tests/local-vec-off/generated_test.go index 0da606fd62c9..f554fe62764b 100644 --- a/pkg/sql/logictest/tests/local-vec-off/generated_test.go +++ b/pkg/sql/logictest/tests/local-vec-off/generated_test.go @@ -2011,6 +2011,13 @@ func TestLogic_udf( runLogicTest(t, "udf") } +func TestLogic_udf_star( + t *testing.T, +) { + defer leaktest.AfterTest(t)() + runLogicTest(t, "udf_star") +} + func TestLogic_union( t *testing.T, ) { diff --git a/pkg/sql/logictest/tests/local/generated_test.go b/pkg/sql/logictest/tests/local/generated_test.go index 8569be3004de..87de51ba4005 100644 --- a/pkg/sql/logictest/tests/local/generated_test.go +++ b/pkg/sql/logictest/tests/local/generated_test.go @@ -2200,6 +2200,13 @@ func TestLogic_udf( runLogicTest(t, "udf") } +func TestLogic_udf_star( + t *testing.T, +) { + defer leaktest.AfterTest(t)() + runLogicTest(t, "udf_star") +} + func TestLogic_union( t *testing.T, ) { diff --git a/pkg/sql/opt/optbuilder/delete.go b/pkg/sql/opt/optbuilder/delete.go index 834eee850a8c..bcac82ceb73d 100644 --- a/pkg/sql/opt/optbuilder/delete.go +++ b/pkg/sql/opt/optbuilder/delete.go @@ -66,7 +66,7 @@ func (b *Builder) buildDelete(del *tree.Delete, inScope *scope) (outScope *scope // Build the final delete statement, including any returned expressions. if resultsNeeded(del.Returning) { - mb.buildDelete(*del.Returning.(*tree.ReturningExprs)) + mb.buildDelete(del.Returning.(*tree.ReturningExprs)) } else { mb.buildDelete(nil /* returning */) } @@ -76,7 +76,7 @@ func (b *Builder) buildDelete(del *tree.Delete, inScope *scope) (outScope *scope // buildDelete constructs a Delete operator, possibly wrapped by a Project // operator that corresponds to the given RETURNING clause. -func (mb *mutationBuilder) buildDelete(returning tree.ReturningExprs) { +func (mb *mutationBuilder) buildDelete(returning *tree.ReturningExprs) { mb.buildFKChecksAndCascadesForDelete() // Project partial index DEL boolean columns. diff --git a/pkg/sql/opt/optbuilder/insert.go b/pkg/sql/opt/optbuilder/insert.go index 5951ab72482b..99cffb0a42e6 100644 --- a/pkg/sql/opt/optbuilder/insert.go +++ b/pkg/sql/opt/optbuilder/insert.go @@ -281,9 +281,9 @@ func (b *Builder) buildInsert(ins *tree.Insert, inScope *scope) (outScope *scope // See mutationBuilder.buildCheckInputScan. mb.insertExpr = mb.outScope.expr - var returning tree.ReturningExprs + var returning *tree.ReturningExprs if resultsNeeded(ins.Returning) { - returning = *ins.Returning.(*tree.ReturningExprs) + returning = ins.Returning.(*tree.ReturningExprs) } switch { @@ -669,7 +669,7 @@ func (mb *mutationBuilder) addSynthesizedColsForInsert() { // buildInsert constructs an Insert operator, possibly wrapped by a Project // operator that corresponds to the given RETURNING clause. -func (mb *mutationBuilder) buildInsert(returning tree.ReturningExprs) { +func (mb *mutationBuilder) buildInsert(returning *tree.ReturningExprs) { // Disambiguate names so that references in any expressions, such as a // check constraint, refer to the correct columns. mb.disambiguateColumns() @@ -874,7 +874,7 @@ func (mb *mutationBuilder) setUpsertCols(insertCols tree.NameList) { // buildUpsert constructs an Upsert operator, possibly wrapped by a Project // operator that corresponds to the given RETURNING clause. -func (mb *mutationBuilder) buildUpsert(returning tree.ReturningExprs) { +func (mb *mutationBuilder) buildUpsert(returning *tree.ReturningExprs) { // Merge input insert and update columns using CASE expressions. mb.projectUpsertColumns() diff --git a/pkg/sql/opt/optbuilder/join.go b/pkg/sql/opt/optbuilder/join.go index be883ffd63a6..a825e44b50c4 100644 --- a/pkg/sql/opt/optbuilder/join.go +++ b/pkg/sql/opt/optbuilder/join.go @@ -33,7 +33,7 @@ import ( func (b *Builder) buildJoin( join *tree.JoinTableExpr, locking lockingSpec, inScope *scope, ) (outScope *scope) { - leftScope := b.buildDataSource(join.Left, nil /* indexFlags */, locking, inScope) + leftScope := b.buildDataSource(&join.Left, nil /* indexFlags */, locking, inScope) inScopeRight := inScope isLateral := b.exprIsLateral(join.Right) @@ -45,7 +45,7 @@ func (b *Builder) buildJoin( inScopeRight.context = exprKindLateralJoin } - rightScope := b.buildDataSource(join.Right, nil /* indexFlags */, locking, inScopeRight) + rightScope := b.buildDataSource(&join.Right, nil /* indexFlags */, locking, inScopeRight) // Check that the same table name is not used on both sides. b.validateJoinTableNames(leftScope, rightScope) diff --git a/pkg/sql/opt/optbuilder/mutation_builder.go b/pkg/sql/opt/optbuilder/mutation_builder.go index 5b152eed1da8..101164006ae2 100644 --- a/pkg/sql/opt/optbuilder/mutation_builder.go +++ b/pkg/sql/opt/optbuilder/mutation_builder.go @@ -1019,7 +1019,7 @@ func (mb *mutationBuilder) mapToReturnColID(tabOrd int) opt.ColumnID { // buildReturning wraps the input expression with a Project operator that // projects the given RETURNING expressions. -func (mb *mutationBuilder) buildReturning(returning tree.ReturningExprs) { +func (mb *mutationBuilder) buildReturning(returning *tree.ReturningExprs) { // Handle case of no RETURNING clause. if returning == nil { expr := mb.outScope.expr diff --git a/pkg/sql/opt/optbuilder/project.go b/pkg/sql/opt/optbuilder/project.go index a9c9e7622a9c..d5ce97a06d2d 100644 --- a/pkg/sql/opt/optbuilder/project.go +++ b/pkg/sql/opt/optbuilder/project.go @@ -79,7 +79,7 @@ func (b *Builder) dropOrderingAndExtraCols(s *scope) { // and adds the resulting aliases and typed expressions to outScope. See the // header comment for analyzeSelectList. func (b *Builder) analyzeProjectionList( - selects tree.SelectExprs, desiredTypes []*types.T, inScope, outScope *scope, + selects *tree.SelectExprs, desiredTypes []*types.T, inScope, outScope *scope, ) { // We need to save and restore the previous values of the replaceSRFs field // and the field in semaCtx in case we are recursively called within a @@ -98,7 +98,7 @@ func (b *Builder) analyzeProjectionList( // and adds the resulting aliases and typed expressions to outScope. See the // header comment for analyzeSelectList. func (b *Builder) analyzeReturningList( - returning tree.ReturningExprs, desiredTypes []*types.T, inScope, outScope *scope, + returning *tree.ReturningExprs, desiredTypes []*types.T, inScope, outScope *scope, ) { // We need to save and restore the previous value of the field in // semaCtx in case we are recursively called within a subquery @@ -109,7 +109,7 @@ func (b *Builder) analyzeReturningList( b.semaCtx.Properties.Require(exprKindReturning.String(), tree.RejectSpecial) inScope.context = exprKindReturning - b.analyzeSelectList(tree.SelectExprs(returning), desiredTypes, inScope, outScope) + b.analyzeSelectList((*tree.SelectExprs)(returning), desiredTypes, inScope, outScope) } // analyzeSelectList is a helper function used by analyzeProjectionList and @@ -119,10 +119,15 @@ func (b *Builder) analyzeReturningList( // // As a side-effect, the appropriate scopes are updated with aggregations // (scope.groupby.aggs) +// +// If we are building a function, the `selects` expressions will be overwritten +// with expressions that replace any `*` expressions with their columns. func (b *Builder) analyzeSelectList( - selects tree.SelectExprs, desiredTypes []*types.T, inScope, outScope *scope, + selects *tree.SelectExprs, desiredTypes []*types.T, inScope, outScope *scope, ) { - for i, e := range selects { + var expansions tree.SelectExprs + expanded := false + for i, e := range *selects { // Start with fast path, looking for simple column reference. texpr := b.resolveColRef(e.Expr, inScope) if texpr == nil { @@ -142,13 +147,24 @@ func (b *Builder) analyzeSelectList( } aliases, exprs := b.expandStar(e.Expr, inScope) + if b.insideFuncDef { + expanded = true + for _, expr := range exprs { + col := expr.(*scopeColumn) + expansions = append(expansions, tree.SelectExpr{Expr: tree.NewColumnItem(&col.table, col.name.ReferenceName())}) + } + } if outScope.cols == nil { - outScope.cols = make([]scopeColumn, 0, len(selects)+len(exprs)-1) + outScope.cols = make([]scopeColumn, 0, len(*selects)+len(exprs)-1) } for j, e := range exprs { outScope.addColumn(scopeColName(tree.Name(aliases[j])), e) } continue + default: + if b.insideFuncDef { + expansions = append(expansions, e) + } } } @@ -158,17 +174,22 @@ func (b *Builder) analyzeSelectList( } texpr = inScope.resolveType(e.Expr, desired) + } else if b.insideFuncDef { + expansions = append(expansions, e) } // Output column names should exactly match the original expression, so we // have to determine the output column name before we perform type // checking. if outScope.cols == nil { - outScope.cols = make([]scopeColumn, 0, len(selects)) + outScope.cols = make([]scopeColumn, 0, len(*selects)) } alias := b.getColName(e) outScope.addColumn(scopeColName(tree.Name(alias)), texpr) } + if b.insideFuncDef && expanded { + *selects = expansions + } } // buildProjectionList builds a set of memo groups that represent the given diff --git a/pkg/sql/opt/optbuilder/select.go b/pkg/sql/opt/optbuilder/select.go index 9b68a2d9fea3..0147240d7df0 100644 --- a/pkg/sql/opt/optbuilder/select.go +++ b/pkg/sql/opt/optbuilder/select.go @@ -44,14 +44,14 @@ import ( // See Builder.buildStmt for a description of the remaining input and // return values. func (b *Builder) buildDataSource( - texpr tree.TableExpr, indexFlags *tree.IndexFlags, locking lockingSpec, inScope *scope, + texpr *tree.TableExpr, indexFlags *tree.IndexFlags, locking lockingSpec, inScope *scope, ) (outScope *scope) { defer func(prevAtRoot bool) { inScope.atRoot = prevAtRoot }(inScope.atRoot) inScope.atRoot = false // NB: The case statements are sorted lexicographically. - switch source := texpr.(type) { + switch source := (*texpr).(type) { case *tree.AliasedTableExpr: if source.IndexFlags != nil { telemetry.Inc(sqltelemetry.IndexHintUseCounter) @@ -64,7 +64,7 @@ func (b *Builder) buildDataSource( locking = locking.filter(source.As.Alias) } - outScope = b.buildDataSource(source.Expr, indexFlags, locking, inScope) + outScope = b.buildDataSource(&source.Expr, indexFlags, locking, inScope) if source.Ordinality { outScope = b.buildWithOrdinality(outScope) @@ -110,7 +110,6 @@ func (b *Builder) buildDataSource( } ds, depName, resName := b.resolveDataSource(tn, privilege.SELECT) - locking = locking.filter(tn.ObjectName) if locking.isSet() { // SELECT ... FOR [KEY] UPDATE/SHARE also requires UPDATE privileges. @@ -142,7 +141,7 @@ func (b *Builder) buildDataSource( } case *tree.ParenTableExpr: - return b.buildDataSource(source.Expr, indexFlags, locking, inScope) + return b.buildDataSource(&source.Expr, indexFlags, locking, inScope) case *tree.RowsFromExpr: return b.buildZip(source.Items, inScope) @@ -1052,7 +1051,7 @@ func (b *Builder) buildSelectClause( // function that refers to variables in fromScope or an ancestor scope, // buildAggregateFunction is called which adds columns to the appropriate // aggInScope and aggOutScope. - b.analyzeProjectionList(sel.Exprs, desiredTypes, fromScope, projectionsScope) + b.analyzeProjectionList(&sel.Exprs, desiredTypes, fromScope, projectionsScope) // Any aggregates in the HAVING, ORDER BY and DISTINCT ON clauses (if they // exist) will be added here. @@ -1216,7 +1215,7 @@ func (b *Builder) buildFromTables( func (b *Builder) buildFromTablesRightDeep( tables tree.TableExprs, locking lockingSpec, inScope *scope, ) (outScope *scope) { - outScope = b.buildDataSource(tables[0], nil /* indexFlags */, locking, inScope) + outScope = b.buildDataSource(&tables[0], nil /* indexFlags */, locking, inScope) // Recursively build table join. tables = tables[1:] @@ -1266,7 +1265,7 @@ func (b *Builder) exprIsLateral(t tree.TableExpr) bool { func (b *Builder) buildFromWithLateral( tables tree.TableExprs, locking lockingSpec, inScope *scope, ) (outScope *scope) { - outScope = b.buildDataSource(tables[0], nil /* indexFlags */, locking, inScope) + outScope = b.buildDataSource(&tables[0], nil /* indexFlags */, locking, inScope) for i := 1; i < len(tables); i++ { scope := inScope // Lateral expressions need to be able to refer to the expressions that @@ -1275,7 +1274,7 @@ func (b *Builder) buildFromWithLateral( scope = outScope scope.context = exprKindLateralJoin } - tableScope := b.buildDataSource(tables[i], nil /* indexFlags */, locking, scope) + tableScope := b.buildDataSource(&tables[i], nil /* indexFlags */, locking, scope) // Check that the same table name is not used multiple times. b.validateJoinTableNames(outScope, tableScope) diff --git a/pkg/sql/opt/optbuilder/testdata/create_function b/pkg/sql/opt/optbuilder/testdata/create_function index 5ee1e03191e2..022af07662c4 100644 --- a/pkg/sql/opt/optbuilder/testdata/create_function +++ b/pkg/sql/opt/optbuilder/testdata/create_function @@ -42,7 +42,7 @@ create-function ├── CREATE FUNCTION f() │ RETURNS workday │ LANGUAGE SQL - │ AS $$SELECT w FROM t.public.workdays;$$ + │ AS $$SELECT w FROM [55(1) AS workdays];$$ └── dependencies ├── workdays [columns: w] └── workday @@ -65,7 +65,7 @@ create-function ├── CREATE FUNCTION f() │ RETURNS STRING │ LANGUAGE SQL - │ AS $$SELECT w::STRING FROM t.public.workdays;$$ + │ AS $$SELECT w::STRING FROM [55(1) AS workdays];$$ └── dependencies └── workdays [columns: w] @@ -76,7 +76,7 @@ create-function ├── CREATE FUNCTION f(IN a INT8) │ RETURNS INT8 │ LANGUAGE SQL - │ AS $$SELECT a FROM t.public.ab;$$ + │ AS $$SELECT a FROM [53(1, 2) AS ab];$$ └── dependencies └── ab [columns: a] @@ -87,7 +87,7 @@ create-function ├── CREATE FUNCTION f() │ RETURNS INT8 │ LANGUAGE SQL - │ AS $$SELECT b FROM t.public.ab@idx;$$ + │ AS $$SELECT b FROM [53(1, 2) AS ab]@idx;$$ └── dependencies └── ab@idx [columns: b] @@ -101,16 +101,22 @@ create-function ├── CREATE FUNCTION f() │ RETURNS INT8 │ LANGUAGE SQL - │ AS $$SELECT a FROM t.public.ab; + │ AS $$SELECT a FROM [53(1, 2) AS ab]; │ SELECT nextval('s');$$ └── dependencies ├── ab [columns: a] └── s build -CREATE FUNCTION f() RETURNS INT LANGUAGE SQL AS $$ SELECT * FROM ab $$ +CREATE FUNCTION f() RETURNS ab LANGUAGE SQL AS $$ SELECT * FROM ab $$ ---- -error (0A000): unimplemented: functions do not currently support * expressions +create-function + ├── CREATE FUNCTION f() + │ RETURNS ab + │ LANGUAGE SQL + │ AS $$SELECT * FROM [53(1, 2) AS ab];$$ + └── dependencies + └── ab [columns: a b] build CREATE FUNCTION f() RETURNS INT LANGUAGE SQL BEGIN ATOMIC SELECT 1; END; diff --git a/pkg/sql/opt/optbuilder/testdata/udf b/pkg/sql/opt/optbuilder/testdata/udf index 4f4bc04dfb1a..1ae251053f61 100644 --- a/pkg/sql/opt/optbuilder/testdata/udf +++ b/pkg/sql/opt/optbuilder/testdata/udf @@ -1166,3 +1166,35 @@ project │ └── projections │ └── variable: i:10 [as=i:13] └── const: 1 + +# -------------------------------------------------- +# UDFs with * expressions. +# -------------------------------------------------- + +exec-ddl +CREATE TABLE tstar ( + a INT +) +---- + +exec-ddl +CREATE FUNCTION fn_star() RETURNS INT LANGUAGE SQL AS 'SELECT * FROM tstar' +---- + +build format=show-scalars +SELECT fn_star() +---- +project + ├── columns: fn_star:5 + ├── values + │ └── tuple + └── projections + └── udf: fn_star [as=fn_star:5] + └── body + └── limit + ├── columns: a:1 + ├── project + │ ├── columns: a:1 + │ └── scan tstar + │ └── columns: a:1 rowid:2!null crdb_internal_mvcc_timestamp:3 tableoid:4 + └── const: 1 diff --git a/pkg/sql/opt/optbuilder/update.go b/pkg/sql/opt/optbuilder/update.go index 3ec93d7551f7..502e414de34b 100644 --- a/pkg/sql/opt/optbuilder/update.go +++ b/pkg/sql/opt/optbuilder/update.go @@ -108,7 +108,7 @@ func (b *Builder) buildUpdate(upd *tree.Update, inScope *scope) (outScope *scope // Build the final update statement, including any returned expressions. if resultsNeeded(upd.Returning) { - mb.buildUpdate(*upd.Returning.(*tree.ReturningExprs)) + mb.buildUpdate(upd.Returning.(*tree.ReturningExprs)) } else { mb.buildUpdate(nil /* returning */) } @@ -326,7 +326,7 @@ func (mb *mutationBuilder) addSynthesizedColsForUpdate() { // buildUpdate constructs an Update operator, possibly wrapped by a Project // operator that corresponds to the given RETURNING clause. -func (mb *mutationBuilder) buildUpdate(returning tree.ReturningExprs) { +func (mb *mutationBuilder) buildUpdate(returning *tree.ReturningExprs) { // Disambiguate names so that references in any expressions, such as a // check constraint, refer to the correct columns. mb.disambiguateColumns() diff --git a/pkg/sql/opt/optbuilder/util.go b/pkg/sql/opt/optbuilder/util.go index c3f7f68781a0..c210674d376c 100644 --- a/pkg/sql/opt/optbuilder/util.go +++ b/pkg/sql/opt/optbuilder/util.go @@ -64,9 +64,6 @@ func (b *Builder) expandStar( if b.insideViewDef { panic(unimplemented.NewWithIssue(10028, "views do not currently support * expressions")) } - if b.insideFuncDef { - panic(unimplemented.NewWithIssue(90080, "functions do not currently support * expressions")) - } switch t := expr.(type) { case *tree.TupleStar: texpr := inScope.resolveType(t.Expr, types.Any)