Allow manual index creation in CAggs

The materialised hypertable resides in the _timescaledb.internal schema
which resulted in permission error at the time of manual index creation
by non super user. To solve this, it now switches to timescaledb user
before index creation of CAgg.

Fixes timescale#4735

CHANGELOG.md

@@ -31,6 +31,7 @@ argument or resolve the type ambiguity by casting to the intended type.
 * #4745 Fix FK constraint violation error while insert into hypertable which references partitioned table
 * #4756 Improve compression job IO performance
 * #4760 Fix segfault when INNER JOINing hypertables
+* #4735 Allow manual index creation for CAggs
 
 **Thanks**
 * @boxhock and @cocowalla for reporting a segfault when JOINing hypertables

src/process_utility.c

@@ -2594,6 +2594,9 @@ process_index_start(ProcessUtilityArgs *args)
 	TupleDesc main_table_desc;
 	Relation main_table_index_relation;
 	LockRelId main_table_index_lock_relid;
+	int sec_ctx;
+	Oid uid = InvalidOid, saved_uid = InvalidOid;
+	ContinuousAgg *cagg = NULL;
 
 	Assert(IsA(stmt, IndexStmt));
 
@@ -2612,13 +2615,15 @@ process_index_start(ProcessUtilityArgs *args)
 	if (NULL == ht)
 	{
 		/* Check if the relation is a Continuous Aggregate */
-		ContinuousAgg *cagg = ts_continuous_agg_find_by_rv(stmt->relation);
+		cagg = ts_continuous_agg_find_by_rv(stmt->relation);
 
 		if (cagg)
 		{
 			/* If the relation is a CAgg and it is finalized */
 			if (ContinuousAggIsFinalized(cagg))
+			{
 				ht = ts_hypertable_get_by_id(cagg->data.mat_hypertable_id);
+			}
 			else
 			{
 				ts_cache_release(hcache);
@@ -2703,12 +2708,25 @@ process_index_start(ProcessUtilityArgs *args)
 		PreventInTransactionBlock(true,
 								  "CREATE INDEX ... WITH (timescaledb.transaction_per_chunk)");
 
+	if (cagg)
+	{
+		/*
+		 * If this is an index creation for cagg, then we need to switch user as the current
+		 * user might not have permissions on the internal schema where cagg index will be
+		 * created.
+		 * Need to restore user soon after this step.
+		 */
+		ts_cagg_permissions_check(ht->main_table_relid, GetUserId());
+		SWITCH_TO_TS_USER(NameStr(cagg->data.direct_view_schema), uid, saved_uid, sec_ctx);
+	}
 	/* CREATE INDEX on the root table of the hypertable */
 	root_table_index = ts_indexing_root_table_create_index(stmt,
 														   args->query_string,
 														   info.extended_options.multitransaction,
 														   hypertable_is_distributed(ht));
 
+	if (cagg)
+		RESTORE_USER(uid, saved_uid, sec_ctx);
 	/* root_table_index will have 0 objectId if the index already exists
 	 * and if_not_exists is true. In that case there is nothing else
 	 * to do here. */

src/ts_catalog/continuous_agg.c

@@ -1762,3 +1762,16 @@ ts_compute_beginning_of_the_next_bucket_variable(int64 timeval,
 	val_new = generic_add_interval(bf, val_new);
 	return ts_time_value_to_internal(val_new, TIMESTAMPOID);
 }
+
+Oid
+ts_cagg_permissions_check(Oid cagg_oid, Oid userid)
+{
+	Oid ownerid = ts_rel_get_owner(cagg_oid);
+
+	if (!has_privs_of_role(userid, ownerid))
+		ereport(ERROR,
+				(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+				 errmsg("must be owner of continuous aggregate \"%s\"", get_rel_name(cagg_oid))));
+
+	return ownerid;
+}

src/ts_catalog/continuous_agg.h

@@ -17,6 +17,29 @@
 
 #define CAGGINVAL_TRIGGER_NAME "ts_cagg_invalidation_trigger"
 
+/*switch to ts user for _timescaledb_internal access */
+#define SWITCH_TO_TS_USER(schemaname, newuid, saved_uid, saved_secctx)                             \
+	do                                                                                             \
+	{                                                                                              \
+		if ((schemaname) &&                                                                        \
+			strncmp(schemaname, INTERNAL_SCHEMA_NAME, strlen(INTERNAL_SCHEMA_NAME)) == 0)          \
+			(newuid) = ts_catalog_database_info_get()->owner_uid;                                  \
+		else                                                                                       \
+			(newuid) = InvalidOid;                                                                 \
+		if ((newuid) != InvalidOid)                                                                \
+		{                                                                                          \
+			GetUserIdAndSecContext(&(saved_uid), &(saved_secctx));                                 \
+			SetUserIdAndSecContext(uid, (saved_secctx) | SECURITY_LOCAL_USERID_CHANGE);            \
+		}                                                                                          \
+	} while (0)
+
+#define RESTORE_USER(newuid, saved_uid, saved_secctx)                                              \
+	do                                                                                             \
+	{                                                                                              \
+		if ((newuid) != InvalidOid)                                                                \
+			SetUserIdAndSecContext(saved_uid, saved_secctx);                                       \
+	} while (0);
+
 typedef enum ContinuousAggViewOption
 {
 	ContinuousEnabled = 0,
@@ -133,6 +156,8 @@ typedef struct CaggPolicyOffset
 	const char *name;
 } CaggPolicyOffset;
 
+extern TSDLLEXPORT Oid ts_cagg_permissions_check(Oid cagg_oid, Oid userid);
+
 extern TSDLLEXPORT const CaggsInfo ts_continuous_agg_get_all_caggs_info(int32 raw_hypertable_id);
 extern TSDLLEXPORT void ts_populate_caggs_info_from_arrays(ArrayType *mat_hypertable_ids,
 														   ArrayType *bucket_widths,

tsl/src/bgw_policy/continuous_aggregate_api.c

@@ -230,19 +230,6 @@ policy_refresh_cagg_check(PG_FUNCTION_ARGS)
 	PG_RETURN_VOID();
 }
 
-static Oid
-ts_cagg_permissions_check(Oid cagg_oid, Oid userid)
-{
-	Oid ownerid = ts_rel_get_owner(cagg_oid);
-
-	if (!has_privs_of_role(userid, ownerid))
-		ereport(ERROR,
-				(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
-				 errmsg("must be owner of continuous aggregate \"%s\"", get_rel_name(cagg_oid))));
-
-	return ownerid;
-}
-
 static void
 json_add_dim_interval_value(JsonbParseState *parse_state, const char *json_label, Oid dim_type,
 							Datum value)

tsl/src/continuous_aggs/create.c

@@ -91,29 +91,6 @@
 #define INTERNAL_TO_TSTZ_FUNCTION "to_timestamp"
 #define INTERNAL_TO_TS_FUNCTION "to_timestamp_without_timezone"
 
-/*switch to ts user for _timescaledb_internal access */
-#define SWITCH_TO_TS_USER(schemaname, newuid, saved_uid, saved_secctx)                             \
-	do                                                                                             \
-	{                                                                                              \
-		if ((schemaname) &&                                                                        \
-			strncmp(schemaname, INTERNAL_SCHEMA_NAME, strlen(INTERNAL_SCHEMA_NAME)) == 0)          \
-			(newuid) = ts_catalog_database_info_get()->owner_uid;                                  \
-		else                                                                                       \
-			(newuid) = InvalidOid;                                                                 \
-		if ((newuid) != InvalidOid)                                                                \
-		{                                                                                          \
-			GetUserIdAndSecContext(&(saved_uid), &(saved_secctx));                                 \
-			SetUserIdAndSecContext(uid, (saved_secctx) | SECURITY_LOCAL_USERID_CHANGE);            \
-		}                                                                                          \
-	} while (0)
-
-#define RESTORE_USER(newuid, saved_uid, saved_secctx)                                              \
-	do                                                                                             \
-	{                                                                                              \
-		if ((newuid) != InvalidOid)                                                                \
-			SetUserIdAndSecContext(saved_uid, saved_secctx);                                       \
-	} while (0);
-
 #define PRINT_MATCOLNAME(colbuf, type, original_query_resno, colno)                                \
 	do                                                                                             \
 	{                                                                                              \

tsl/test/expected/cagg_permissions.out

@@ -55,6 +55,18 @@ as
 select location, max(humidity)
 from conditions
 group by time_bucket(100, timec), location WITH NO DATA;
+-- Manually create index on CAgg
+CREATE INDEX cagg_idx on mat_refresh_test(location);
+\c :TEST_DBNAME :ROLE_SUPERUSER
+CREATE USER not_priv;
+\c :TEST_DBNAME not_priv
+-- A user with no ownership on the Cagg cannot create index on it. -- This should fail
+\set ON_ERROR_STOP 0
+CREATE INDEX cagg_idx on mat_refresh_test(humidity);
+ERROR:  must be owner of hypertable "_materialized_hypertable_2"
+\c :TEST_DBNAME :ROLE_SUPERUSER
+DROP USER not_priv;
+\c :TEST_DBNAME :ROLE_DEFAULT_PERM_USER
 SELECT add_continuous_aggregate_policy('mat_refresh_test', NULL, -200::integer, '12 h'::interval);
  add_continuous_aggregate_policy 
 ---------------------------------

tsl/test/sql/cagg_permissions.sql

@@ -54,6 +54,18 @@ select location, max(humidity)
 from conditions
 group by time_bucket(100, timec), location WITH NO DATA;
 
+-- Manually create index on CAgg
+CREATE INDEX cagg_idx on mat_refresh_test(location);
+\c :TEST_DBNAME :ROLE_SUPERUSER
+CREATE USER not_priv;
+\c :TEST_DBNAME not_priv
+-- A user with no ownership on the Cagg cannot create index on it. -- This should fail
+\set ON_ERROR_STOP 0
+CREATE INDEX cagg_idx on mat_refresh_test(humidity);
+\c :TEST_DBNAME :ROLE_SUPERUSER
+DROP USER not_priv;
+\c :TEST_DBNAME :ROLE_DEFAULT_PERM_USER
+
 SELECT add_continuous_aggregate_policy('mat_refresh_test', NULL, -200::integer, '12 h'::interval);
 
 insert into conditions