Skip to content

Commit

Permalink
PG16: Update query relation permission checking
Browse files Browse the repository at this point in the history
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
  • Loading branch information
lkshminarayanan authored and fabriziomello committed Sep 27, 2023
1 parent c22d0d6 commit f7acf86
Show file tree
Hide file tree
Showing 11 changed files with 159 additions and 29 deletions.
37 changes: 25 additions & 12 deletions src/copy.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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)
{
Expand Down Expand Up @@ -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

Expand All @@ -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
Expand Down Expand Up @@ -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)
Expand All @@ -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.
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
4 changes: 4 additions & 0 deletions src/nodes/chunk_dispatch/chunk_insert_state.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down
16 changes: 15 additions & 1 deletion src/planner/expand_hypertable.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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;
Expand Down
13 changes: 11 additions & 2 deletions src/planner/planner.c
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -1319,6 +1320,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:
Expand All @@ -1334,11 +1343,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);
}
Expand Down
3 changes: 3 additions & 0 deletions tsl/src/continuous_aggs/create.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
43 changes: 40 additions & 3 deletions tsl/src/continuous_aggs/finalize.c
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -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,
Expand All @@ -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)
{
Expand All @@ -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)
{
Expand All @@ -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)
Expand All @@ -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;
}
Expand Down
9 changes: 8 additions & 1 deletion tsl/src/fdw/modify_exec.c
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -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,
Expand Down
15 changes: 12 additions & 3 deletions tsl/src/fdw/modify_plan.c
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand All @@ -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);
Expand Down
Loading

0 comments on commit f7acf86

Please sign in to comment.