sql/opt: add index on pg_proc(oid) and support regproc/procedure in foldOIDFamilyCast

[e-mbrown]

Informs: #91022

Add index on pgproc(oc).
Typecast to regproc and regprocedure can use the
index on pgproc(oid).

Release note: None

[cockroach-teamcity]

This change is 

[mgartner]

Can you reword the commit/PR title so it's clear that there is more to this change than just the foldOIDFamilyCast function. The index added to the table was a surprise after reading the commit title.

[mgartner]

The tests in this file are verifying query plans to test that we can correctly build query plans, so I don't think these two tests should live here too. Perhaps pkg/sql/logictest/testdata/logic_test/cast would be a better place?

[mgartner]

I'm not convinced we need to use PerformCast here. If we use it, we'll need some more checks to ensure that the cast is valid, e.g. use cast.ValidCast, otherwise we'll be attempting to fold expressions like 12.3::FLOAT::REGPROC which I think would fail.

Can we instead use similar logic as in the case oid.T_oid: at the top of the switch?

[rafiss]

i think we should be able to use PerformCast as long as we add similar cases like the ones we have in the Regtype case

[e-mbrown]

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

---

pkg/sql/opt/exec/execbuilder/testdata/scalar line 1055 at r1 (raw file):

Previously, mgartner (Marcus Gartner) wrote…

The tests in this file are verifying query plans to test that we can correctly build query plans, so I don't think these two tests should live here too. Perhaps pkg/sql/logictest/testdata/logic_test/cast would be a better place?

A test like this exists in pkg/sql/logictest/testdata/logic_test/pgoidtype so i'll remove them.

[rafiss]

looks close!

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @e-mbrown and @mgartner)

---

pkg/sql/opt/norm/fold_constants_funcs.go line 418 at r3 (raw file):

				return nil, false, err
			}
			dOid = tree.NewDOidWithType(tree.MustBeDOid(cDatum).Oid, types.RegProc)

we should be able to replace all of this:

			dOid = tree.NewDOidWithType(tree.MustBeDOid(cDatum).Oid, types.RegProcedure)

			procName, overload, err := c.f.catalog.ResolveFunctionByOID(c.f.ctx, dOid.Oid)
			if err != nil {
				return nil, false, err
			}

			dOid = tree.NewDOidWithTypeAndName(overload.Oid, types.RegProcedure, procName)

with this:

dOid = tree.MustBeDOid(cDatum)

(The PerformCast call already takes care of resolving the function, so no need to do it again)

---

pkg/sql/opt/norm/fold_constants_funcs.go line 434 at r3 (raw file):

				return nil, false, err
			}
			dOid = tree.NewDOidWithType(tree.MustBeDOid(cDatum).Oid, types.RegProcedure)

ditto, replace with dOid = tree.MustBeDOid(cDatum)

[e-mbrown]

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

---

pkg/sql/opt/norm/fold_constants_funcs.go line 418 at r3 (raw file):

Previously, rafiss (Rafi Shamim) wrote…

we should be able to replace all of this:

			dOid = tree.NewDOidWithType(tree.MustBeDOid(cDatum).Oid, types.RegProcedure)

			procName, overload, err := c.f.catalog.ResolveFunctionByOID(c.f.ctx, dOid.Oid)
			if err != nil {
				return nil, false, err
			}

			dOid = tree.NewDOidWithTypeAndName(overload.Oid, types.RegProcedure, procName)

with this:

dOid = tree.MustBeDOid(cDatum)

(The PerformCast call already takes care of resolving the function, so no need to do it again)

Done.

---

pkg/sql/opt/norm/fold_constants_funcs.go line 434 at r3 (raw file):

Previously, rafiss (Rafi Shamim) wrote…

ditto, replace with dOid = tree.MustBeDOid(cDatum)

Done.

[rafiss]

lgtm! you just need to rewrite the create_statements logic test to account for the index you added

[mgartner]

It looks like some unintended changes made it into the create_statements file.

[rafiss]

@e-mbrown you can use this command to rewrite the file:

./dev testlogic base --config=local --files=create_statements --rewrite

[e-mbrown]

When I rewrite the test, tables other thanpg_proc change in the output. Not sure why.

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

[rafiss]

this is also failing a benchmark test, probably due to #90842. i will look into this

10:47:17       --- FAIL: BenchmarkAsJSON/tsquery
10:47:17           json_test.go:56: unexpected type *tree.DTSQuery for AsJSON
10:47:17       --- FAIL: BenchmarkAsJSON/tsvector
10:47:17           json_test.go:56: unexpected type *tree.DTSVector for AsJSON

as far as the create_statements diff, maybe it's expected after #91702 ... but i'm not sure

[rafiss]

@msirek fixed the benchmark issue in 5506cf8 - thank you!

[rafiss]

i think this query will work. the issue is with the newlines in the result

SELECT
  regexp_replace(create_statement, '\n', ' ', 'g'),
  regexp_replace(create_nofks, '\n', ' ', 'g'),
  alter_statements,
  validate_statements
FROM
  crdb_internal.create_statements
WHERE
  database_name = 'test'
AND
  schema_name NOT IN ('pg_catalog', 'pg_extension', 'crdb_internal', 'information_schema')

[e-mbrown]

Thanks for all the help!

bors r=rafiss

[craig]

Build succeeded:

• Bazel Essential CI (Cockroach)