Repair relacl on upgrade

If users have accidentally been removed from `pg_authid` as a result of
bugs where dropping a user did not revoke privileges from all tables
where the had privileges, it will not be possible to create new chunks
since these require the user to be found when copying the privileges
for the parent table (either compressed hypertable or normal
hypertable).

To fix the situation, we repair the `pg_class` table by updating the
`relacl` for relations and remove any user that do not have an entry in
`pg_authid`.

.unreleased/fix_6290

@@ -0,0 +1 @@
+Fixes: #6290 Repair relacl on upgrade

sql/maintenance_utils.sql

@@ -117,3 +117,43 @@ BEGIN
     END CASE;
 END
 $$ LANGUAGE plpgsql;
+
+-- A version of makeaclitem that accepts a comma-separated list of
+-- privileges rather than just a single privilege. This is copied from
+-- PG16, but since we need to support earlier versions, we provide it
+-- with the extension.
+--
+-- This is intended for internal usage and interface might change.
+CREATE FUNCTION _timescaledb_functions.makeaclitem(regrole, regrole, text, bool)
+RETURNS AclItem AS '@MODULE_PATHNAME@', 'ts_makeaclitem'
+LANGUAGE C STABLE PARALLEL SAFE;
+
+-- Repair relation ACL by removing roles that do not exist in pg_authid.
+CREATE PROCEDURE _timescaledb_functions.repair_relation_acls()
+LANGUAGE SQL AS $$
+  WITH
+    badrels AS (
+	SELECT oid::regclass
+	  FROM (SELECT oid, (aclexplode(relacl)).* FROM pg_class) AS rels
+	 WHERE rels.grantee != 0
+	   AND rels.grantee NOT IN (SELECT oid FROM pg_authid)
+    ),
+    pickacls AS (
+      SELECT oid::regclass,
+	     _timescaledb_functions.makeaclitem(
+	         b.grantee,
+		 b.grantor,
+		 string_agg(b.privilege_type, ','),
+		 b.is_grantable
+	     ) AS acl
+	FROM (SELECT oid, (aclexplode(relacl)).* AS a FROM pg_class) AS b
+       WHERE b.grantee IN (SELECT oid FROM pg_authid)
+       GROUP BY oid, b.grantee, b.grantor, b.is_grantable
+    ),
+    cleanacls AS (
+      SELECT oid, array_agg(acl) AS acl FROM pickacls GROUP BY oid
+    )
+  UPDATE pg_class c
+     SET relacl = (SELECT acl FROM cleanacls n WHERE c.oid = n.oid)
+   WHERE oid IN (SELECT oid FROM badrels)
+$$ SET search_path TO pg_catalog, pg_temp;

sql/updates/latest-dev.sql

@@ -250,3 +250,34 @@ CREATE FUNCTION @[email protected]_chunks(
      created_after          "any" = NULL
  ) RETURNS SETOF REGCLASS AS '@MODULE_PATHNAME@', 'ts_chunk_show_chunks'
  LANGUAGE C STABLE PARALLEL SAFE;
+
+CREATE FUNCTION _timescaledb_functions.makeaclitem(regrole, regrole, text, bool)
+RETURNS AclItem AS '@MODULE_PATHNAME@', 'ts_makeaclitem'
+LANGUAGE C STABLE PARALLEL SAFE;
+
+-- Repair relations that have relacl entries for users that do not exist in pg_authid
+WITH
+  badrels AS (
+      SELECT oid::regclass
+	FROM (SELECT oid, (aclexplode(relacl)).* FROM pg_catalog.pg_class) AS rels
+       WHERE rels.grantee != 0
+	 AND rels.grantee NOT IN (SELECT oid FROM pg_catalog.pg_authid)
+  ),
+  pickacls AS (
+    SELECT oid::regclass,
+	   _timescaledb_functions.makeaclitem(
+	       b.grantee,
+	       b.grantor,
+	       string_agg(b.privilege_type, ','),
+	       b.is_grantable
+	   ) AS acl
+      FROM (SELECT oid, (aclexplode(relacl)).* AS a FROM pg_catalog.pg_class) AS b
+     WHERE b.grantee IN (SELECT oid FROM pg_catalog.pg_authid)
+     GROUP BY oid, b.grantee, b.grantor, b.is_grantable
+  ),
+  cleanacls AS (
+    SELECT oid, array_agg(acl) AS acl FROM pickacls GROUP BY oid
+  )
+UPDATE pg_catalog.pg_class c
+   SET relacl = (SELECT acl FROM cleanacls n WHERE c.oid = n.oid)
+ WHERE oid IN (SELECT oid FROM badrels);

