PG16: Update query relation permission checking

PG16 moves the permission checking information out of the range table entries into a new struct - RTEPermissionInfo. This commit updates timescaledb to use the this new mechanism when compiling with PG16. postgres/postgres@a61b1f74 postgres/postgres@b803b7d1 postgres/postgres@47bb9db7
timescale · Sep 28, 2023 · 642970f · 642970f
1 parent f40d3fd
commit 642970f
Show file tree

Hide file tree

Showing 14 changed files with 195 additions and 34 deletions.
diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -346,9 +346,9 @@ set(PG_VERSION "${PG_VERSION_MAJOR}.${PG_VERSION_MINOR}")
 # Ensure that PostgreSQL version is supported and consistent with src/compat.h
 # version check
 if((${PG_VERSION_MAJOR} LESS "13")
-   OR (${PG_VERSION_MAJOR} GREATER "15")
+   OR (${PG_VERSION_MAJOR} GREATER "16")
    AND NOT (${EXPERIMENTAL}))
-  message(FATAL_ERROR "TimescaleDB only supports PostgreSQL 13, 14 and 15")
+  message(FATAL_ERROR "TimescaleDB only supports PostgreSQL 13, 14, 15 and 16")
 else()
   message(STATUS "Compiling against PostgreSQL version ${PG_VERSION}")
 endif()

diff --git a/src/copy.c b/src/copy.c
@@ -694,7 +694,7 @@ has_other_before_insert_row_trigger_than_ts(ResultRelInfo *resultRelInfo)
  * Use COPY FROM to copy data from file to relation.
  */
 static uint64
