optbuilder: fix schema-qualified REGCLASS cast failing in UDFs

[ikristina]

When building a UDF or view, maybeTrackRegclassDependenciesForViews evaluates any constant REGCLASS expression to record the referenced data source as a schema dependency. The previous implementation resolved the data source by converting the evaluated datum to a string via DOid.String(), then constructing an unqualified table name from it.

The root cause is that ParseDOid stores only the bare object name in DOid.name (pkg/sql/sem/eval/parse_doid.go:189-206) when creating the datum (e.g. "myseq" instead of "sc.myseq"). The schema is used to resolve the OID correctly, but is then discarded from the name field. DOid.String() therefore returns only the bare object name, and constructing an unqualified table name from it fails when the object's schema is not in the search path - at CREATE FUNCTION time, not at call time.

The fix bypasses the lossy name entirely by asserting the evaluated datum to *tree.DOid and calling ResolveDataSourceByID with the numeric OID directly, which is always correct regardless of schema. This bug affects both PL/pgSQL and SQL UDFs since both languages go through finishBuildScalar, which calls this function.

The function is also refactored to use early returns instead of nested if blocks, reducing indentation depth.

Resolves: #167057
Epic: CRDB-62227

Release note (bug fix): Fixed a bug where creating a PL/pgSQL or SQL user-defined function that references a schema-qualified sequence or table via a REGCLASS cast (e.g.
nextval('sc.myseq'::REGCLASS)) would fail with "relation does not exist" at CREATE FUNCTION time when the object's schema was not in the search path.

[trunk-io]

Merging to master in this repository is managed by Trunk.

• To merge this pull request, check the box to the left or comment /trunk merge below.

After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here

[blathers-crl]

Thank you for contributing to CockroachDB. Please ensure you have followed the guidelines for creating a PR.

My owl senses detect your PR is good for review. Please keep an eye out for any test failures in CI.

_{🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.}

[cockroach-teamcity]

This change is 

[cockroach-teamcity]

⚪ Sysbench [SQL, 3node, oltp_read_write]

Metric	Old Commit	New Commit	Delta	Note
⚪ sec/op	11.18m ±5%	11.07m ±5%	~	p=0.461 n=15
⚪ allocs/op	8.163k ±3%	8.136k ±1%	~	p=0.309 n=15

Reproduce

benchdiff binaries:

mkdir -p benchdiff/f00b89c/bin/1058449141
gcloud storage cp gs://cockroach-microbench-ci/builds/f00b89c2e50e1f6809d214d2957e6d5ec96f3da1/bin/pkg_sql_tests benchdiff/f00b89c/bin/1058449141/cockroachdb_cockroach_pkg_sql_tests
chmod +x benchdiff/f00b89c/bin/1058449141/cockroachdb_cockroach_pkg_sql_tests
mkdir -p benchdiff/9d2a41d/bin/1058449141
gcloud storage cp gs://cockroach-microbench-ci/builds/9d2a41d63b95f4c84c58e4391c868f25d0c4b950/bin/pkg_sql_tests benchdiff/9d2a41d/bin/1058449141/cockroachdb_cockroach_pkg_sql_tests
chmod +x benchdiff/9d2a41d/bin/1058449141/cockroachdb_cockroach_pkg_sql_tests

benchdiff command:

# NB: for best (most stable) results, also add a suitable `--benchtime` that
# results in ~1s to ~5s of benchmark runs. For example, if ops average ~3ms, a
# benchtime of `1000x` is appropriate.
#
# Some benchmarks (in particular BenchmarkSysbench) output additional memory
# profiles covering only the execution (excluding the setup/teardown) - those
# should be preferred for analysis since they more closely correspond to what's
# reported as B/op and alloc/op.
benchdiff --run=^BenchmarkSysbench/SQL/3node/oltp_read_write$ --old=9d2a41d --new=f00b89c --memprofile ./pkg/sql/tests

⚪ Sysbench [KV, 3node, oltp_read_only]

Metric	Old Commit	New Commit	Delta	Note
⚪ sec/op	3.217m ±2%	3.247m ±2%	~	p=0.217 n=15
⚪ allocs/op	2.107k ±1%	2.107k ±0%	~	p=0.715 n=15

Reproduce

benchdiff binaries:

mkdir -p benchdiff/f00b89c/bin/1058449141
gcloud storage cp gs://cockroach-microbench-ci/builds/f00b89c2e50e1f6809d214d2957e6d5ec96f3da1/bin/pkg_sql_tests benchdiff/f00b89c/bin/1058449141/cockroachdb_cockroach_pkg_sql_tests
chmod +x benchdiff/f00b89c/bin/1058449141/cockroachdb_cockroach_pkg_sql_tests
mkdir -p benchdiff/9d2a41d/bin/1058449141
gcloud storage cp gs://cockroach-microbench-ci/builds/9d2a41d63b95f4c84c58e4391c868f25d0c4b950/bin/pkg_sql_tests benchdiff/9d2a41d/bin/1058449141/cockroachdb_cockroach_pkg_sql_tests
chmod +x benchdiff/9d2a41d/bin/1058449141/cockroachdb_cockroach_pkg_sql_tests

