From aa9e23ba4ad4f6de567afb20eebd07017e272efd Mon Sep 17 00:00:00 2001 From: Dipesh Dhameliya Date: Thu, 10 Apr 2025 09:10:43 +0530 Subject: [PATCH] Avoid checking permission of Babelfish temp tables on parallel worker (#560) Consider following facts, 1. Babelfish temp tables are implemented using ENR which is not shared between different backends. So if Parallel worker tries to check permissions on Babelfish then it will fail. 2. Any user should be able to access Babelfish temp tables under given session. 3. Postgres by default does not allow parallel operations on temp tables. Attempt to do so will result in run time error. Due to above facts, we should avoid permission check on temp tables within parallel workers while ensuring that leader does required permission check on other tables. This commits achieves this behaviour by introducing following three hooks, ParallelQueryMain_hook -- Hook that allows other extensions to pass on additional details from Leader node to parallel worker. For example, Babelfish extension can pass details of Babelfish temp table defined under current session with Parallel workers. ExecInitParallelPlan_hook -- Hook that allows Parallel worker to gather additional details passed by Leader node. For example, Babelfish extension can collect the details of Babelfish temp table shared by Leader node so that it can avoid permission checks. ExecCheckOneRelPerms_hook -- Hook that allows extension control permission checking on given relation/table. For example, Babelfish can use it to avoid permission check on temp tables under parallel worker. Task: BABEL-5703 Signed-off-by: Dipesh Dhameliya (cherry picked from commit 5e62ffa9bd5bdd5a1978b7896fa0450afe113ad0) --- src/backend/executor/execMain.c | 29 ++++++++++++++++++++++++++--- src/backend/executor/execParallel.c | 19 +++++++++++++++++++ src/include/access/parallel.h | 4 ---- src/include/executor/execParallel.h | 11 +++++++++++ src/include/executor/executor.h | 5 +++++ 5 files changed, 61 insertions(+), 7 deletions(-) diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c index f41e1591ec0..28e982de191 100644 --- a/src/backend/executor/execMain.c +++ b/src/backend/executor/execMain.c @@ -78,6 +78,8 @@ ExecutorEnd_hook_type ExecutorEnd_hook = NULL; TriggerRecuresiveCheck_hook_type TriggerRecuresiveCheck_hook = NULL; check_rowcount_hook_type check_rowcount_hook = NULL; +ExecCheckOneRelPerms_hook_type ExecCheckOneRelPerms_hook = NULL; + /* Hook for plugin to get control in ExecCheckPermissions() */ ExecutorCheckPerms_hook_type ExecutorCheckPerms_hook = NULL; @@ -650,6 +652,19 @@ ExecCheckOneRelPerms(RTEPermissionInfo *perminfo) requiredPerms = perminfo->requiredPerms; Assert(requiredPerms != 0); + /* + * Babelfish specific logic - Babelfish temp table is implemented + * using ENR which is not shared with parallel worker and parallel + * operations are not allowed for temp table in Postgres. Babelfish + * can skip permission check for such use cases under parallel worker + * using this hook. + * Note - This hook must not be used outside of Babelfish parallel worker + */ + if (ExecCheckOneRelPerms_hook && + IsBabelfishParallelWorker() && + (*ExecCheckOneRelPerms_hook)(perminfo)) + return true; + /* * userid to check as: current user unless we have a setuid indication. * @@ -842,10 +857,9 @@ InitPlan(QueryDesc *queryDesc, int eflags) int i; /* - * Do permissions checks if not Babelfish parallel worker + * Do permissions checks */ - if (!IsBabelfishParallelWorker()) - ExecCheckPermissions(rangeTable, plannedstmt->permInfos, true); + ExecCheckPermissions(rangeTable, plannedstmt->permInfos, true); /* * initialize the node's execution state @@ -3066,3 +3080,12 @@ EvalPlanQualEnd(EPQState *epqstate) epqstate->relsubs_done = NULL; epqstate->relsubs_blocked = NULL; } + +/* + * ExecCheckOneRelPerms_wrapper - wrapper around ExecCheckOneRelPerms + */ +bool +ExecCheckOneRelPerms_wrapper(RTEPermissionInfo *perminfo) +{ + return ExecCheckOneRelPerms(perminfo); +} diff --git a/src/backend/executor/execParallel.c b/src/backend/executor/execParallel.c index cc2b8ccab70..8bdfb218ed6 100644 --- a/src/backend/executor/execParallel.c +++ b/src/backend/executor/execParallel.c @@ -124,6 +124,9 @@ typedef struct ExecParallelInitializeDSMContext int nnodes; } ExecParallelInitializeDSMContext; +ExecInitParallelPlan_hook_type ExecInitParallelPlan_hook = NULL; +ParallelQueryMain_hook_type ParallelQueryMain_hook = NULL; + /* Helper functions that run in the parallel leader. */ static char *ExecSerializePlan(Plan *plan, EState *estate); static bool ExecParallelEstimate(PlanState *planstate, @@ -685,6 +688,10 @@ ExecInitParallelPlan(PlanState *planstate, EState *estate, mul_size(PARALLEL_TUPLE_QUEUE_SIZE, pcxt->nworkers)); shm_toc_estimate_keys(&pcxt->estimator, 1); + /* Let extension estimate a dynamic shared memory needed to communicate additional context */ + if (ExecInitParallelPlan_hook) + (*ExecInitParallelPlan_hook)(estate, pcxt, true); + /* * Give parallel-aware nodes a chance to add to the estimates, and get a * count of how many PlanState nodes there are. @@ -768,6 +775,12 @@ ExecInitParallelPlan(PlanState *planstate, EState *estate, shm_toc_insert(pcxt->toc, PARALLEL_KEY_WAL_USAGE, walusage_space); pei->wal_usage = walusage_space; + /* Give extension a chance to share additional context */ + if (ExecInitParallelPlan_hook) + { + (*ExecInitParallelPlan_hook)(estate, pcxt,false); + } + /* Set up the tuple queues that the workers will write into. */ pei->tqueue = ExecParallelSetupTupleQueues(pcxt, false); @@ -1428,6 +1441,12 @@ ParallelQueryMain(dsm_segment *seg, shm_toc *toc) area_space = shm_toc_lookup(toc, PARALLEL_KEY_DSA, false); area = dsa_attach_in_place(area_space, seg); + /* Give extension chance to retrieve additional context shared by leader node */ + if (ParallelQueryMain_hook) + { + (*ParallelQueryMain_hook)(toc); + } + /* Start up the executor */ queryDesc->plannedstmt->jitFlags = fpes->jit_flags; ExecutorStart(queryDesc, fpes->eflags); diff --git a/src/include/access/parallel.h b/src/include/access/parallel.h index b97fa06afec..b1c2c4b6814 100644 --- a/src/include/access/parallel.h +++ b/src/include/access/parallel.h @@ -82,9 +82,6 @@ extern void ParallelWorkerMain(Datum main_arg); /* Below helpers are added to support parallel workers in Babelfish context */ extern bool IsBabelfishParallelWorker(void); -/* Key for BabelfishFixedParallelState */ -#define BABELFISH_PARALLEL_KEY_FIXED UINT64CONST(0xBBF0000000000001) - /* Hooks for communicating babelfish related information to parallel worker */ typedef void (*bbf_InitializeParallelDSM_hook_type)(ParallelContext *pcxt, bool estimate); extern PGDLLIMPORT bbf_InitializeParallelDSM_hook_type bbf_InitializeParallelDSM_hook; @@ -92,5 +89,4 @@ extern PGDLLIMPORT bbf_InitializeParallelDSM_hook_type bbf_InitializeParallelDSM typedef void (*bbf_ParallelWorkerMain_hook_type)(shm_toc *toc); extern PGDLLIMPORT bbf_ParallelWorkerMain_hook_type bbf_ParallelWorkerMain_hook; - #endif /* PARALLEL_H */ diff --git a/src/include/executor/execParallel.h b/src/include/executor/execParallel.h index 39a8792a319..6a5f3447046 100644 --- a/src/include/executor/execParallel.h +++ b/src/include/executor/execParallel.h @@ -48,4 +48,15 @@ extern void ExecParallelReinitialize(PlanState *planstate, extern void ParallelQueryMain(dsm_segment *seg, shm_toc *toc); +typedef void (*ParallelQueryMain_hook_type)(shm_toc *toc); +extern PGDLLIMPORT ParallelQueryMain_hook_type ParallelQueryMain_hook; + +/* + * When estimate = true passed then caller wants extension to estimate a dynamic shared memory (DSM) + * needed by that extension to communicate additional context with Parallel worker. + * When estimate = false then caller wants to insert additional context to DSM. + */ +typedef void (*ExecInitParallelPlan_hook_type)(EState *estate, ParallelContext *pcxt, bool estimate); +extern PGDLLIMPORT ExecInitParallelPlan_hook_type ExecInitParallelPlan_hook; + #endif /* EXECPARALLEL_H */ diff --git a/src/include/executor/executor.h b/src/include/executor/executor.h index 0f0eaf4aeb1..2a685ae6f62 100644 --- a/src/include/executor/executor.h +++ b/src/include/executor/executor.h @@ -111,6 +111,9 @@ extern PGDLLIMPORT ExecUpdateResultTypeTL_hook_type ExecUpdateResultTypeTL_hook; typedef bool (*check_rowcount_hook_type) (int es_processed); extern PGDLLEXPORT check_rowcount_hook_type check_rowcount_hook; +typedef bool (*ExecCheckOneRelPerms_hook_type) (RTEPermissionInfo *perminfo); +extern PGDLLEXPORT ExecCheckOneRelPerms_hook_type ExecCheckOneRelPerms_hook; + /* * prototypes from functions in execAmi.c */ @@ -224,6 +227,8 @@ extern void standard_ExecutorEnd(QueryDesc *queryDesc); extern void ExecutorRewind(QueryDesc *queryDesc); extern bool ExecCheckPermissions(List *rangeTable, List *rteperminfos, bool ereport_on_violation); +/* Wrapper around ExecCheckOneRelPerms */ +extern bool ExecCheckOneRelPerms_wrapper(RTEPermissionInfo *perminfo); extern void CheckValidResultRel(ResultRelInfo *resultRelInfo, CmdType operation); extern void InitResultRelInfo(ResultRelInfo *resultRelInfo, Relation resultRelationDesc,