sql: fix some type / string conversion brokenness

[knz]

Fixes #57347
Fixes #57187

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.

[cockroach-teamcity]

This change is 

[knz]

@otan I could use some help here: this Pr introduces a regression:

select 'pg_catalog.int'::regtype; 

previously returns "bigint" and now returns an error "type unknown".

I checked and verified the following:

• the regtype cast passes the string pg_catalog.int properly to (*planner).GetTypeFromValidSQLSyntax()
• the parse via parser.GetTypeFromValidSQLSyntax() succeeds
• the resolve logic passes everything through properly, such that the string int is presented to (*virtualSchemaEntry) GetObjectByName()

Here my track runs cold. I am not entirely sure the resolution logic is even supposed to arrive to virtualSchemaEntry for built-in types (I originally intended them to, but maybe this was done differently since I last looked).

If it does, then maybe the lookup succeeds, but then the type entry becomes lost elsewhere.

I suspect that the unresolved type name gets passed via .String() somewhere, which would be invalid.

[otan]

The problem seems to be that SELECT 1::"int" no longer works. (note this does not work in PG either)

When we had SELECT 1::int, when parsing this in sql.y, it knew to convert this to a types.T with the int type.

cockroach/pkg/sql/parser/sql.y

Lines 9411 to 9428 in 9f9c1ce

	// SQL numeric data types
	numeric:
	INT
	{
	$$.val = sqllex.(*lexer).nakedIntType
	}
	| INTEGER
	{
	$$.val = sqllex.(*lexer).nakedIntType
	}
	| SMALLINT
	{
	$$.val = types.Int2
	}
	| BIGINT
	{
	$$.val = types.Int
	}

Unfortunately, sql.y doesn't seem to understand 1::"int", which makes this broken here

cockroach/pkg/sql/virtual_schema.go

Lines 361 to 364 in 9f9c1ce

	typ, ok := tree.GetStaticallyKnownType(typRef)
	if !ok {
	return nil, nil
	}

It seems like the logic here should be wholesale replaced with some mapping of known type names -> types.T, instead of relying on the parser. OR, do not escape the text, and if there is an error with the parsing logic error out silently.

[otan]

Actually, maybe this is a good thing. This does not work on PG either:

otan=# select 'pg_catalog.int'::regtype;
ERROR:  type "pg_catalog.int" does not exist
LINE 1: select 'pg_catalog.int'::regtype;

[otan]

Reviewed 11 of 13 files at r1.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @jordanlewis, @knz, and @otan)

---

pkg/sql/sem/tree/eval.go, line 3032 at r1 (raw file):

	// GetTypeFromValidSQLSyntax parses a column type when the input
	// string uses the parseable SQL representation of a type name, e.g.
	// "INT(13)" or "pg_catalog.int".

nit: i don't think pg_catalog.int would work here as the schema separator . would be folded in, i.e. we'd be doing SELECT 1::"pg_catalog.int" which is invalid.

[otan]

(accept assumes we don't want to allow int as a type, but the user must use the full name, i.e. int4, int8... may be worth noting explicitly in the release note if that is the case)

[knz]

Actually, maybe this is a good thing. This does not work on PG either:

Aaayyy yes indeed! That is awesome. Thanks.

[knz]

@otan I changed the comment you suggested, also added some tests. PTAL.

[otan]

Reviewed 1 of 13 files at r1, 7 of 7 files at r2.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @jordanlewis and @knz)

---

pkg/sql/logictest/testdata/logic_test/drop_type, line 349 at r2 (raw file):

CREATE TYPE d."b+c" AS ENUM('hello')

query 

remove for tests to pass

---

pkg/sql/logictest/testdata/logic_test/drop_type, line 355 at r2 (raw file):

statement ok
DROP DATABASE d;

nit: remove trailing ;

---

pkg/sql/sem/tree/eval.go, line 3032 at r1 (raw file):

Previously, otan (Oliver Tan) wrote…

nit: i don't think pg_catalog.int would work here as the schema separator . would be folded in, i.e. we'd be doing SELECT 1::"pg_catalog.int" which is invalid.

remove pg_catalog. here?

[otan]

thanks for jumping onto this @knz !

[knz]

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @jordanlewis and @otan)

---

pkg/sql/logictest/testdata/logic_test/drop_type, line 349 at r2 (raw file):

Previously, otan (Oliver Tan) wrote…

remove for tests to pass

Done.

---

pkg/sql/logictest/testdata/logic_test/drop_type, line 355 at r2 (raw file):

Previously, otan (Oliver Tan) wrote…

nit: remove trailing ;

Done

---

pkg/sql/sem/tree/eval.go, line 3032 at r1 (raw file):

Previously, otan (Oliver Tan) wrote…

remove pg_catalog. here?

Nah actually it's meant to work. Added more examples.

[knz]

TFYR!

bors r=otan

[craig]

Build failed:

• GitHub CI (Cockroach)

[knz]

bors r=otan

[craig]

Build succeeded:

• GitHub CI (Cockroach)

[otan]

Shall we backport this to 20.2?

[knz]

that works for me