Repair relacl on extension upgrade

.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/post-update.sql

@@ -194,3 +194,6 @@ BEGIN
 END;
 $$;
 
+-- Repair relations that have relacl entries for users that do not
+-- exist in pg_authid
+CALL _timescaledb_functions.repair_relation_acls();

sql/updates/reverse-dev.sql

@@ -191,3 +191,6 @@ CREATE FUNCTION @[email protected]_chunks(
      newer_than             "any" = NULL
  ) RETURNS SETOF REGCLASS AS '@MODULE_PATHNAME@', 'ts_chunk_show_chunks'
  LANGUAGE C STABLE PARALLEL SAFE;
+
+DROP PROCEDURE IF EXISTS _timescaledb_functions.repair_relation_acls();
+DROP FUNCTION IF EXISTS _timescaledb_functions.makeaclitem(regrole, regrole, text, bool);

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,100 @@
 	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
+ts_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;
+}
+
+/*
+ * This is copied from PostgreSQL 16.0 since versions before 16.0 does not
+ * support lists for privileges but we need that.
+ */
+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 = ts_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/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/updates/post.repair.sql

@@ -24,3 +24,7 @@ WHERE extname = 'timescaledb' \gset
     \ir post.repair.hierarchical_cagg.sql
 \endif
 
+\z repair_test_int
+\z repair_test_extra
+
+

test/sql/updates/setup.repair.sql

@@ -15,6 +15,9 @@ SELECT extversion >= '2.10.0' AS has_cagg_joins
 FROM pg_extension
 WHERE extname = 'timescaledb' \gset
 
+CREATE USER wizard;
+CREATE USER "Random L User";
+
 CREATE TABLE repair_test_int(time integer not null, temp float8, tag integer, color integer);
 CREATE TABLE repair_test_timestamptz(time timestamptz not null, temp float8, tag integer, color integer);
 CREATE TABLE repair_test_extra(time timestamptz not null, temp float8, tag integer, color integer);
@@ -65,6 +68,16 @@ INSERT INTO repair_test_date VALUES
 ALTER TABLE _timescaledb_catalog.chunk_constraint
       DROP CONSTRAINT chunk_constraint_dimension_slice_id_fkey;
 
+-- Grant privileges to some tables above. All should be repaired.
+GRANT ALL ON repair_test_int TO wizard;
+GRANT ALL ON repair_test_extra TO wizard;
+GRANT SELECT, INSERT ON repair_test_int TO "Random L User";
+GRANT INSERT ON repair_test_extra TO "Random L User";
+
+-- Break the relacl of the table by deleting users directly from
+-- pg_authid table.
+DELETE FROM pg_authid WHERE rolname IN ('wizard', 'Random L User');
+
 \if :test_repair_dimension
 
 CREATE VIEW slices AS (

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)