From b29728db38b6539aad1bd3691781483f20992594 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 +- .../logictest/testdata/logic_test/generators | 20 +++- pkg/sql/logictest/testdata/logic_test/tuple | 94 ++++++++++++++++++- 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 | 3 +- pkg/sql/sem/tree/type_check.go | 64 +++++++++++-- pkg/sql/sem/tree/type_check_test.go | 35 ++++++- 9 files changed, 205 insertions(+), 24 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/generators b/pkg/sql/logictest/testdata/logic_test/generators index 3951db613915..a3ade632c980 100644 --- a/pkg/sql/logictest/testdata/logic_test/generators +++ b/pkg/sql/logictest/testdata/logic_test/generators @@ -469,18 +469,30 @@ a 3 subtest srf_accessor -query error pq: unimplemented: access to field in composite expression: "1" +query error pq: type int is not composite SELECT (1).* -query error pq: unimplemented: access to field in composite expression: "1" +query error pq: type int is not composite +SELECT ((1)).* + +query error pq: type int is not composite SELECT (1).x -query error pq: unimplemented: access to field in composite expression: "'a'" +query error pq: type int is not composite +SELECT ((1)).x + +query error pq: type string is not composite SELECT ('a').* -query error pq: unimplemented: access to field in composite expression: "'a'" +query error pq: type string is not composite +SELECT (('a')).* + +query error pq: type string is not composite SELECT ('a').x +query error pq: type string is not composite +SELECT (('a')).x + query error pq: unnest\(\): cannot determine type of empty array. Consider annotating with the desired type, for example ARRAY\[\]:::int\[\] SELECT (unnest(ARRAY[])).* diff --git a/pkg/sql/logictest/testdata/logic_test/tuple b/pkg/sql/logictest/testdata/logic_test/tuple index 08ceb84f2eeb..2f9761c154d6 100644 --- a/pkg/sql/logictest/testdata/logic_test/tuple +++ b/pkg/sql/logictest/testdata/logic_test/tuple @@ -793,9 +793,97 @@ 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: mismatch in tuple definition: 2 expressions, 1 labels +SELECT ((1, '2') AS a) + +query error pq: mismatch in tuple definition: 1 expressions, 2 labels +SELECT (ROW(1) AS a, b) + +# Tuple labels must be different +query error pq: found duplicate tuple label: "a, a" +SELECT ((1, '2') AS a, a) + +query error pq: found duplicate tuple label: "a, a, b" +SELECT ((1, '2', true) AS a, a, b) + +query error pq: found duplicate tuple label: "a, b, a" +SELECT ((1, '2', true) AS a, b, a) + +query error pq: found duplicate tuple label: "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 + +## base working case + +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 + +## A collection of error cases + +# column doesn't exist +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 + +# Missing extra parentheses +query error pq: syntax error at or near "." +SELECT ((1,2,3) AS a,b,c).x + +query error pq: syntax error at or near "." +SELECT ((1,2,3) AS a,b,c).* + +# No labels +query error pq: type tuple{int, int, int} is not composite +SELECT ((1,2,3)).x + +query error pq: type tuple{int, int, int} is not composite +SELECT ((1,2,3)).* + +# Non unique labels +query error pq: found duplicate tuple label: "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 cf8fabd2a3d7..5a550c203fe8 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 008c23307194..9078139a9ace 100644 --- a/pkg/sql/sem/tree/eval.go +++ b/pkg/sql/sem/tree/eval.go @@ -3396,7 +3396,8 @@ 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") + "programmer error: column access expressions must be replaced before evaluation", + ) } // Eval implements the TypedExpr interface. diff --git a/pkg/sql/sem/tree/type_check.go b/pkg/sql/sem/tree/type_check.go index ff38e6f92709..a38cbea75209 100644 --- a/pkg/sql/sem/tree/type_check.go +++ b/pkg/sql/sem/tree/type_check.go @@ -390,8 +390,49 @@ 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, + ) + } + + labels := resolvedType.(types.TTuple).Labels + // Ensure that the tuple is indeed labeled. + if len(labels) == 0 { + 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", + ) + } + + // Go through all of the labels to find a match. + colIndex := -1 + for i, label := range labels { + if label == expr.ColName { + colIndex = i + break + } + } + + if colIndex < 0 { + return nil, pgerror.NewErrorf(pgerror.CodeDatatypeMismatchError, + "could not identify column \"%s\" in %s", expr.ColName, resolvedType, + ) + } + + return expr.Expr.(*Tuple).Exprs[colIndex].(TypedExpr), nil } // TypeCheck implements the Expr interface. @@ -826,10 +867,10 @@ 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)", + "mismatch in tuple definition: %d expressions, %d labels", len(expr.Exprs), len(expr.Labels), ) } @@ -849,9 +890,20 @@ 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. + for i := range expr.Labels { + for j := 0; j < i; j++ { + if expr.Labels[i] == expr.Labels[j] { + return nil, pgerror.NewErrorf(pgerror.CodeSyntaxError, + "found duplicate tuple label: %q", ErrString(&expr.Labels), + ) + } + } + } + expr.types.Labels = make([]string, len(expr.Labels)) - for i, label := range expr.Labels { - expr.types.Labels[i] = label.Normalize() + for i := range expr.Labels { + expr.types.Labels[i] = expr.Labels[i].Normalize() } } 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..2701e2dcb962 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`, `1:::INT`}, + {`((('1', 2) AS a, b)).a`, `'1':::STRING`}, + {`((('1', 2) AS a, b)).b`, `2:::INT`}, + // 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,16 +218,37 @@ 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)`, + `mismatch in tuple definition: 2 expressions, 1 labels`, }, { `(ROW (1) AS a,b)`, - `the number of expressions in a labeled tuple (1) must match the number of labels (2)`, + `mismatch in tuple definition: 1 expressions, 2 labels`, + }, + { + `((1,2) AS a,a)`, + `duplicate tuple label: "a, a"`, + }, + { + `((1,2,3) AS a,b,a)`, + `duplicate tuple label: "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`, }, } for _, d := range testData {