opt: check privileges after data sources in memo stale check

[DrewKimball]

Describe the problem

#96045 moved the data sources used for metadata dependency tracking from a slice to a map. Maps in go are traversed in nondeterministic order, and the privileges on a data source are checked immediately after the data source itself is checked for staleness. This means that a table that is referenced only indirectly through a foreign key relation (by ID) can cause the memo stale check to return a privilege error in some cases when the referenced table is checked before the table that references it, and a user with different privileges + search path has run the same SQL statement.

To Reproduce

In an open CRDB session:

CREATE DATABASE test1;
USE test1;
CREATE TABLE ab (a INT PRIMARY KEY, b INT);
INSERT INTO ab VALUES (1, 1);
CREATE TABLE xy (x INT, y INT, FOREIGN KEY (x) REFERENCES ab(a));
CREATE ROLE test1;
GRANT ALL ON xy TO test1;
GRANT ALL ON ab TO test1;
ALTER ROLE test1 WITH LOGIN;

CREATE DATABASE test2;
USE test2;
CREATE TABLE ab (a INT PRIMARY KEY, b INT);
INSERT INTO ab VALUES (1, 1);
CREATE TABLE xy (x INT, y INT, FOREIGN KEY (x) REFERENCES ab(a));
CREATE ROLE test2;
GRANT ALL ON xy TO test2;
GRANT ALL ON ab TO test2;
ALTER ROLE test2 WITH LOGIN;

Run concurrently:

while true; do ./cockroach sql --insecure -u test1 -d test1 -e 'INSERT into xy VALUES(1, 1)' ; done
while true; do ./cockroach sql --insecure -u test2 -d test2 -e 'INSERT into xy VALUES(1, 1)' ; done

Example result:

INSERT 0 1

Time: 8ms

INSERT 0 1

Time: 6ms

ERROR: user test2 does not have SELECT privilege on relation ab
SQLSTATE: 42501
Failed running "sql"
INSERT 0 1

Time: 5ms

INSERT 0 1

Time: 8ms

Expected behavior
Rather than returning an error to the user, the cached memo should simply be invalidated.

Jira issue: CRDB-27415

gz#16801

gz#16795

[chengxiong-ruan]

Nice! I actually tried the same repro last week but wasn't able to get the error for some reason :( maybe just bad luck...

[joshimhoff]

Will the bug fix be in v22.2.9?

[chengxiong-ruan]

release-22.2: opt: check privileges after data sources in memo staleness check #102652

yes, it will, given that the backport was merged on 04/29 and v22.2.9 and looks like there're still other v22.2.9 release blockers.

[joshimhoff]

Thanks!