Skip to content

Commit a315b60

Browse files
sql/opt: treat privilege errors as stale memos during dependency checks
Previously, when the query cache's `CheckDependencies` re-resolved data sources from a cached memo and encountered a privilege error (e.g. because the memo referenced objects in a different database context), the error was propagated to the user. This caused unqualified function calls to fail with USAGE privilege errors referencing schemas from the wrong database when two databases had identically-named functions in custom schemas. Now, `maybeSwallowMetadataResolveErr` also swallows `pgcode.InsufficientPrivilege` errors, treating them as indicators that the memo is stale. The memo is evicted and replanned in the correct user/database context, where genuine privilege errors will surface during planning. Fixes: #168992 Release note (bug fix): Fixed a bug where unqualified function calls could fail with incorrect privilege errors when two databases on the same cluster had identically-named functions in custom schemas. The query cache could serve a memo from one database context to another, causing USAGE privilege errors referencing schemas from the wrong database. Co-Authored-By: roachdev-claude <roachdev-claude-bot@cockroachlabs.com>
1 parent 8ba05e7 commit a315b60

2 files changed

Lines changed: 86 additions & 5 deletions

File tree

pkg/sql/logictest/testdata/logic_test/udf_privileges

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -805,3 +805,78 @@ statement ok
805805
SET ROLE root
806806

807807
subtest end
808+
809+
# Regression test for #168992. Unqualified function calls should not leak
810+
# descriptors across database boundaries via the query cache. When two databases
811+
# have identically-named functions in custom schemas, the memo cached from one
812+
# database context must be treated as stale when accessed from another, rather
813+
# than producing a privilege error referencing schemas from the wrong database.
814+
subtest cross_database_function_resolution
815+
816+
statement ok
817+
CREATE DATABASE db_a;
818+
CREATE DATABASE db_b;
819+
CREATE ROLE role_a;
820+
CREATE ROLE role_b;
821+
GRANT role_a TO testuser;
822+
GRANT role_b TO testuser;
823+
GRANT CONNECT ON DATABASE db_a TO role_a;
824+
GRANT CONNECT ON DATABASE db_b TO role_b;
825+
826+
statement ok
827+
USE db_a;
828+
CREATE SCHEMA sch_a;
829+
GRANT USAGE ON SCHEMA sch_a TO role_a;
830+
CREATE TABLE sch_a.records (id INT PRIMARY KEY, label TEXT NOT NULL);
831+
GRANT SELECT ON TABLE sch_a.records TO role_a;
832+
CREATE FUNCTION sch_a.get_records(p_id INT) RETURNS TABLE(id INT, label TEXT) LANGUAGE SQL AS $$
833+
SELECT id, label FROM db_a.sch_a.records WHERE id >= p_id ORDER BY id
834+
$$;
835+
GRANT EXECUTE ON FUNCTION sch_a.get_records TO role_a;
836+
INSERT INTO sch_a.records VALUES (1, 'alpha');
837+
838+
statement ok
839+
USE db_b;
840+
CREATE SCHEMA sch_b;
841+
GRANT USAGE ON SCHEMA sch_b TO role_b;
842+
CREATE TABLE sch_b.records (id INT PRIMARY KEY, label TEXT NOT NULL);
843+
GRANT SELECT ON TABLE sch_b.records TO role_b;
844+
CREATE FUNCTION sch_b.get_records(p_id INT) RETURNS TABLE(id INT, label TEXT) LANGUAGE SQL AS $$
845+
SELECT id, label FROM db_b.sch_b.records WHERE id >= p_id ORDER BY id
846+
$$;
847+
GRANT EXECUTE ON FUNCTION sch_b.get_records TO role_b;
848+
INSERT INTO sch_b.records VALUES (1, 'beta');
849+
850+
# Populate the query cache for get_records via role_a in db_a.
851+
statement ok
852+
USE db_a;
853+
SET ROLE role_a;
854+
SET search_path = sch_a;
855+
856+
query I
857+
SELECT count(*) FROM get_records(1)
858+
----
859+
1
860+
861+
# Switch to role_b in db_b. The cached memo references db_a.sch_a, but that
862+
# should be detected as stale rather than producing a privilege error.
863+
statement ok
864+
SET ROLE role_b;
865+
USE db_b;
866+
SET search_path = sch_b;
867+
868+
query I
869+
SELECT count(*) FROM get_records(1)
870+
----
871+
1
872+
873+
statement ok
874+
SET ROLE root;
875+
USE test;
876+
SET search_path = public;
877+
DROP DATABASE db_a CASCADE;
878+
DROP DATABASE db_b CASCADE;
879+
DROP ROLE role_a;
880+
DROP ROLE role_b;
881+
882+
subtest end

pkg/sql/opt/metadata.go

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -626,18 +626,24 @@ func (md *Metadata) CheckDependencies(
626626
return true, nil
627627
}
628628

629-
// handleMetadataResolveErr swallows errors that are thrown when a database
630-
// object is dropped, since such an error potentially only means that the
631-
// metadata is stale and should be re-resolved.
629+
// maybeSwallowMetadataResolveErr swallows errors that are thrown when a
630+
// database object cannot be resolved during a metadata staleness check, since
631+
// such an error potentially only means that the metadata is stale and should be
632+
// re-resolved. This includes cases where the object no longer exists, or where
633+
// the current user lacks privileges to access it (e.g. because the cached memo
634+
// references objects in a different database context). In both cases, the memo
635+
// is treated as stale and will be replanned in the correct context, which will
636+
// surface genuine privilege errors during planning if applicable.
632637
func maybeSwallowMetadataResolveErr(err error) error {
633638
if err == nil {
634639
return nil
635640
}
636-
// Handle when the object no longer exists.
641+
// Handle when the object no longer exists or is inaccessible.
637642
switch pgerror.GetPGCode(err) {
638643
case pgcode.UndefinedObject, pgcode.UndefinedTable, pgcode.UndefinedDatabase,
639644
pgcode.UndefinedSchema, pgcode.UndefinedFunction, pgcode.InvalidName,
640-
pgcode.InvalidSchemaName, pgcode.InvalidCatalogName:
645+
pgcode.InvalidSchemaName, pgcode.InvalidCatalogName,
646+
pgcode.InsufficientPrivilege:
641647
return nil
642648
}
643649
if errors.Is(err, catalog.ErrDescriptorDropped) {

0 commit comments

Comments
 (0)