release-20.2: sql: fix some type / string conversion brokenness

pkg/internal/sqlsmith/schema.go

@@ -286,7 +286,7 @@ ORDER BY
 			currentCols = nil
 		}
 
-		coltyp, err := s.typeFromName(typ)
+		coltyp, err := s.typeFromSQLTypeSyntax(typ)
 		if err != nil {
 			return nil, err
 		}

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 {

pkg/sql/faketreeeval/evalctx.go

@@ -136,8 +136,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)
 }
 

pkg/sql/logictest/testdata/logic_test/builtin_function

@@ -1927,6 +1927,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)),
@@ -1960,13 +1961,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
+bpchar
 date
 numeric
 double precision
@@ -2001,6 +2005,8 @@ bigint[]
 smallint[]
 text[]
 character varying[]
+integer
+smallint
 
 query T
 VALUES (format_type('anyelement'::regtype, NULL)),

pkg/sql/logictest/testdata/logic_test/drop_type

@@ -333,3 +333,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<b" AS ENUM('hello');
+CREATE TYPE d."b+c" AS ENUM('hello')
+
+statement ok
+DROP TYPE d."b+c"
+
+statement ok
+DROP DATABASE d

pkg/sql/logictest/testdata/logic_test/grant_type

@@ -10,27 +10,35 @@ statement ok
 CREATE TYPE public.enum_a AS ENUM ('hello', 'goodbye');
 GRANT USAGE ON TYPE public.enum_a TO user1
 
+statement ok
+CREATE TYPE public."enum_a+b" AS ENUM ('hello', 'goodbye');
+GRANT USAGE ON TYPE public."enum_a+b" TO user1
+
 statement ok
 CREATE TYPE schema_a.enum_b AS ENUM ('hi', 'bye');
 GRANT ALL ON TYPE schema_a.enum_b TO user1
 
 query TTTTT colnames
-SHOW GRANTS ON TYPE schema_a.enum_b, enum_a, int
+SHOW GRANTS ON TYPE schema_a.enum_b, "enum_a+b", enum_a, int4
 ----
 database_name  schema_name  type_name  grantee  privilege_type
-test           pg_catalog   int        admin    ALL
-test           pg_catalog   int        public   USAGE
-test           pg_catalog   int        root     ALL
+test           pg_catalog   int4       admin    ALL
+test           pg_catalog   int4       public   USAGE
+test           pg_catalog   int4       root     ALL
 test           public       enum_a     admin    ALL
 test           public       enum_a     public   USAGE
 test           public       enum_a     root     ALL
 test           public       enum_a     user1    USAGE
+test           public       enum_a+b   admin    ALL
+test           public       enum_a+b   public   USAGE
+test           public       enum_a+b   root     ALL
+test           public       enum_a+b   user1    USAGE
 test           schema_a     enum_b     admin    ALL
 test           schema_a     enum_b     root     ALL
 test           schema_a     enum_b     user1    ALL
 
 query TTTTT colnames
-SHOW GRANTS ON TYPE schema_a.enum_b, enum_a, int FOR user1
+SHOW GRANTS ON TYPE schema_a.enum_b, enum_a, int4 FOR user1
 ----
 database_name  schema_name  type_name  grantee  privilege_type
 test           public       enum_a     user1    USAGE
@@ -41,6 +49,7 @@ SHOW GRANTS FOR user1
 ----
 database_name  schema_name  relation_name  grantee  privilege_type
 test           public       enum_a         user1    USAGE
+test           public       enum_a+b       user1    USAGE
 test           schema_a     enum_b         user1    ALL
 
 statement error type "non_existent" does not exist

pkg/sql/logictest/testdata/logic_test/pgoidtype

@@ -131,6 +131,11 @@ SELECT 'numeric(10,3)'::REGTYPE, 'numeric( 10, 3 )'::REGTYPE
 ----
 numeric  numeric
 
+query OO
+SELECT '"char"'::REGTYPE, 'pg_catalog.int4'::REGTYPE
+----
+character  integer
+
 query error type 'foo.' does not exist
 SELECT 'foo.'::REGTYPE
 
@@ -149,6 +154,9 @@ SELECT 'blah'::REGNAMESPACE
 query error type 'blah' does not exist
 SELECT 'blah'::REGTYPE
 
+query error type 'pg_catalog.int' does not exist
+SELECT 'pg_catalog.int'::REGTYPE
+
 ## Test other cast syntaxes
 
 query O

pkg/sql/opt/optgen/exprgen/parse_type.go

@@ -42,7 +42,7 @@ func ParseType(typeStr string) (*types.T, error) {
 		}
 		return types.MakeTuple(colTypes), nil
 	}
