From 8d0c95bff306f6581a7db75c84c0e461a68e2463 Mon Sep 17 00:00:00 2001 From: Raphael 'kena' Poss Date: Wed, 2 Dec 2020 02:33:08 +0100 Subject: [PATCH] sql: fix some type / string conversion brokenness This ensures that e.g. `drop type "a+b"` works if there is a type indeed named `a+b`. Release note (bug fix): DROP TYPE and certain other statements that work over SQL scalar types now properly support type names containing special characters. --- pkg/internal/sqlsmith/schema.go | 2 +- pkg/internal/sqlsmith/type.go | 6 +- pkg/sql/faketreeeval/evalctx.go | 4 +- .../testdata/logic_test/builtin_function | 8 +- .../logictest/testdata/logic_test/drop_type | 13 +++ .../logictest/testdata/logic_test/grant_type | 19 +++-- .../logictest/testdata/logic_test/pgoidtype | 8 ++ pkg/sql/opt/optgen/exprgen/parse_type.go | 2 +- pkg/sql/opt/testutils/testcat/test_catalog.go | 2 +- pkg/sql/parser/BUILD.bazel | 1 - pkg/sql/parser/parse.go | 84 +++++++++++-------- pkg/sql/parser/parse_test.go | 18 ---- pkg/sql/planner.go | 6 +- .../schemachange/alter_column_type_test.go | 2 +- pkg/sql/sem/tree/datum.go | 2 +- pkg/sql/sem/tree/eval.go | 7 +- pkg/sql/stats/json.go | 4 +- pkg/sql/virtual_schema.go | 2 +- 18 files changed, 113 insertions(+), 77 deletions(-) diff --git a/pkg/internal/sqlsmith/schema.go b/pkg/internal/sqlsmith/schema.go index 3dc20f9a3f7c..c6c48d5b4286 100644 --- a/pkg/internal/sqlsmith/schema.go +++ b/pkg/internal/sqlsmith/schema.go @@ -314,7 +314,7 @@ ORDER BY currentCols = nil } - coltyp, err := s.typeFromName(typ) + coltyp, err := s.typeFromSQLTypeSyntax(typ) if err != nil { return nil, err } diff --git a/pkg/internal/sqlsmith/type.go b/pkg/internal/sqlsmith/type.go index 768d7f52d5aa..04f3ea53d3d7 100644 --- a/pkg/internal/sqlsmith/type.go +++ b/pkg/internal/sqlsmith/type.go @@ -21,10 +21,10 @@ import ( "github.com/lib/pq/oid" ) -func (s *Smither) typeFromName(name string) (*types.T, error) { - typRef, err := parser.ParseType(name) +func (s *Smither) typeFromSQLTypeSyntax(typeStr string) (*types.T, error) { + typRef, err := parser.GetTypeFromValidSQLSyntax(typeStr) if err != nil { - return nil, errors.AssertionFailedf("failed to parse type: %v", name) + return nil, errors.AssertionFailedf("failed to parse type: %v", typeStr) } typ, err := tree.ResolveType(context.Background(), typRef, s) if err != nil { diff --git a/pkg/sql/faketreeeval/evalctx.go b/pkg/sql/faketreeeval/evalctx.go index a04edec6158a..be9aa5d27eed 100644 --- a/pkg/sql/faketreeeval/evalctx.go +++ b/pkg/sql/faketreeeval/evalctx.go @@ -137,8 +137,8 @@ func (ep *DummyEvalPlanner) ResolveTableName( return 0, errors.WithStack(errEvalPlanner) } -// ParseType is part of the tree.EvalPlanner interface. -func (ep *DummyEvalPlanner) ParseType(sql string) (*types.T, error) { +// GetTypeFromValidSQLSyntax is part of the tree.EvalPlanner interface. +func (ep *DummyEvalPlanner) GetTypeFromValidSQLSyntax(sql string) (*types.T, error) { return nil, errors.WithStack(errEvalPlanner) } diff --git a/pkg/sql/logictest/testdata/logic_test/builtin_function b/pkg/sql/logictest/testdata/logic_test/builtin_function index 399a76b11abb..6676090349c2 100644 --- a/pkg/sql/logictest/testdata/logic_test/builtin_function +++ b/pkg/sql/logictest/testdata/logic_test/builtin_function @@ -1962,6 +1962,7 @@ VALUES (format_type('anyelement'::regtype, -1)), (format_type('bool'::regtype, -1)), (format_type('bytea'::regtype, -1)), (format_type('char'::regtype, -1)), + (format_type('"char"'::regtype, -1)), (format_type('date'::regtype, -1)), (format_type('decimal'::regtype, -1)), (format_type('float'::regtype, -1)), @@ -1995,13 +1996,16 @@ VALUES (format_type('anyelement'::regtype, -1)), (format_type('int[]'::regtype, -1)), (format_type('int2[]'::regtype, -1)), (format_type('string[]'::regtype, -1)), - (format_type('varchar[]'::regtype, -1)) + (format_type('varchar[]'::regtype, -1)), + (format_type('pg_catalog.int4'::regtype, -1)), + (format_type('pg_catalog.int2'::regtype, -1)) ---- anyelement bit boolean bytea bpchar +"char" date numeric double precision @@ -2036,6 +2040,8 @@ bigint[] smallint[] text[] character varying[] +integer +smallint query T VALUES (format_type('anyelement'::regtype, NULL)), diff --git a/pkg/sql/logictest/testdata/logic_test/drop_type b/pkg/sql/logictest/testdata/logic_test/drop_type index 01bbb8b65449..c8e97bf0f2ed 100644 --- a/pkg/sql/logictest/testdata/logic_test/drop_type +++ b/pkg/sql/logictest/testdata/logic_test/drop_type @@ -338,3 +338,16 @@ DROP DATABASE d query I SELECT id FROM system.namespace WHERE name LIKE 'd_t%' + +subtest regression_57187 + +statement ok +CREATE DATABASE d; +CREATE TYPE d."a