src/utils.c

@@ -42,7 +42,14 @@
 #include "utils.h"
 #include "time_utils.h"
 
+typedef struct
+{
+	const char *name;
+	AclMode value;
+} priv_map;
+
 TS_FUNCTION_INFO_V1(ts_pg_timestamp_to_unix_microseconds);
+TS_FUNCTION_INFO_V1(ts_makeaclitem);
 
 /*
  * Convert a Postgres TIMESTAMP to BIGINT microseconds relative the UNIX epoch.
@@ -1378,3 +1385,96 @@ ts_table_has_tuples(Oid table_relid, LOCKMODE lockmode)
 	table_close(rel, lockmode);
 	return hastuples;
 }
+
+/*
+ * This is copied from PostgreSQL 16.0 since versions before 16.0 does not
+ * support lists for privileges.
+ */
+static AclMode
+convert_any_priv_string(text *priv_type_text, const priv_map *privileges)
+{
+	AclMode result = 0;
+	char *priv_type = text_to_cstring(priv_type_text);
+	char *chunk;
+	char *next_chunk;
+
+	/* We rely on priv_type being a private, modifiable string */
+	for (chunk = priv_type; chunk; chunk = next_chunk)
+	{
+		int chunk_len;
+		const priv_map *this_priv;
+
+		/* Split string at commas */
+		next_chunk = strchr(chunk, ',');
+		if (next_chunk)
+			*next_chunk++ = '\0';
+
+		/* Drop leading/trailing whitespace in this chunk */
+		while (*chunk && isspace((unsigned char) *chunk))
+			chunk++;
+		chunk_len = strlen(chunk);
+		while (chunk_len > 0 && isspace((unsigned char) chunk[chunk_len - 1]))
+			chunk_len--;
+		chunk[chunk_len] = '\0';
+
+		/* Match to the privileges list */
+		for (this_priv = privileges; this_priv->name; this_priv++)
+		{
+			if (pg_strcasecmp(this_priv->name, chunk) == 0)
+			{
+				result |= this_priv->value;
+				break;
+			}
+		}
+		if (!this_priv->name)
+			ereport(ERROR,
+					(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+					 errmsg("unrecognized privilege type: \"%s\"", chunk)));
+	}
+
+	pfree(priv_type);
+	return result;
+}
+
+Datum
+ts_makeaclitem(PG_FUNCTION_ARGS)
+{
+	Oid grantee = PG_GETARG_OID(0);
+	Oid grantor = PG_GETARG_OID(1);
+	text *privtext = PG_GETARG_TEXT_PP(2);
+	bool goption = PG_GETARG_BOOL(3);
+	AclItem *result;
+	AclMode priv;
+	static const priv_map any_priv_map[] = {
+		{ "SELECT", ACL_SELECT },
+		{ "INSERT", ACL_INSERT },
+		{ "UPDATE", ACL_UPDATE },
+		{ "DELETE", ACL_DELETE },
+		{ "TRUNCATE", ACL_TRUNCATE },
+		{ "REFERENCES", ACL_REFERENCES },
+		{ "TRIGGER", ACL_TRIGGER },
+		{ "EXECUTE", ACL_EXECUTE },
+		{ "USAGE", ACL_USAGE },
+		{ "CREATE", ACL_CREATE },
+		{ "TEMP", ACL_CREATE_TEMP },
+		{ "TEMPORARY", ACL_CREATE_TEMP },
+		{ "CONNECT", ACL_CONNECT },
+#if PG16_GE
+		{ "SET", ACL_SET },
+		{ "ALTER SYSTEM", ACL_ALTER_SYSTEM },
+#endif
+		{ "RULE", 0 }, /* ignore old RULE privileges */
+		{ NULL, 0 }
+	};
+
+	priv = convert_any_priv_string(privtext, any_priv_map);
+
+	result = (AclItem *) palloc(sizeof(AclItem));
+
+	result->ai_grantee = grantee;
+	result->ai_grantor = grantor;
+
+	ACLITEM_SET_PRIVS_GOPTIONS(*result, priv, (goption ? priv : ACL_NO_RIGHTS));
+
+	PG_RETURN_ACLITEM_P(result);
+}

test/expected/repair.out

