From d7705eee4eaef025abbbb7d31464398ce1e11b7c Mon Sep 17 00:00:00 2001 From: Bram Gruneir Date: Mon, 21 May 2018 12:51:29 -0400 Subject: [PATCH] sql: Add the ability to access a specific column in a labeled tuple This work is yet another step towards #16971. The labeled tuples introduced in #25283 can now be accessed using their labels. It's important to note that they require an extra parentheses around them to do so. ```sql SELECT (((1,'2',true) AS a, b, c)).a +---+ | a | +---+ | 1 | +---+ SELECT (((1,'2',true) AS a, b, c)).b +---+ | b | +---+ | 2 | +---+ SELECT (((1,'2',true) AS a, b, c)).c +------+ | c | +------+ | true | +------+ ``` This change prompted the need to restrict the labels in the tuple to be unique; checks and test cases for that have been added as well. Star expansion is still not allowed. Release note: Labeled tuples can now be accessed using their labels (e.g. `SELECT (((1,'2',true) AS a, b, c)).a` but it requires an extra level of parentheses to do so. --- pkg/sql/copy.go | 2 +- pkg/sql/logictest/testdata/logic_test/tuple | 75 ++++++++++++++++++++- pkg/sql/parser/parse_test.go | 4 ++ pkg/sql/render.go | 5 +- pkg/sql/sem/builtins/pg_builtins.go | 2 +- pkg/sql/sem/tree/eval.go | 7 +- pkg/sql/sem/tree/expr.go | 7 +- pkg/sql/sem/tree/type_check.go | 54 +++++++++++++-- pkg/sql/sem/tree/type_check_test.go | 32 ++++++++- 9 files changed, 168 insertions(+), 20 deletions(-) diff --git a/pkg/sql/copy.go b/pkg/sql/copy.go index 2e96d2a60518..c143b937262f 100644 --- a/pkg/sql/copy.go +++ b/pkg/sql/copy.go @@ -71,7 +71,7 @@ type copyMachine struct { p planner // parsingEvalCtx is an EvalContext used for the very limited needs to strings - // parsing. Is it not correcly initialized with timestamps, transactions and + // parsing. Is it not correctly initialized with timestamps, transactions and // other things that statements more generally need. parsingEvalCtx *tree.EvalContext } diff --git a/pkg/sql/logictest/testdata/logic_test/tuple b/pkg/sql/logictest/testdata/logic_test/tuple index 08ceb84f2eeb..3a82f5d7453e 100644 --- a/pkg/sql/logictest/testdata/logic_test/tuple +++ b/pkg/sql/logictest/testdata/logic_test/tuple @@ -793,9 +793,78 @@ true query error pq: tuples \(\(\(\(1, 2\) AS a, b\), 'equal'\) AS c, d\), \(\(\(\(1, 'huh'\) AS e, f\), 'equal'\) AS g, h\) are not comparable at index 1: tuples \(\(1, 2\) AS a, b\), \(\(1, 'huh'\) AS e, f\) are not comparable at index 2: unsupported comparison operator: = SELECT ((((1, 2) AS a, b), 'equal') AS c, d) = ((((1, 'huh') AS e, f), 'equal') AS g, h) -# Try giving everything the same name for a fairly well nested tuple. +# Ensure the number of labels matches the number of expressions +query error pq: the number of expressions in a labeled tuple \(2\) must match the number of labels \(1\) +SELECT ((1, '2') AS a) + +query error pq: the number of expressions in a labeled tuple \(1\) must match the number of labels \(2\) +SELECT (ROW(1) AS a, b) + +# Tuple labels must be different +query error pq: each label in a tuple must be unique \(\["a" "a"\]\) +SELECT ((1, '2') AS a, a) + +query error pq: each label in a tuple must be unique \(\["a" "a" "b"\]\) +SELECT ((1, '2', true) AS a, a, b) + +query error pq: each label in a tuple must be unique \(\["a" "b" "a"\]\) +SELECT ((1, '2', true) AS a, b, a) + +query error pq: each label in a tuple must be unique \(\["b" "a" "a"\]\) +SELECT ((1, '2', true) AS b, a, a) + +# But inner tuples can reuse labels query T colnames -SELECT ((((((1, '2', 3) AS a, a, a), ((4,'5') AS a, a), (ROW(6) AS a)) AS a, a, a), ((7, 8) AS a, a), (ROW('9') AS a)) AS a, a, a) +SELECT ((((((1, '2', 3) AS a, b, c), ((4,'5') AS a, b), (ROW(6) AS a)) AS a, b, c), ((7, 8) AS a, b), (ROW('9') AS a)) AS a, b, c) ---- -((((((1, '2', 3) AS a, a, a), ((4, '5') AS a, a), (ROW(6) AS a)) AS a, a, a), ((7, 8) AS a, a), (ROW('9') AS a)) AS a, a, a) +((((((1, '2', 3) AS a, b, c), ((4, '5') AS a, b), (ROW(6) AS a)) AS a, b, c), ((7, 8) AS a, b), (ROW('9') AS a)) AS a, b, c) (((1, '2', 3), (4, '5'), (6)),(7, 8),('9')) + +# Accessing a specific column +query error pq: could not identify column "x" in tuple{int AS a, int AS b, int AS c} +SELECT (((1,2,3) AS a,b,c)).x + +query ITBITB colnames +SELECT (((1,'2',true) AS a,b,c)).a + ,(((1,'2',true) AS a,b,c)).b + ,(((1,'2',true) AS a,b,c)).c + ,((ROW(1,'2',true) AS a,b,c)).a + ,((ROW(1,'2',true) AS a,b,c)).b + ,((ROW(1,'2',true) AS a,b,c)).c +---- +a b c a b c +1 2 true 1 2 true + +query error pq: each label in a tuple must be unique \(\["a" "b" "a"\]\) +SELECT (((1,'2',true) AS a,b,a)).a + +# The following are all cases that will be addressed in future work + +# Accessing all the columns +query error pq: star expansion of tuples is not supported +SELECT (((1,'2',true) AS a,b,c)).* + +query error pq: star expansion of tuples is not supported +SELECT ((ROW(1,'2',true) AS a,b,c)).* + +# From a Subquery + +query error pq: type tuple{int, string, bool} is not composite +SELECT ((t)).e, ((t)).f, ((t)).g +FROM ( + SELECT ((1,'2',true) AS e,f,g) AS t +) + +# From a table directly + +statement ok +CREATE TABLE t (a int, b string) + +statement ok +INSERT INTO t VALUES (1, 'one'), (2, 'two') + +query error pq: column name "t" not found +SELECT (t).a FROM t + +statement ok +DROP TABLE t diff --git a/pkg/sql/parser/parse_test.go b/pkg/sql/parser/parse_test.go index 85aa1c0fbc39..a32dcb42d8f2 100644 --- a/pkg/sql/parser/parse_test.go +++ b/pkg/sql/parser/parse_test.go @@ -529,8 +529,12 @@ func TestParse(t *testing.T) { {`SELECT ((1, 2, 3) AS a, b, c)`}, {`SELECT (ROW(1, 2, 3))`}, {`SELECT (ROW(1, 2, 3) AS a, b, c)`}, + {`SELECT ((ROW(1, 2, 3) AS a, b, c)).a`}, + {`SELECT ((ROW(1, 2, 3) AS a, b, c)).*`}, {`SELECT (ROW())`}, {`SELECT (ROW() AS a)`}, + {`SELECT ((ROW() AS a)).a`}, + {`SELECT ((ROW() AS a)).*`}, {`SELECT (TABLE a)`}, {`SELECT 0x1`}, {`SELECT 'Deutsch' COLLATE "DE"`}, diff --git a/pkg/sql/render.go b/pkg/sql/render.go index 5c279fa5b447..db3a963ccee4 100644 --- a/pkg/sql/render.go +++ b/pkg/sql/render.go @@ -502,11 +502,10 @@ func (v *srfExtractionVisitor) VisitPre(expr tree.Expr) (recurse bool, newNode t switch t := expr.(type) { case *tree.ColumnAccessExpr: - // TODO(knz): support arbitrary composite expressions. fe, ok := t.Expr.(*tree.FuncExpr) if !ok { - v.err = pgerror.UnimplementedWithIssueErrorf(24866, - "access to field in composite expression: %q", tree.ErrString(t.Expr)) + // If there is no function inside the column access expression, then it + // will be dealt with elsewhere. return false, expr } newFe := *fe diff --git a/pkg/sql/sem/builtins/pg_builtins.go b/pkg/sql/sem/builtins/pg_builtins.go index 22571f92236f..d78b0b079e80 100644 --- a/pkg/sql/sem/builtins/pg_builtins.go +++ b/pkg/sql/sem/builtins/pg_builtins.go @@ -357,7 +357,7 @@ func getTableNameForArg(ctx *tree.EvalContext, arg tree.Datum) (*tree.TableName, type pgPrivList map[string]func(withGrantOpt bool) (tree.Datum, error) // parsePrivilegeStr parses the provided privilege string and calls into the -// privilege option map for each specified privielge. +// privilege option map for each specified privilege. func parsePrivilegeStr(arg tree.Datum, availOpts pgPrivList) (tree.Datum, error) { // Postgres allows WITH GRANT OPTION to be added to a privilege type to // test whether the privilege is held with grant option. diff --git a/pkg/sql/sem/tree/eval.go b/pkg/sql/sem/tree/eval.go index 594e4d7ab9c6..6a860207badd 100644 --- a/pkg/sql/sem/tree/eval.go +++ b/pkg/sql/sem/tree/eval.go @@ -3395,8 +3395,11 @@ func (expr *CollateExpr) Eval(ctx *EvalContext) (Datum, error) { // Eval implements the TypedExpr interface. func (expr *ColumnAccessExpr) Eval(ctx *EvalContext) (Datum, error) { - return nil, pgerror.NewErrorf(pgerror.CodeInternalError, - "programmer error: column access expressions must be replaced before evaluation") + d, err := expr.Expr.(TypedExpr).Eval(ctx) + if err != nil { + return nil, err + } + return d.(*DTuple).D[expr.ColIndex], nil } // Eval implements the TypedExpr interface. diff --git a/pkg/sql/sem/tree/expr.go b/pkg/sql/sem/tree/expr.go index def855b0b53a..ff7fc8a80da1 100644 --- a/pkg/sql/sem/tree/expr.go +++ b/pkg/sql/sem/tree/expr.go @@ -1494,9 +1494,10 @@ func (node *CollateExpr) Format(ctx *FmtCtx) { // ColumnAccessExpr represents (SRF).x or (SRF).* expression. Specifically, it // allows accessing the column(s) from a Set Retruning Function. type ColumnAccessExpr struct { - Expr Expr - ColName string - Star bool + Expr Expr + ColName string + Star bool + ColIndex int // This should only be set during type checking. typeAnnotation } diff --git a/pkg/sql/sem/tree/type_check.go b/pkg/sql/sem/tree/type_check.go index ff38e6f92709..332c1e3d8a02 100644 --- a/pkg/sql/sem/tree/type_check.go +++ b/pkg/sql/sem/tree/type_check.go @@ -390,8 +390,45 @@ func (expr *CollateExpr) TypeCheck(ctx *SemaContext, desired types.T) (TypedExpr // TypeCheck implements the Expr interface. func (expr *ColumnAccessExpr) TypeCheck(ctx *SemaContext, desired types.T) (TypedExpr, error) { - return nil, pgerror.NewErrorf(pgerror.CodeInternalError, - "programmer error: column access expressions must be replaced before type checking") + subExpr, err := expr.Expr.TypeCheck(ctx, desired) + if err != nil { + return nil, err + } + expr.Expr = subExpr + resolvedType := subExpr.ResolvedType() + + if !resolvedType.FamilyEqual(types.FamTuple) { + return nil, pgerror.NewErrorf(pgerror.CodeDatatypeMismatchError, + "type %s is not composite", resolvedType) + } + + if expr.Star { + return nil, pgerror.NewErrorf(pgerror.CodeDatatypeMismatchError, + "star expansion of tuples is not supported") + } + + // Ensure that the tuple is indeed labeled. + if len(resolvedType.(types.TTuple).Labels) == 0 { + return nil, pgerror.NewErrorf(pgerror.CodeDatatypeMismatchError, + "type %s is not composite", resolvedType) + } + + // Go through all of the labels to find a match. + expr.ColIndex = -1 + for i, label := range resolvedType.(types.TTuple).Labels { + if label == expr.ColName { + expr.ColIndex = i + break + } + } + + if expr.ColIndex < 0 { + return nil, pgerror.NewErrorf(pgerror.CodeDatatypeMismatchError, + "could not identify column \"%s\" in %s", expr.ColName, resolvedType) + } + + expr.typ = expr.Expr.(*Tuple).types + return expr, nil } // TypeCheck implements the Expr interface. @@ -826,7 +863,7 @@ func (expr *StrVal) TypeCheck(ctx *SemaContext, desired types.T) (TypedExpr, err // TypeCheck implements the Expr interface. func (expr *Tuple) TypeCheck(ctx *SemaContext, desired types.T) (TypedExpr, error) { - // If there are labels, make sure there are the correct number. + // Ensure the number of labels matches the number of expressions. if len(expr.Labels) > 0 && len(expr.Labels) != len(expr.Exprs) { return nil, pgerror.NewErrorf(pgerror.CodeSyntaxError, "the number of expressions in a labeled tuple (%d) must match the number of labels (%d)", @@ -849,9 +886,18 @@ func (expr *Tuple) TypeCheck(ctx *SemaContext, desired types.T) (TypedExpr, erro } // Copy the labels if there are any. if len(expr.Labels) > 0 { + // Ensure that there are no repeat labels. + uniqueLabels := map[string]struct{}{} expr.types.Labels = make([]string, len(expr.Labels)) for i, label := range expr.Labels { - expr.types.Labels[i] = label.Normalize() + normalizedLabel := label.Normalize() + if _, ok := uniqueLabels[normalizedLabel]; ok { + return nil, pgerror.NewErrorf(pgerror.CodeSyntaxError, + "each label in a tuple must be unique (%q)", expr.Labels, + ) + } + uniqueLabels[normalizedLabel] = struct{}{} + expr.types.Labels[i] = normalizedLabel } } return expr, nil diff --git a/pkg/sql/sem/tree/type_check_test.go b/pkg/sql/sem/tree/type_check_test.go index d9f372b9dfbb..4300f5479563 100644 --- a/pkg/sql/sem/tree/type_check_test.go +++ b/pkg/sql/sem/tree/type_check_test.go @@ -141,6 +141,10 @@ func TestTypeCheck(t *testing.T) { {`(ROW (1,2))`, `ROW(1:::INT, 2:::INT)`}, {`ROW(1:::INT, 2:::INT)`, `ROW(1:::INT, 2:::INT)`}, + {`((ROW (1) AS a)).a`, `((ROW(1:::INT) AS a)).a`}, + {`((('1', 2) AS a, b)).a`, `((('1':::STRING, 2:::INT) AS a, b)).a`}, + {`((('1', 2) AS a, b)).b`, `((('1':::STRING, 2:::INT) AS a, b)).b`}, + // These outputs, while bizarre looking, are correct and expected. The // type annotation is caused by the call to tree.Serialize, which formats the // output using the Parseable formatter which inserts type annotations @@ -214,9 +218,6 @@ func TestTypeCheckError(t *testing.T) { {`ANNOTATE_TYPE(ANNOTATE_TYPE(1, int), decimal)`, `incompatible type annotation for ANNOTATE_TYPE(1, INT) as decimal, found type: int`}, {`3:::int[]`, `incompatible type annotation for 3 as int[], found type: int`}, - {`((unnest(ARRAY[]:::int[])).*)`, `column access expressions must be replaced before type checking`}, - {`((unnest(ARRAY[]:::int[])).unnest)`, `column access expressions must be replaced before type checking`}, - { `((1,2) AS a)`, `the number of expressions in a labeled tuple (2) must match the number of labels (1)`, @@ -225,6 +226,31 @@ func TestTypeCheckError(t *testing.T) { `(ROW (1) AS a,b)`, `the number of expressions in a labeled tuple (1) must match the number of labels (2)`, }, + { + `((1,2) AS a,a)`, + `each label in a tuple must be unique (["a" "a"])`, + }, + { + `((1,2,3) AS a,b,a)`, + `each label in a tuple must be unique (["a" "b" "a"])`, + }, + { + `((ROW (1, '2') AS a,b)).x`, + `could not identify column "x" in tuple{int AS a, string AS b}`, + }, + { + `(((1, '2') AS a,b)).x`, + `could not identify column "x" in tuple{int AS a, string AS b}`, + }, + { + `((ROW (1) AS a)).*`, + `star expansion of tuples is not supported`, + }, + { + `((('1', 2) AS a, b)).*`, + `star expansion of tuples is not supported`, + }, + // TODO(bram): same label failure case here ******** } for _, d := range testData { expr, err := parser.ParseExpr(d.expr)