benchdiff command:

# NB: for best (most stable) results, also add a suitable `--benchtime` that
# results in ~1s to ~5s of benchmark runs. For example, if ops average ~3ms, a
# benchtime of `1000x` is appropriate.
#
# Some benchmarks (in particular BenchmarkSysbench) output additional memory
# profiles covering only the execution (excluding the setup/teardown) - those
# should be preferred for analysis since they more closely correspond to what's
# reported as B/op and alloc/op.
benchdiff --run=^BenchmarkSysbench/KV/3node/oltp_read_only$ --old=9d2a41d --new=f00b89c --memprofile ./pkg/sql/tests

🔴 Sysbench [KV, 3node, oltp_write_only]

Metric	Old Commit	New Commit	Delta	Note
🔴 sec/op	2.978m ±1%	3.024m ±0%	+1.56%	p=0.000 n=15
⚪ allocs/op	4.212k ±0%	4.211k ±0%	~	p=0.782 n=15

Reproduce

benchdiff binaries:

mkdir -p benchdiff/f00b89c/bin/1058449141
gcloud storage cp gs://cockroach-microbench-ci/builds/f00b89c2e50e1f6809d214d2957e6d5ec96f3da1/bin/pkg_sql_tests benchdiff/f00b89c/bin/1058449141/cockroachdb_cockroach_pkg_sql_tests
chmod +x benchdiff/f00b89c/bin/1058449141/cockroachdb_cockroach_pkg_sql_tests
mkdir -p benchdiff/9d2a41d/bin/1058449141
gcloud storage cp gs://cockroach-microbench-ci/builds/9d2a41d63b95f4c84c58e4391c868f25d0c4b950/bin/pkg_sql_tests benchdiff/9d2a41d/bin/1058449141/cockroachdb_cockroach_pkg_sql_tests
chmod +x benchdiff/9d2a41d/bin/1058449141/cockroachdb_cockroach_pkg_sql_tests

benchdiff command:

# NB: for best (most stable) results, also add a suitable `--benchtime` that
# results in ~1s to ~5s of benchmark runs. For example, if ops average ~3ms, a
# benchtime of `1000x` is appropriate.
#
# Some benchmarks (in particular BenchmarkSysbench) output additional memory
# profiles covering only the execution (excluding the setup/teardown) - those
# should be preferred for analysis since they more closely correspond to what's
# reported as B/op and alloc/op.
benchdiff --run=^BenchmarkSysbench/KV/3node/oltp_write_only$ --old=9d2a41d --new=f00b89c --memprofile ./pkg/sql/tests

Artifacts

download:

mkdir -p new
gcloud storage cp gs://cockroach-microbench-ci/artifacts/f00b89c2e50e1f6809d214d2957e6d5ec96f3da1/24057425385-2/\* new/
mkdir -p old
gcloud storage cp gs://cockroach-microbench-ci/artifacts/9d2a41d63b95f4c84c58e4391c868f25d0c4b950/24057425385-2/\* old/

built with commit: f00b89c2e50e1f6809d214d2957e6d5ec96f3da1

[ZhouXing19]

LGTM, thanks for writing this up! Just one nit comment.

@ZhouXing19 reviewed 2 files and all commit messages, and made 2 comments.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on ikristina and mw5h).

---

pkg/sql/logictest/testdata/logic_test/udf_regressions line 837 at r1 (raw file):

SELECT sc_62227.next_id_sql()
----
2

nit: might worth adding a test case where we directly use the oid as the source of the cast. e.g.:

# The oid might change as the test evolve / under different configuration, thus we store it in a variable "seqoid".
let $seqoid
SELECT 'sc_62227.myseq'::REGCLASS::OID;

statement ok
CREATE OR REPLACE FUNCTION  sc_62227.next_id()
RETURNS INT
LANGUAGE plpgsql
AS $$
DECLARE
  v INT;
BEGIN
  SELECT nextval($seqoid::REGCLASS) INTO v;
  RETURN v;
END;
$$;

query TT
SHOW CREATE FUNCTION sc_62227.next_id;
----

[blathers-crl]

Thank you for updating your pull request.

My owl senses detect your PR is good for review. Please keep an eye out for any test failures in CI.

_{🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.}

[ikristina]

Hi, is there anything I can do to fix the failing pipeline?

[michae2]

Hi @ikristina, try rebasing on master and pushing again. That should fix things.