builtins: automatically add builtins for each type cast

[otan]

In PG, casts from one type to another can also be use the function syntax, e.g. date(now()) = now()::date. This is done at type resolution time.

Unfortunately we do not support that in type resolution, and from experience long ago it was tricky to do so (happy to be proven wrong).

This change instead defines a builtin for each castable type, which emulates the same behavior. We already kind of do this for oid and inet, so this isn't much worse right?

Release note (sql change): Each type cast is now expressable as a function, e.g. now()::date can be expressed as date(now()).

Resolves #97067

[cockroach-teamcity]

This change is 

[rafiss]

this is cool! did you also see #94859

there's one note about PG though -- if i recall correctly, in postgres each cast is implemented by calling the respective builtin function. this PR is going the other way though: each builtin function that is being added here is implemented by calling into the cast code. i'm not sure how much this distinction matters. one thing that could be related is in the pg_cast table, we should start filling out the castfunc column

[otan]

there's one note about PG though -- if i recall correctly, in postgres each cast is implemented by calling the respective builtin function.

that's true; ideally we make this work in the type resolution layer like pg does:

If no exact match is found, see if the function call appears to be a special type conversion request. This happens if the function call has just one argument and the function name is the same as the (internal) name of some data type. Furthermore, the function argument must be either an unknown-type literal, or a type that is binary-coercible to the named data type, or a type that could be converted to the named data type by applying that type's I/O functions (that is, the conversion is either to or from one of the standard string types). When these conditions are met, the function call is treated as a form of CAST specification. [11]

i don't recall it being easy to put this into type resolution (pls prove me wrong :D).

---

this is also annoying as this PR will probably tie us in deeper to the int8/float8 by default instead of int4/float4. basically to make this work:

query error OID out of range: -2147483649
SELECT oid(-2147483649)

[mgartner]

i don't recall it being easy to put this into type resolution (pls prove me wrong :D).

If you define the builtins, I wonder if the function resolution we have will work well enough. It's unlikely to work 100% consistently with Postgres until we rewrite our function/type resolution (see #75101 and ##88374 (comment)).

[otan]

If you define the builtins, I wonder if the function resolution we have will work well enough.

based on my manual tests + my random test for casts it works good enough for now.

It's unlikely to work 100% consistently with Postgres until we rewrite our function/type resolution

we'll get there one day. can't say i didn't try :P

[cucaroach]

Seems fine to me but this is a a little outside my purview, would like to defer to Rafi or Marcus.

Reviewed 8 of 13 files at r1, all commit messages.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @otan)

---

pkg/internal/sqlsmith/schema.go line 500 at r1 (raw file):

		if n := tree.Name(def.Name); n.String() != def.Name {
			// sqlsmith doesn't seem to know how to quote these.

Whats going on here?

[mgartner]

Reviewed 4 of 13 files at r1.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @cucaroach and @otan)

---