@@ -0,0 +1,74 @@
+-- This file and its contents are licensed under the Apache License 2.0.
+-- Please see the included NOTICE for copyright information and
+-- LICENSE-APACHE for a copy of the license.
+-- We are testing different repair functions here to make sure that
+-- they work as expected.
+\c :TEST_DBNAME :ROLE_SUPERUSER
+CREATE USER wizard;
+CREATE USER "Random L User";
+CREATE TABLE test_table_1(time timestamptz not null, temp float);
+SELECT create_hypertable('test_table_1', by_range('time', '1 day'::interval));
+ create_hypertable 
+-------------------
+ (1,t)
+(1 row)
+
+INSERT INTO test_table_1(time,temp)
+SELECT time, 100 * random()
+  FROM generate_series(
+           '2000-01-01'::timestamptz,
+           '2000-01-05'::timestamptz,
+           '1min'::interval
+       ) time;
+CREATE TABLE test_table_2(time timestamptz not null, temp float);
+SELECT create_hypertable('test_table_2', by_range('time', '1 day'::interval));
+ create_hypertable 
+-------------------
+ (2,t)
+(1 row)
+
+INSERT INTO test_table_2(time,temp)
+SELECT time, 100 * random()
+  FROM generate_series(
+           '2000-01-01'::timestamptz,
+           '2000-01-05'::timestamptz,
+           '1min'::interval
+       ) time;
+GRANT ALL ON test_table_1 TO wizard;
+GRANT ALL ON test_table_2 TO wizard;
+GRANT SELECT, INSERT ON test_table_1 TO "Random L User";
+GRANT INSERT ON test_table_2 TO "Random L User";
+-- Break the relacl of the table by deleting users
+DELETE FROM pg_authid WHERE rolname IN ('wizard', 'Random L User');
+CREATE TABLE saved (LIKE pg_class);
+INSERT INTO saved SELECT * FROM pg_class;
+CALL _timescaledb_functions.repair_relation_acls();
+-- The only thing we should see here are the relations we broke and
+-- the privileges we added for that user. No other relations should be
+-- touched.
+WITH
+   lhs AS (SELECT oid, aclexplode(relacl) FROM pg_class),
+   rhs AS (SELECT oid, aclexplode(relacl) FROM saved)
+SELECT rhs.oid::regclass
+FROM lhs FULL OUTER JOIN rhs ON row(lhs) = row(rhs)
+WHERE lhs.oid IS NULL AND rhs.oid IS NOT NULL
+GROUP BY rhs.oid;
+                   oid                   
+-----------------------------------------
+ test_table_1
+ _timescaledb_internal._hyper_1_1_chunk
+ _timescaledb_internal._hyper_1_2_chunk
+ _timescaledb_internal._hyper_1_3_chunk
+ _timescaledb_internal._hyper_1_4_chunk
+ _timescaledb_internal._hyper_1_5_chunk
+ test_table_2
+ _timescaledb_internal._hyper_2_6_chunk
+ _timescaledb_internal._hyper_2_7_chunk
+ _timescaledb_internal._hyper_2_8_chunk
+ _timescaledb_internal._hyper_2_9_chunk
+ _timescaledb_internal._hyper_2_10_chunk
+(12 rows)
+
+DROP TABLE saved;
+DROP TABLE test_table_1;
+DROP TABLE test_table_2;

test/expected/util.out

@@ -2,3 +2,11 @@
 -- Please see the included NOTICE for copyright information and
 -- LICENSE-APACHE for a copy of the license.
 \set ECHO errors
+        item        
+--------------------
+ wizard=a/wizard
+ wizard=ar/wizard
+ wizard=a*/wizard
+ wizard=a*r*/wizard
+(4 rows)
+

test/sql/CMakeLists.txt

