insert into <reference table> select from <another reference table> fails with permission error

[mtuncer]

it requires 2 users, and 2 reference tables to reproduce.

Following repro is created on Azure Cosmos DB for PostgreSQL cluster with 2 worker nodes.

First create a schemas and source tables with citus user

-- citus user
create schema etl;
grant usage on schema etl to testread;

create table etl.videos(videoid bigint, lengthinseconds int);

select create_reference_table('etl.videos');

insert into etl.videos select  (random() * 10000)::BIGINT, (random() * 1000)::int from generate_series(1, 1000000);

GRANT select ON table etl.videos TO testread;

create schema summary;

GRANT ALL ON SCHEMA summary TO testread;

Then switch to other user testread

---- testread user
CREATE TABLE summary.small_videos ( videoid bigint );
select create_reference_table('summary.small_videos');

-- verify simple select query works
SELECT DISTINCT vids.videoid
FROM etl.videos vids
WHERE ((vids.lengthinseconds <= 10)) LIMIT 1;

-- then test run insert 
INSERT INTO summary.small_videos (videoid)
SELECT DISTINCT vids.videoid
FROM etl.videos vids
WHERE ((vids.lengthinseconds <= 10)) LIMIT 1;

it fails with

ERROR:  permission denied for table videos
CONTEXT:  while executing command on <worker-0 dns name>:5432   

Examining worker logs revealed that worker query fails while running lock_shard_resources on etl.videos table with lockmode = 7

There is a workaround to use CTEs like

WITH test_data AS (SELECT DISTINCT vids.videoid
FROM etl.videos vids
WHERE ((vids.lengthinseconds <= 10)) limit 1)
INSERT INTO summary.small_videos select * FROM test_data

PG Version : PG 16.9 Citus Version : 12.1.6

[colm-mchugh]

A CPG customer reported a variation of this issue today;

Create tables and role as citus user:

CREATE TABLE public.main_table (id SERIAL NOT NULL PRIMARY KEY, referenced_id INTEGER);
CREATE TABLE public.referenced_table (id SERIAL NOT NULL PRIMARY KEY);
ALTER TABLE main_table ADD FOREIGN KEY (referenced_id) REFERENCES referenced_table (id);

CREATE ROLE secondrole WITH LOGIN;

GRANT SELECT, INSERT, UPDATE, DELETE ON TABLE main_table TO secondrole;
GRANT SELECT, MAINTAIN  ON TABLE referenced_table TO secondrole;

Now connect as secondrole user and update main_table; this works fine, because citus granted UPDATE permission:

\c - secondrole
UPDATE main_table  SET referenced_id = 2 WHERE referenced_id = 1;

Next, citus user converts the tables to reference tables:

\c - citus
SELECT create_reference_table('main_table');
SELECT create_reference_table('referenced_table');

Consequently, user secondrole can no longer update main_table:

\c - secondrole
UPDATE main_table  SET referenced_id = 2 WHERE referenced_id = 1;
ERROR:  permission denied for table referenced_table

The error occurs because, as with the originally reported issue, each worker attempts to lock both table shards with ExclusiveLock:

lock_shard_resources( main_table_123, ExclusiveLock)
lock_shard_resources( referenced_table_456, ExclusiveLock)  <-- fails when running as user secondrole

~~Not clear if there's a viable workaround other than also granting UPDATE permission to the secondary role.~~ Workaround: https://github.com/citusdata/citus/pull/6293

[colm-mchugh]

The errors described in this issue are a consequence of how a5b087c89bc9 "Support FKs between reference tables" is implemented. When updating a reference table, function LockReferencedReferenceShardResources() locks the shards of reference tables that the reference table has a foreign key dependency on:

 * LockReferencedReferenceShardResources acquires resource locks with the
 * given lock mode on the reference tables which has a foreign key from
 * the given relation.
 *
 * It also gets resource locks on worker nodes to prevent concurrent write
 * operations on reference tables from metadata nodes.

The error occurs when getting resource locks on worker nodes:

		/*
		 * When there is metadata, all nodes can write to the reference table,
		 * but the writes need to be serialised. To achieve that, all nodes will
		 * take the shard resource lock on the first worker node via RPC, except
		 * for the first worker node which will just take it the regular way.
		 */
		LockShardListResourcesOnFirstWorker(lockMode, referencedShardIntervalList); <== errors out

which resolves to SELECT lock_shard_resources( LOCKMODE, ARRAY[ shardid ]) on the worker node, where permissions check fails:

		if (!SkipAdvisoryLockPermissionChecks)
		{
			EnsureTablePermissions(relationId, aclMode, aclMaskHow);  <== errors out
		}

EnsureTablePermissions() errors out because user secondrole does not have write permissions on table referenced_table. Setting citus.skip_advisory_lock_permission_checks to on enables the update to succeed, and this is probably a better way to work around the problem than granting write permissions on the referenced table. But the default behavior seems to be by design so this looks like a feature, not a bug.

[colm-mchugh]

The fix for #6293 Add citus.skip_advisory_lock_permission_checks provides a workaround for this issue; set citus.skip_advisory_lock_permission_checks to on. As noted there, While dealing with replicated tables, Citus acquires some locks that Postgres would not need. This might create some compatibility issues. The relevant example is INSERT into ref_table1 SELECT * FROM ref_table2 where citus acquires an EXCLUSIVE lock on ref_table2. The question for this issue is given UPDATE ref_table1 SET col1 = 1 where ref_table1 has a foreign key dependency on ref_table2, is it correct to acquire an exclusive lock on ref_table2 ? If yes then this issue is resolved by #6293. If no then we probably need to enhance LockReferencedReferenceShardResources() to distinguish between reference table dependencies.

[onurctirtir]

This is the expected behavior so we might want to remove "cherry-pick" labels - unless we want to investigate this more, check if we can altogether stop acquiring those advisory locks (probably we cannot) and merge a PR for that.

Otherwise, i.e., if we won't do anything about this, then we should at least make sure to clearly document this behavior in docs. Probably we already have a few lines explaining the mentioned the GUC in the docs, but we should probably re-explain this behavior in a new section like "Known behavioral differences" or such.