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 21, 2023 · 7b8e361 · 7b8e361
1 parent 53e4ac0
commit 7b8e361
Show file tree

Hide file tree

Showing 11 changed files with 159 additions and 29 deletions.
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>
@@ -1320,6 +1321,14 @@ 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);
+
+#if PG16_LT
+	AclMode requiredPerms = rte->requiredPerms;
+#else
+	RTEPermissionInfo *perminfo = getRTEPermissionInfo(query->rteperminfos, rte);
+	AclMode requiredPerms = perminfo->requiredPerms;
+#endif
+
 	switch (type)
 	{
 		case TS_REL_HYPERTABLE:
@@ -1335,11 +1344,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/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,9 @@ 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);
+#endif
 		ListCell *l;
 		foreach (l, inp->final_userquery->jointree->fromlist)
 		{
@@ -648,15 +657,26 @@ 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
+				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 +691,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 +739,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/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,16 @@ fdw_begin_foreign_modify(PlanState *pstate, ResultRelInfo *rri, CmdType operatio
 	}
 
 	/* Construct an execution state. */
+#if PG16_LT
+	Oid checkAsUser = rte->checkAsUser;
+#else
+	RTEPermissionInfo *perminfo = getRTEPermissionInfo(estate->es_rteperminfos, rte);
+	Oid checkAsUser = perminfo->checkAsUser;
+#endif
 	fmstate = create_foreign_modify(estate,
 									rri->ri_RelationDesc,
 									operation,
-									rte->checkAsUser,
+									checkAsUser,
 									subplan,
 									query,
 									target_attrs,

diff --git a/tsl/src/fdw/modify_plan.c b/tsl/src/fdw/modify_plan.c
@@ -4,6 +4,7 @@
  * LICENSE-TIMESCALE for a copy of the license.
  */
 #include <postgres.h>
+#include <parser/parse_relation.h>
 #include <parser/parsetree.h>
 #include <access/sysattr.h>
 #include <utils/rel.h>
@@ -33,12 +34,12 @@ get_insert_attrs(Relation rel)
 }
 
 static List *
-get_update_attrs(RangeTblEntry *rte)
+get_update_attrs(Bitmapset *updatedCols)
 {
 	List *attrs = NIL;
 	int col = -1;
 
-	while ((col = bms_next_member(rte->updatedCols, col)) >= 0)
+	while ((col = bms_next_member(updatedCols, col)) >= 0)
 	{
 		/* bit numbers are offset by FirstLowInvalidHeapAttributeNumber */
 		AttrNumber attno = col + FirstLowInvalidHeapAttributeNumber;
@@ -187,7 +188,14 @@ fdw_plan_foreign_modify(PlannerInfo *root, ModifyTable *plan, Index result_relat
 							 &retrieved_attrs);
 			break;
 		case CMD_UPDATE:
-			target_attrs = get_update_attrs(rte);
+		{
+#if PG16_LT
+			Bitmapset *updatedCols = rte->updatedCols;
+#else
+			RTEPermissionInfo *perminfo = getRTEPermissionInfo(root->parse->rteperminfos, rte);
+			Bitmapset *updatedCols = perminfo->updatedCols;
+#endif
+			target_attrs = get_update_attrs(updatedCols);
 			deparseUpdateSql(&sql,
 							 rte,
 							 result_relation,
@@ -197,6 +205,7 @@ fdw_plan_foreign_modify(PlannerInfo *root, ModifyTable *plan, Index result_relat
 							 &retrieved_attrs);
 			data_nodes = get_chunk_data_nodes(rel->rd_id);
 			break;
+		}
 		case CMD_DELETE:
 			deparseDeleteSql(&sql, rte, result_relation, rel, returning_list, &retrieved_attrs);
 			data_nodes = get_chunk_data_nodes(rel->rd_id);