@@ -42,6 +42,7 @@ set(TEST_FILES
     plan_hypertable_inline.sql
     relocate_extension.sql
     reloptions.sql
+    repair.sql
     size_utils.sql
     sort_optimization.sql
     sql_query.sql

test/sql/repair.sql

@@ -0,0 +1,61 @@
+-- This file and its contents are licensed under the Apache License 2.0.
+-- Please see the included NOTICE for copyright information and
+-- LICENSE-APACHE for a copy of the license.
+
+-- We are testing different repair functions here to make sure that
+-- they work as expected.
+
+\c :TEST_DBNAME :ROLE_SUPERUSER
+
+CREATE USER wizard;
+CREATE USER "Random L User";
+
+CREATE TABLE test_table_1(time timestamptz not null, temp float);
+SELECT create_hypertable('test_table_1', by_range('time', '1 day'::interval));
+
+INSERT INTO test_table_1(time,temp)
+SELECT time, 100 * random()
+  FROM generate_series(
+           '2000-01-01'::timestamptz,
+           '2000-01-05'::timestamptz,
+           '1min'::interval
+       ) time;
+
+CREATE TABLE test_table_2(time timestamptz not null, temp float);
+SELECT create_hypertable('test_table_2', by_range('time', '1 day'::interval));
+
+INSERT INTO test_table_2(time,temp)
+SELECT time, 100 * random()
+  FROM generate_series(
+           '2000-01-01'::timestamptz,
+           '2000-01-05'::timestamptz,
+           '1min'::interval
+       ) time;
+
+GRANT ALL ON test_table_1 TO wizard;
+GRANT ALL ON test_table_2 TO wizard;
+GRANT SELECT, INSERT ON test_table_1 TO "Random L User";
+GRANT INSERT ON test_table_2 TO "Random L User";
+
+-- Break the relacl of the table by deleting users
+DELETE FROM pg_authid WHERE rolname IN ('wizard', 'Random L User');
+
+CREATE TABLE saved (LIKE pg_class);
+INSERT INTO saved SELECT * FROM pg_class;
+
+CALL _timescaledb_functions.repair_relation_acls();
+
+-- The only thing we should see here are the relations we broke and
+-- the privileges we added for that user. No other relations should be
+-- touched.
+WITH
+   lhs AS (SELECT oid, aclexplode(relacl) FROM pg_class),
+   rhs AS (SELECT oid, aclexplode(relacl) FROM saved)
+SELECT rhs.oid::regclass
+FROM lhs FULL OUTER JOIN rhs ON row(lhs) = row(rhs)
+WHERE lhs.oid IS NULL AND rhs.oid IS NOT NULL
+GROUP BY rhs.oid;
+
+DROP TABLE saved;
+DROP TABLE test_table_1;
+DROP TABLE test_table_2;

test/sql/util.sql

@@ -4,10 +4,21 @@
 
 \set ECHO errors
 \set VERBOSITY default
+\c :TEST_DBNAME :ROLE_SUPERUSER
 
 DO $$
 BEGIN
   ASSERT( _timescaledb_functions.get_partition_for_key(''::text) = 669664877 );
   ASSERT( _timescaledb_functions.get_partition_for_key('dev1'::text) = 1129986420 );
   ASSERT( _timescaledb_functions.get_partition_for_key('longlonglonglongpartitionkey'::text) = 1169179734);
 END$$;
+
+CREATE USER wizard;
+SELECT * FROM (
+    VALUES
+       (_timescaledb_functions.makeaclitem('wizard', 'wizard', 'insert', false)),
+       (_timescaledb_functions.makeaclitem('wizard', 'wizard', 'insert,select', false)),
+       (_timescaledb_functions.makeaclitem('wizard', 'wizard', 'insert', true)),
+       (_timescaledb_functions.makeaclitem('wizard', 'wizard', 'insert,select', true)))
+    AS t(item);
+DROP USER wizard;

tsl/test/shared/expected/extension.out

@@ -107,6 +107,7 @@ ORDER BY pronamespace::regnamespace::text COLLATE "C", p.oid::regprocedure::text
  _timescaledb_functions.invalidation_process_hypertable_log(integer,integer,regtype,integer[],bigint[],bigint[],text[])
  _timescaledb_functions.last_combinefunc(internal,internal)
  _timescaledb_functions.last_sfunc(internal,anyelement,"any")
+ _timescaledb_functions.makeaclitem(regrole,regrole,text,boolean)
  _timescaledb_functions.materialization_invalidation_log_delete(integer)
  _timescaledb_functions.partialize_agg(anyelement)
  _timescaledb_functions.ping_data_node(name,interval)
@@ -127,6 +128,7 @@ ORDER BY pronamespace::regnamespace::text COLLATE "C", p.oid::regprocedure::text
  _timescaledb_functions.recompress_chunk_segmentwise(regclass,boolean)
  _timescaledb_functions.relation_size(regclass)
  _timescaledb_functions.remote_txn_heal_data_node(oid)
+ _timescaledb_functions.repair_relation_acls()
  _timescaledb_functions.restart_background_workers()
  _timescaledb_functions.rxid_in(cstring)
  _timescaledb_functions.rxid_out(rxid)