-copyfrom(CopyChunkState *ccstate, List *range_table, Hypertable *ht, MemoryContext copycontext,
+copyfrom(CopyChunkState *ccstate, ParseState *pstate, Hypertable *ht, MemoryContext copycontext,
 		 void (*callback)(void *), void *arg)
 {
 	ResultRelInfo *resultRelInfo;
@@ -719,7 +719,7 @@ copyfrom(CopyChunkState *ccstate, List *range_table, Hypertable *ht, MemoryConte
 	ExprState *qualexpr = NULL;
 	ChunkDispatch *dispatch = ccstate->dispatch;
 
-	Assert(range_table);
+	Assert(pstate->p_rtable);
 
 	if (ccstate->rel->rd_rel->relkind != RELKIND_RELATION)
 	{
@@ -813,7 +813,12 @@ copyfrom(CopyChunkState *ccstate, List *range_table, Hypertable *ht, MemoryConte
 					  NULL,
 					  0);
 #else
-	ExecInitRangeTable(estate, range_table);
+#if PG16_LT
+	ExecInitRangeTable(estate, pstate->p_rtable);
+#else
+	Assert(pstate->p_rteperminfos != NULL);
+	ExecInitRangeTable(estate, pstate->p_rtable, pstate->p_rteperminfos);
+#endif
 	ExecInitResultRelation(estate, resultRelInfo, 1);
 #endif
 
@@ -825,7 +830,7 @@ copyfrom(CopyChunkState *ccstate, List *range_table, Hypertable *ht, MemoryConte
 	estate->es_result_relations = resultRelInfo;
 	estate->es_num_result_relations = 1;
 	estate->es_result_relation_info = resultRelInfo;
-	estate->es_range_table = range_table;
+	estate->es_range_table = pstate->p_rtable;
 
 	ExecInitRangeTable(estate, estate->es_range_table);
 #endif
@@ -1259,6 +1264,8 @@ copy_constraints_and_check(ParseState *pstate, Relation rel, List *attnums)
 		addRangeTableEntryForRelation(pstate, rel, RowExclusiveLock, NULL, false, false);
 	RangeTblEntry *rte = nsitem->p_rte;
 	addNSItemToQuery(pstate, nsitem, true, true, true);
+
+#if PG16_LT
 	rte->requiredPerms = ACL_INSERT;
 
 	foreach (cur, attnums)
@@ -1268,6 +1275,18 @@ copy_constraints_and_check(ParseState *pstate, Relation rel, List *attnums)
 	}
 
 	ExecCheckRTPerms(pstate->p_rtable, true);
+#else
+	RTEPermissionInfo *perminfo = nsitem->p_perminfo;
+	perminfo->requiredPerms = ACL_INSERT;
+
+	foreach (cur, attnums)
+	{
+		int attno = lfirst_int(cur) - FirstLowInvalidHeapAttributeNumber;
+		perminfo->insertedCols = bms_add_member(perminfo->insertedCols, attno);
+	}
+
+	ExecCheckPermissions(pstate->p_rtable, list_make1(perminfo), true);
+#endif
 
 	/*
 	 * Permission check for row security policies.
@@ -1390,8 +1409,7 @@ timescaledb_DoCopy(const CopyStmt *stmt, const char *queryString, uint64 *proces
 		/* Or create a new memory context. */
 		copycontext = AllocSetContextCreate(CurrentMemoryContext, "COPY", ALLOCSET_DEFAULT_SIZES);
 #endif
-		*processed =
-			copyfrom(ccstate, pstate->p_rtable, ht, copycontext, CopyFromErrorCallback, cstate);
+		*processed = copyfrom(ccstate, pstate, ht, copycontext, CopyFromErrorCallback, cstate);
 	}
 
 	copy_chunk_state_destroy(ccstate);
@@ -1467,12 +1485,7 @@ timescaledb_move_from_table_to_chunks(Hypertable *ht, LOCKMODE lockmode)
 	snapshot = RegisterSnapshot(GetLatestSnapshot());
 	scandesc = table_beginscan(rel, snapshot, 0, NULL);
 	ccstate = copy_chunk_state_create(ht, rel, next_copy_from_table_to_chunks, NULL, scandesc);
-	copyfrom(ccstate,
-			 pstate->p_rtable,
-			 ht,
-			 copycontext,
-			 copy_table_to_chunk_error_callback,
-			 scandesc);
+	copyfrom(ccstate, pstate, ht, copycontext, copy_table_to_chunk_error_callback, scandesc);
 	copy_chunk_state_destroy(ccstate);
 	table_endscan(scandesc);
 	UnregisterSnapshot(snapshot);

diff --git a/src/nodes/chunk_dispatch/chunk_insert_state.c b/src/nodes/chunk_dispatch/chunk_insert_state.c
@@ -650,12 +650,16 @@ ts_chunk_insert_state_create(const Chunk *chunk, ChunkDispatch *dispatch)
 
 	if (chunk->relkind == RELKIND_FOREIGN_TABLE)
 	{
+#if PG16_LT
 		RangeTblEntry *rte =
 			rt_fetch(relinfo->ri_RangeTableIndex, dispatch->estate->es_range_table);
 
 		Assert(rte != NULL);
 
 		state->user_id = OidIsValid(rte->checkAsUser) ? rte->checkAsUser : GetUserId();
+#else
+		state->user_id = ExecGetResultRelCheckAsUser(relinfo, state->estate);
+#endif
 		state->chunk_data_nodes = ts_chunk_data_nodes_copy(chunk);
 	}
 

diff --git a/src/planner/expand_hypertable.c b/src/planner/expand_hypertable.c
@@ -1424,7 +1424,14 @@ ts_plan_expand_hypertable_chunks(Hypertable *ht, PlannerInfo *root, RelOptInfo *
 		childrte->inh = false;
 		/* clear the magic bit */
 		childrte->ctename = NULL;
+#if PG16_LT
 		childrte->requiredPerms = 0;
+#else
+		/* Since PG16, the permission info is maintained separetely. Unlink
+		 * the old perminfo from the RTE to disable permission checking.
+		 */
+		childrte->perminfoindex = 0;
+#endif
 		childrte->securityQuals = NIL;
 		parse->rtable = lappend(parse->rtable, childrte);
 		child_rtindex = list_length(parse->rtable);
@@ -1474,8 +1481,15 @@ ts_plan_expand_hypertable_chunks(Hypertable *ht, PlannerInfo *root, RelOptInfo *
 
 		data_node_rte->inh = false;
 		data_node_rte->ctename = NULL;
-		data_node_rte->requiredPerms = 0;
 		data_node_rte->securityQuals = NIL;
+#if PG16_LT
+		data_node_rte->requiredPerms = 0;
+#else
+		/* Since PG16, the permission info is maintained separetely. Unlink
+		 * the old perminfo from the RTE to disable permission checking.
+		 */
+		data_node_rte->perminfoindex = 0;
+#endif
 		parse->rtable = lappend(parse->rtable, data_node_rte);
 		rti = list_length(parse->rtable);
 		root->simple_rte_array[rti] = data_node_rte;

diff --git a/src/planner/planner.c b/src/planner/planner.c
@@ -21,6 +21,7 @@
 #include <optimizer/restrictinfo.h>
 #include <optimizer/tlist.h>
 #include <parser/parsetree.h>
+#include <parser/parse_relation.h>
 #include <utils/elog.h>
 #include <utils/fmgroids.h>
 #include <utils/guc.h>
@@ -1319,6 +1320,18 @@ timescaledb_get_relation_info_hook(PlannerInfo *root, Oid relation_objectid, boo
 	Query *query = root->parse;
 	Hypertable *ht;
 	const TsRelType type = ts_classify_relation(root, rel, &ht);
+	AclMode requiredPerms = 0;
+
+#if PG16_LT
+	requiredPerms = rte->requiredPerms;
+#else
+	if (rte->perminfoindex > 0)
+	{
+		RTEPermissionInfo *perminfo = getRTEPermissionInfo(query->rteperminfos, rte);
+		requiredPerms = perminfo->requiredPerms;
+	}
+#endif
+
 	switch (type)
 	{
 		case TS_REL_HYPERTABLE:
@@ -1334,11 +1347,11 @@ timescaledb_get_relation_info_hook(PlannerInfo *root, Oid relation_objectid, boo
 			 * assumes permission checking has been done already during the first planner call.
 			 * We don't want to touch the UPDATE/DELETEs, so we need to check all the regular
 			 * conditions here that are checked during preprocess_query, as well as the
-			 * condition that rte->requiredPerms is not requiring UPDATE/DELETE on this rel.
+			 * condition that requiredPerms is not requiring UPDATE/DELETE on this rel.
 			 */
 			if (ts_guc_enable_optimizations && ts_guc_enable_constraint_exclusion && inhparent &&
 				rte->ctename == NULL && !IS_UPDL_CMD(query) && query->resultRelation == 0 &&
-				query->rowMarks == NIL && (rte->requiredPerms & (ACL_UPDATE | ACL_DELETE)) == 0)
+				query->rowMarks == NIL && (requiredPerms & (ACL_UPDATE | ACL_DELETE)) == 0)
 			{
 				rte_mark_for_expansion(rte);
 			}

diff --git a/tsl/src/continuous_aggs/common.c b/tsl/src/continuous_aggs/common.c
@@ -100,12 +100,14 @@ relation_oid(Name schema, Name name)
 void
 RemoveRangeTableEntries(Query *query)
 {
+#if PG16_LT
 	List *rtable = query->rtable;
 	Assert(list_length(rtable) >= 3);
 	rtable = list_delete_first(rtable);
 	query->rtable = list_delete_first(rtable);
 	OffsetVarNodes((Node *) query, -2, 0);
 	Assert(list_length(query->rtable) >= 1);
+#endif
 }
 
 /*

diff --git a/tsl/src/continuous_aggs/create.c b/tsl/src/continuous_aggs/create.c
@@ -557,6 +557,9 @@ mattablecolumninfo_get_partial_select_query(MatTableColumnInfo *mattblinfo, Quer
 	CAGG_MAKEQUERY(partial_selquery, userview_query);
 	partial_selquery->rtable = copyObject(userview_query->rtable);
 	partial_selquery->jointree = copyObject(userview_query->jointree);
+#if PG16_GE
+	partial_selquery->rteperminfos = copyObject(userview_query->rteperminfos);
+#endif
 
 	partial_selquery->targetList = mattblinfo->partial_seltlist;
 	partial_selquery->groupClause = mattblinfo->partial_grouplist;

diff --git a/tsl/src/continuous_aggs/finalize.c b/tsl/src/continuous_aggs/finalize.c
@@ -9,6 +9,9 @@
  * Continuous Aggregates (with partials)
  */
 #include "finalize.h"
+
+#include <parser/parse_relation.h>
+
 #include "create.h"
 #include "common.h"
 #include <partialize_finalize.h>
@@ -609,6 +612,9 @@ finalizequery_get_select_query(FinalizeQueryInfo *inp, List *matcollist,
 	ListCell *lc;
 	FromExpr *fromexpr;
 	RangeTblEntry *rte;
+#if PG16_GE
+	RTEPermissionInfo *perminfo;
+#endif
 
 	/*
 	 * For initial cagg creation rtable will have only 1 entry,
@@ -628,6 +634,10 @@ finalizequery_get_select_query(FinalizeQueryInfo *inp, List *matcollist,
 		rte->inh = true;
 		rte->rellockmode = 1;
 		rte->eref = copyObject(rte->alias);
+#if PG16_GE
+		perminfo = addRTEPermissionInfo(&final_selquery->rteperminfos, rte);
+		perminfo->selectedCols = NULL;
+#endif
 		ListCell *l;
 		foreach (l, inp->final_userquery->jointree->fromlist)
 		{
@@ -648,15 +658,29 @@ finalizequery_get_select_query(FinalizeQueryInfo *inp, List *matcollist,
 #if PG14_GE
 				rte->join_using_alias = jrte->join_using_alias;
 #endif
+#if PG16_LT
 				rte->selectedCols = jrte->selectedCols;
+#else
+				if (jrte->perminfoindex > 0)
+				{
+					RTEPermissionInfo *jperminfo =
+						getRTEPermissionInfo(inp->final_userquery->rteperminfos, jrte);
+					perminfo->selectedCols = jperminfo->selectedCols;
+				}
+#endif
 			}
 		}
 	}
 	else
 	{
 		rte = llast_node(RangeTblEntry, inp->final_userquery->rtable);
 		rte->eref->colnames = NIL;
+#if PG16_LT
 		rte->selectedCols = NULL;
+#else
+		perminfo = getRTEPermissionInfo(inp->final_userquery->rteperminfos, rte);
+		perminfo->selectedCols = NULL;
+#endif
 	}
 	if (rte->eref->colnames == NIL)
 	{
@@ -671,18 +695,28 @@ finalizequery_get_select_query(FinalizeQueryInfo *inp, List *matcollist,
 		{
 			ColumnDef *cdef = lfirst_node(ColumnDef, lc);
 			rte->eref->colnames = lappend(rte->eref->colnames, makeString(cdef->colname));
-			rte->selectedCols = bms_add_member(rte->selectedCols,
-											   list_length(rte->eref->colnames) -
-												   FirstLowInvalidHeapAttributeNumber);
+			int attno = list_length(rte->eref->colnames) - FirstLowInvalidHeapAttributeNumber;
+#if PG16_LT
+			rte->selectedCols = bms_add_member(rte->selectedCols, attno);
+#else
+			perminfo->selectedCols = bms_add_member(perminfo->selectedCols, attno);
+#endif
 		}
 	}
 	rte->relid = mattbladdress->objectId;
 	rte->rtekind = RTE_RELATION;
 	rte->relkind = RELKIND_RELATION;
 	rte->tablesample = NULL;
+#if PG16_LT
 	rte->requiredPerms |= ACL_SELECT;
 	rte->insertedCols = NULL;
 	rte->updatedCols = NULL;
+#else
+	perminfo->relid = mattbladdress->objectId;
+	perminfo->requiredPerms |= ACL_SELECT;
+	perminfo->insertedCols = NULL;
+	perminfo->updatedCols = NULL;
+#endif
 
 	/* 2. Fixup targetlist with the correct rel information. */
 	foreach (lc, inp->final_seltlist)
@@ -709,13 +743,20 @@ finalizequery_get_select_query(FinalizeQueryInfo *inp, List *matcollist,
 	{
 		RangeTblRef *rtr;
 		final_selquery->rtable = list_make1(rte);
+#if PG16_GE
+		/* perminfo has been set already in the previous if/else */
+		Assert(list_length(final_selquery->rteperminfos) == 1);
+#endif
 		rtr = makeNode(RangeTblRef);
 		rtr->rtindex = 1;
 		fromexpr = makeFromExpr(list_make1(rtr), NULL);
 	}
 	else
 	{
 		final_selquery->rtable = inp->final_userquery->rtable;
+#if PG16_GE
+		final_selquery->rteperminfos = inp->final_userquery->rteperminfos;
+#endif
 		fromexpr = inp->final_userquery->jointree;
 		fromexpr->quals = NULL;
 	}

diff --git a/tsl/src/continuous_aggs/options.c b/tsl/src/continuous_aggs/options.c
@@ -110,13 +110,23 @@ cagg_find_groupingcols(ContinuousAgg *agg, Hypertable *mat_ht)
 	Oid mat_relid = mat_ht->main_table_relid;
 	Query *finalize_query;
 
+#if PG16_LT
 	/* The view rule has dummy old and new range table entries as the 1st and 2nd entries */
 	Assert(list_length(cagg_view_query->rtable) >= 2);
+#endif
+
 	if (cagg_view_query->setOperations)
 	{
-		/* This corresponds to the union view.
-		 * the 3rd RTE entry has the SELECT 1 query from the union view. */
+		/*
+		 * This corresponds to the union view.
+		 *   PG16_LT the 3rd RTE entry has the SELECT 1 query from the union view.
+		 *   PG16_GE the 1st RTE entry has the SELECT 1 query from the union view
+		 */
+#if PG16_LT
 		RangeTblEntry *finalize_query_rte = lthird(cagg_view_query->rtable);
+#else
+		RangeTblEntry *finalize_query_rte = linitial(cagg_view_query->rtable);
+#endif
 		if (finalize_query_rte->rtekind != RTE_SUBQUERY)
 			ereport(ERROR,
 					(errcode(ERRCODE_TS_UNEXPECTED),

diff --git a/tsl/src/fdw/modify_exec.c b/tsl/src/fdw/modify_exec.c
@@ -5,6 +5,7 @@
  */
 #include <postgres.h>
 #include <executor/executor.h>
+#include <parser/parse_relation.h>
 #include <parser/parsetree.h>
 #include <nodes/plannodes.h>
 #include <commands/explain.h>
@@ -347,10 +348,20 @@ fdw_begin_foreign_modify(PlanState *pstate, ResultRelInfo *rri, CmdType operatio
 	}
 
 	/* Construct an execution state. */
+	Oid checkAsUser = InvalidOid;
+#if PG16_LT
+	checkAsUser = rte->checkAsUser;
+#else
+	if (rte->perminfoindex > 0)
+	{
+		RTEPermissionInfo *perminfo = getRTEPermissionInfo(estate->es_rteperminfos, rte);
+		checkAsUser = perminfo->checkAsUser;
+	}
+#endif
 	fmstate = create_foreign_modify(estate,
 									rri->ri_RelationDesc,
 									operation,
-									rte->checkAsUser,
+									checkAsUser,
 									subplan,
 									query,
 									target_attrs,