pkg/sql/sem/builtins/pg_builtins.go line 200 at r1 (raw file):

	}
	for toOID, def := range castBuiltins {
		n := strings.ToLower(types.OidToType[toOID].SQLString())

I don't think SQLString works here - Postgres's naming of these functions doesn't make the SQLString naming of a type. For example, varchar, bit, decimal, and char don't seem to be functions in Postgres. It looks like they might exist, but with different names. For example bpchar seems to work for the char type:

marcus=# select pg_typeof('foo'::CHAR), pg_typeof(bpchar('foo'));
 pg_typeof | pg_typeof
-----------+-----------
 character | character
(1 row)

---

pkg/sql/sem/builtins/cast_test.go line 68 at r1 (raw file):

	switch typ.Oid() {
	case oid.T_char:
		return "char"

oid.T_char represents the "char" type, not to be confused with the CHAR type:

marcus=# SELECT pg_typeof('foo'::CHAR), pg_typeof('foo'::"char"), pg_typeof("char"('foo'));
 pg_typeof | pg_typeof | pg_typeof
-----------+-----------+-----------
 character | "char"    | "char"
(1 row)

So you'll want to keep the double quotes in the query, e.g., SELECT "char"(...) instead of SELECT char(...). I think typ.SQLString already includes the double quotes for this type.

[otan]

pkg/sql/sem/builtins/pg_builtins.go line 200 at r1 (raw file):

Previously, mgartner (Marcus Gartner) wrote…

I don't think SQLString works here - Postgres's naming of these functions doesn't make the SQLString naming of a type. For example, varchar, bit, decimal, and char don't seem to be functions in Postgres. It looks like they might exist, but with different names. For example bpchar seems to work for the char type:

marcus=# select pg_typeof('foo'::CHAR), pg_typeof(bpchar('foo'));
 pg_typeof | pg_typeof
-----------+-----------
 character | character
(1 row)

i think this is still right, you just need to quote the name.

otan=# select "varbit"('0100');
 varbit
--------
 0100
(1 row)

otan=# select "numeric"(1234::int);
 numeric
---------
    1234
(1 row)

otan=# select "char"(96::int);
 char
------
 `
(1 row)

[otan]

pkg/sql/sem/builtins/cast_test.go line 68 at r1 (raw file):

Previously, mgartner (Marcus Gartner) wrote…

oid.T_char represents the "char" type, not to be confused with the CHAR type:

marcus=# SELECT pg_typeof('foo'::CHAR), pg_typeof('foo'::"char"), pg_typeof("char"('foo'));
 pg_typeof | pg_typeof | pg_typeof
-----------+-----------+-----------
 character | "char"    | "char"
(1 row)

So you'll want to keep the double quotes in the query, e.g., SELECT "char"(...) instead of SELECT char(...). I think typ.SQLString already includes the double quotes for this type.

no this is right, tree.Name("char").String() makes it a "char".

[otan]

pkg/sql/sem/builtins/cast_test.go line 68 at r1 (raw file):

Previously, otan (Oliver Tan) wrote…

no this is right, tree.Name("char").String() makes it a "char".

ah nah good catch, had to rearrange a few things around.

[otan]

pkg/sql/sem/builtins/cast_test.go line 68 at r1 (raw file):

Previously, otan (Oliver Tan) wrote…

ah nah good catch, had to rearrange a few things around.

(sorry i mean, this is still the same, but bpchar / numeric was a surprise)

[rafiss]

nit: can you fill in the castfunc column of pg_cast?

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

---

docs/generated/sql/functions.md line 361 at r2 (raw file):

</table>

### Cast functions

i'd actually be in favor of marking all these as hidden. what do you think?

[otan]

hmm, castfunc would be "wrong" though compared to PG. is that ok?

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @cucaroach, @mgartner, and @rafiss)

---

docs/generated/sql/functions.md line 361 at r2 (raw file):

Previously, rafiss (Rafi Shamim) wrote…

i'd actually be in favor of marking all these as hidden. what do you think?

Done.

[rafiss]

wrong in which way? you mean it would have different function OIDs? i think that's fine -- the OID would still refer to the correct function in pg_proc.

[otan]

hmm not so good for something like castfunc::regproc - i think i prefer NULL for now until we rehaul our type overload system to make this point the correct way (i.e. casts call funcs, not funcs being derived from casts)

[rafiss]

i still don't follow. what's wrong with castfunc::regproc? i think if you shared the error or incorrect result i would understand better

[otan]

ah, nvm, i see what you're getting at.

[otan]

hmm it's a little tricky because we defined casts on all oids, but the functions derived is in actuality based on the family.

[otan]

(changes made + ready for another look!)

[rafiss]

thanks for adding this!

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

---

pkg/sql/sem/builtins/fixed_oids.go line 2048 at r6 (raw file):

	2070: `crdb_internal.num_inverted_index_entries(val: tsvector, version: int) -> int`,
	2072: `crdb_internal.upsert_dropped_relation_gc_ttl(desc_id: int, gc_ttl: interval) -> bool`,
	2073: `box2d(geometry: geometry) -> box2d`,

just wanna confirm - so if someone adds a new type later on, they'll get an init error telling them they need to add an entry into this fixed OID map?

[otan]

bors r=rafiss

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @cucaroach, @mgartner, and @rafiss)

---

pkg/sql/sem/builtins/fixed_oids.go line 2048 at r6 (raw file):

Previously, rafiss (Rafi Shamim) wrote…

just wanna confirm - so if someone adds a new type later on, they'll get an init error telling them they need to add an entry into this fixed OID map?

yep!

[craig]

👎 Rejected by code reviews

outdated

[otan]

bors r=rafiss

[craig]

Build succeeded:

• Bazel Essential CI (Cockroach)