-	typ, err := parser.ParseType(typeStr)
+	typ, err := parser.GetTypeFromValidSQLSyntax(typeStr)
 	if err != nil {
 		return nil, err
 	}

pkg/sql/opt/testutils/testcat/test_catalog.go

@@ -968,7 +968,7 @@ func (ts *TableStat) Histogram() []cat.HistogramBucket {
 	if ts.js.HistogramColumnType == "" || ts.js.HistogramBuckets == nil {
 		return nil
 	}
-	colTypeRef, err := parser.ParseType(ts.js.HistogramColumnType)
+	colTypeRef, err := parser.GetTypeFromValidSQLSyntax(ts.js.HistogramColumnType)
 	if err != nil {
 		panic(err)
 	}

pkg/sql/parser/parse.go

@@ -256,8 +256,13 @@ func HasMultipleStatements(sql string) bool {
 	return false
 }
 
-// ParseQualifiedTableName parses a SQL string of the form
-// `[ database_name . ] [ schema_name . ] table_name`.
+// ParseQualifiedTableName parses a possibly qualified table name. The
+// table name must contain one or more name parts, using the full
+// input SQL syntax: each name part containing special characters, or
+// non-lowercase characters, must be enclosed in double quote. The
+// name may not be an invalid table name (the caller is responsible
+// for guaranteeing that only valid table names are provided as
+// input).
 func ParseQualifiedTableName(sql string) (*tree.TableName, error) {
 	name, err := ParseTableName(sql)
 	if err != nil {
@@ -267,7 +272,12 @@ func ParseQualifiedTableName(sql string) (*tree.TableName, error) {
 	return &tn, nil
 }
 
-// ParseTableName parses a table name.
+// ParseTableName parses a table name. The table name must contain one
+// or more name parts, using the full input SQL syntax: each name
+// part containing special characters, or non-lowercase characters,
+// must be enclosed in double quote. The name may not be an invalid
+// table name (the caller is responsible for guaranteeing that only
+// valid table names are provided as input).
 func ParseTableName(sql string) (*tree.UnresolvedObjectName, error) {
 	// We wrap the name we want to parse into a dummy statement since our parser
 	// can only parse full statements.
@@ -292,6 +302,8 @@ func ParseTableName(sql string) (*tree.UnresolvedObjectName, error) {
 // around this limitation by parsing the input table name as a column name
 // with a fake non-keyword prefix, and then shifting the result down into an
 // UnresolvedObjectName.
+//
+// TODO(knz): This function is obsolete (only used in cockroach dump). Remove it.
 func ParseTableNameWithQualifiedNames(sql string) (*tree.UnresolvedObjectName, error) {
 	stmt, err := ParseOne(fmt.Sprintf("SELECT fakeprefix.%s", sql))
 	if err != nil {
@@ -319,15 +331,22 @@ func parseExprs(exprs []string) (tree.Exprs, error) {
 	return set.Values, nil
 }
 
-// ParseExprs is a short-hand for parseExprs(sql)
+// ParseExprs parses a comma-delimited sequence of SQL scalar
+// expressions. The caller is responsible for ensuring that the input
+// is, in fact, a comma-delimited sequence of SQL scalar expressions —
+// the results are undefined if the string contains invalid SQL
+// syntax.
 func ParseExprs(sql []string) (tree.Exprs, error) {
 	if len(sql) == 0 {
 		return tree.Exprs{}, nil
 	}
 	return parseExprs(sql)
 }
 
-// ParseExpr is a short-hand for parseExprs([]string{sql})
+// ParseExpr parses a SQL scalar expression. The caller is responsible
+// for ensuring that the input is, in fact, a valid SQL scalar
+// expression — the results are undefined if the string contains
+// invalid SQL syntax.
 func ParseExpr(sql string) (tree.Expr, error) {
 	exprs, err := parseExprs([]string{sql})
 	if err != nil {
@@ -339,8 +358,29 @@ func ParseExpr(sql string) (tree.Expr, error) {
 	return exprs[0], nil
 }
 
-// ParseType parses a column type.
-func ParseType(sql string) (tree.ResolvableTypeReference, error) {
+// GetTypeReferenceFromName turns a type name into a type
+// reference. This supports only “simple” (single-identifier)
+// references to built-in types, when the identifer has already been
+// parsed away from the input SQL syntax.
+func GetTypeReferenceFromName(typeName tree.Name) (tree.ResolvableTypeReference, error) {
+	expr, err := ParseExpr(fmt.Sprintf("1::%s", typeName.String()))
+	if err != nil {
+		return nil, err
+	}
+
+	cast, ok := expr.(*tree.CastExpr)
+	if !ok {
+		return nil, errors.AssertionFailedf("expected a tree.CastExpr, but found %T", expr)
+	}
+
+	return cast.Type, nil
+}
+
+// GetTypeFromValidSQLSyntax retrieves a type from its SQL syntax. The caller is
+// responsible for guaranteeing that the type expression is valid
+// SQL. This includes verifying that complex identifiers are enclosed
+// in double quotes, etc.
+func GetTypeFromValidSQLSyntax(sql string) (tree.ResolvableTypeReference, error) {
 	expr, err := ParseExpr(fmt.Sprintf("1::%s", sql))
 	if err != nil {
 		return nil, err

pkg/sql/planner.go

@@ -469,10 +469,10 @@ func (p *planner) DistSQLPlanner() *DistSQLPlanner {
 	return p.extendedEvalCtx.DistSQLPlanner
 }
 
-// ParseType implements the tree.EvalPlanner interface.
+// GetTypeFromValidSQLSyntax implements the tree.EvalPlanner interface.
 // We define this here to break the dependency from eval.go to the parser.
-func (p *planner) ParseType(sql string) (*types.T, error) {
-	ref, err := parser.ParseType(sql)
+func (p *planner) GetTypeFromValidSQLSyntax(sql string) (*types.T, error) {
+	ref, err := parser.GetTypeFromValidSQLSyntax(sql)
 	if err != nil {
 		return nil, err
 	}

pkg/sql/schemachange/alter_column_type_test.go

@@ -38,7 +38,7 @@ func TestColumnConversions(t *testing.T) {
 	defer leaktest.AfterTest(t)()
 
 	columnType := func(typStr string) *types.T {
-		t, err := parser.ParseType(typStr)
+		t, err := parser.GetTypeFromValidSQLSyntax(typStr)
 		if err != nil {
 			panic(err)
 		}

pkg/sql/sem/tree/casts.go

@@ -1118,7 +1118,7 @@ func PerformCast(ctx *EvalContext, d Datum, t *types.T) (Datum, error) {
 				}
 				return queryOid(ctx, t, NewDString(funcDef.Name))
 			case oid.T_regtype:
-				parsedTyp, err := ctx.Planner.ParseType(s)
+				parsedTyp, err := ctx.Planner.GetTypeFromValidSQLSyntax(s)
 				if err == nil {
 					return &DOid{
 						semanticType: t,

pkg/sql/sem/tree/eval.go

@@ -2979,8 +2979,11 @@ type EvalDatabase interface {
 type EvalPlanner interface {
 	EvalDatabase
 	TypeReferenceResolver
-	// ParseType parses a column type.
-	ParseType(sql string) (*types.T, error)
+
+	// GetTypeFromValidSQLSyntax parses a column type when the input
+	// string uses the parseable SQL representation of a type name, e.g.
+	// `INT(13)`, `mytype`, `"mytype"`, `pg_catalog.int4` or `"public".mytype`.
+	GetTypeFromValidSQLSyntax(sql string) (*types.T, error)
 
 	// EvalSubquery returns the Datum for the given subquery node.
 	EvalSubquery(expr *Subquery) (Datum, error)

pkg/sql/stats/json.go

@@ -33,7 +33,7 @@ type JSONStatistic struct {
 	NullCount     uint64   `json:"null_count"`
 	// HistogramColumnType is the string representation of the column type for the
 	// histogram (or unset if there is no histogram). Parsable with
-	// tree.ParseType.
+	// tree.GetTypeFromValidSQLSyntax.
 	HistogramColumnType string            `json:"histo_col_type"`
 	HistogramBuckets    []JSONHistoBucket `json:"histo_buckets,omitempty"`
 }
@@ -106,7 +106,7 @@ func (js *JSONStatistic) GetHistogram(
 		return nil, nil
 	}
 	h := &HistogramData{}
-	colTypeRef, err := parser.ParseType(js.HistogramColumnType)
+	colTypeRef, err := parser.GetTypeFromValidSQLSyntax(js.HistogramColumnType)
 	if err != nil {
 		return nil, err
 	}

pkg/sql/virtual_schema.go

@@ -349,7 +349,7 @@ func (v virtualSchemaEntry) GetObjectByName(
 		// invalid input type like "notatype" will be parsed successfully as
 		// a ResolvableTypeReference, so the error here does not need to be
 		// intercepted and inspected.
-		typRef, err := parser.ParseType(name)
+		typRef, err := parser.GetTypeReferenceFromName(tree.Name(name))
 		if err != nil {
 			return nil, err
 		}