From b803b7d132e3505ab77c29acf91f3d1caa298f95 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Mon, 6 Mar 2023 13:10:57 -0500 Subject: [PATCH] Fill EState.es_rteperminfos more systematically. While testing a fix for bug #17823, I discovered that EvalPlanQualStart failed to copy es_rteperminfos from the parent EState, resulting in failure if anything in EPQ execution wanted to consult that information. This led me to conclude that commit a61b1f748 had been too haphazard about where to fill es_rteperminfos, and that we need to be sure that that happens exactly where es_range_table gets filled. So I changed the signature of ExecInitRangeTable to help ensure that this new requirement doesn't get missed. (Indeed, pgoutput.c was also failing to fill it. Maybe we don't ever need it there, but I wouldn't bet on that.) No test case yet; one will arrive with the fix for #17823. But that needs to be back-patched, while this fix is HEAD-only. Discussion: https://postgr.es/m/17823-b64909cf7d63de84@postgresql.org --- src/backend/commands/copyfrom.c | 8 +------- src/backend/executor/execMain.c | 8 ++++---- src/backend/executor/execUtils.c | 6 +++++- src/backend/replication/logical/worker.c | 6 ++++-- src/backend/replication/pgoutput/pgoutput.c | 7 ++++++- src/include/executor/executor.h | 2 +- 6 files changed, 21 insertions(+), 16 deletions(-) diff --git a/src/backend/commands/copyfrom.c b/src/backend/commands/copyfrom.c index af52faca6d..29cd1cf4a6 100644 --- a/src/backend/commands/copyfrom.c +++ b/src/backend/commands/copyfrom.c @@ -757,16 +757,10 @@ CopyFrom(CopyFromState cstate) * index-entry-making machinery. (There used to be a huge amount of code * here that basically duplicated execUtils.c ...) */ - ExecInitRangeTable(estate, cstate->range_table); + ExecInitRangeTable(estate, cstate->range_table, cstate->rteperminfos); resultRelInfo = target_resultRelInfo = makeNode(ResultRelInfo); ExecInitResultRelation(estate, resultRelInfo, 1); - /* - * Copy the RTEPermissionInfos into estate as well, so that - * ExecGetInsertedCols() et al will work correctly. - */ - estate->es_rteperminfos = cstate->rteperminfos; - /* Verify the named relation is a valid target for INSERT */ CheckValidResultRel(resultRelInfo, CMD_INSERT); diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c index 39bfb48dc2..7f5968db87 100644 --- a/src/backend/executor/execMain.c +++ b/src/backend/executor/execMain.c @@ -807,15 +807,14 @@ InitPlan(QueryDesc *queryDesc, int eflags) int i; /* - * Do permissions checks and save the list for later use. + * Do permissions checks */ ExecCheckPermissions(rangeTable, plannedstmt->permInfos, true); - estate->es_rteperminfos = plannedstmt->permInfos; /* * initialize the node's execution state */ - ExecInitRangeTable(estate, rangeTable); + ExecInitRangeTable(estate, rangeTable, plannedstmt->permInfos); estate->es_plannedstmt = plannedstmt; estate->es_part_prune_infos = plannedstmt->partPruneInfos; @@ -2805,11 +2804,12 @@ EvalPlanQualStart(EPQState *epqstate, Plan *planTree) rcestate->es_range_table = parentestate->es_range_table; rcestate->es_range_table_size = parentestate->es_range_table_size; rcestate->es_relations = parentestate->es_relations; - rcestate->es_queryEnv = parentestate->es_queryEnv; rcestate->es_rowmarks = parentestate->es_rowmarks; + rcestate->es_rteperminfos = parentestate->es_rteperminfos; rcestate->es_plannedstmt = parentestate->es_plannedstmt; rcestate->es_junkFilter = parentestate->es_junkFilter; rcestate->es_output_cid = parentestate->es_output_cid; + rcestate->es_queryEnv = parentestate->es_queryEnv; /* * ResultRelInfos needed by subplans are initialized from scratch when the diff --git a/src/backend/executor/execUtils.c b/src/backend/executor/execUtils.c index cfa95a07e4..6eac5f354c 100644 --- a/src/backend/executor/execUtils.c +++ b/src/backend/executor/execUtils.c @@ -121,6 +121,7 @@ CreateExecutorState(void) estate->es_range_table_size = 0; estate->es_relations = NULL; estate->es_rowmarks = NULL; + estate->es_rteperminfos = NIL; estate->es_plannedstmt = NULL; estate->es_part_prune_infos = NIL; @@ -755,11 +756,14 @@ ExecOpenScanRelation(EState *estate, Index scanrelid, int eflags) * indexed by rangetable index. */ void -ExecInitRangeTable(EState *estate, List *rangeTable) +ExecInitRangeTable(EState *estate, List *rangeTable, List *permInfos) { /* Remember the range table List as-is */ estate->es_range_table = rangeTable; + /* ... and the RTEPermissionInfo List too */ + estate->es_rteperminfos = permInfos; + /* Set size of associated arrays */ estate->es_range_table_size = list_length(rangeTable); diff --git a/src/backend/replication/logical/worker.c b/src/backend/replication/logical/worker.c index cfb2ab6248..c7d1734a17 100644 --- a/src/backend/replication/logical/worker.c +++ b/src/backend/replication/logical/worker.c @@ -671,6 +671,7 @@ create_edata_for_relation(LogicalRepRelMapEntry *rel) ApplyExecutionData *edata; EState *estate; RangeTblEntry *rte; + List *perminfos = NIL; ResultRelInfo *resultRelInfo; edata = (ApplyExecutionData *) palloc0(sizeof(ApplyExecutionData)); @@ -683,9 +684,10 @@ create_edata_for_relation(LogicalRepRelMapEntry *rel) rte->relid = RelationGetRelid(rel->localrel); rte->relkind = rel->localrel->rd_rel->relkind; rte->rellockmode = AccessShareLock; - ExecInitRangeTable(estate, list_make1(rte)); - addRTEPermissionInfo(&estate->es_rteperminfos, rte); + addRTEPermissionInfo(&perminfos, rte); + + ExecInitRangeTable(estate, list_make1(rte), perminfos); edata->targetRelInfo = resultRelInfo = makeNode(ResultRelInfo); diff --git a/src/backend/replication/pgoutput/pgoutput.c b/src/backend/replication/pgoutput/pgoutput.c index 0df1acbb7a..00a2d73dab 100644 --- a/src/backend/replication/pgoutput/pgoutput.c +++ b/src/backend/replication/pgoutput/pgoutput.c @@ -23,6 +23,7 @@ #include "fmgr.h" #include "nodes/makefuncs.h" #include "optimizer/optimizer.h" +#include "parser/parse_relation.h" #include "replication/logical.h" #include "replication/logicalproto.h" #include "replication/origin.h" @@ -792,6 +793,7 @@ create_estate_for_relation(Relation rel) { EState *estate; RangeTblEntry *rte; + List *perminfos = NIL; estate = CreateExecutorState(); @@ -800,7 +802,10 @@ create_estate_for_relation(Relation rel) rte->relid = RelationGetRelid(rel); rte->relkind = rel->rd_rel->relkind; rte->rellockmode = AccessShareLock; - ExecInitRangeTable(estate, list_make1(rte)); + + addRTEPermissionInfo(&perminfos, rte); + + ExecInitRangeTable(estate, list_make1(rte), perminfos); estate->es_output_cid = GetCurrentCommandId(false); diff --git a/src/include/executor/executor.h b/src/include/executor/executor.h index e7e25c057e..946abc0051 100644 --- a/src/include/executor/executor.h +++ b/src/include/executor/executor.h @@ -568,7 +568,7 @@ extern bool ExecRelationIsTargetRelation(EState *estate, Index scanrelid); extern Relation ExecOpenScanRelation(EState *estate, Index scanrelid, int eflags); -extern void ExecInitRangeTable(EState *estate, List *rangeTable); +extern void ExecInitRangeTable(EState *estate, List *rangeTable, List *permInfos); extern void ExecCloseRangeTableRelations(EState *estate); extern void ExecCloseResultRelations(EState *estate);