Skip to content

Commit 6826f23

Browse files
authored
Avoid checking permission of Babelfish temp tables on parallel worker (#569) (#571)
Consider following facts, 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. Any user should be able to access Babelfish temp tables under given session. 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. ExecCheckRTEPerms_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. Also, this changes introduces stringToBms(...) API using which we can directly build Bitmapset from given string. This is especially useful by Babelfish parallel worker to prepare Bitmapset of temp relids from string shared by Leader node. Task: BABEL-5703 Signed-off-by: Dipesh Dhameliya <[email protected]>
1 parent 1f8a424 commit 6826f23

File tree

7 files changed

+80
-7
lines changed

7 files changed

+80
-7
lines changed

src/backend/executor/execMain.c

+27-3
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,9 @@ check_rowcount_hook_type check_rowcount_hook = NULL;
8181
/* Hook for plugin to get control in ExecCheckRTPerms() */
8282
ExecutorCheckPerms_hook_type ExecutorCheckPerms_hook = NULL;
8383

84+
/* Hook for plugin to get control in ExecCheckRTEPerms() */
85+
ExecCheckRTEPerms_hook_type ExecCheckRTEPerms_hook = NULL;
86+
8487
/* decls for local routines only used within this module */
8588
static void InitPlan(QueryDesc *queryDesc, int eflags);
8689
static void CheckValidRowMarkRel(Relation rel, RowMarkType markType);
@@ -621,6 +624,19 @@ ExecCheckRTEPerms(RangeTblEntry *rte)
621624

622625
relOid = rte->relid;
623626

627+
/*
628+
* Babelfish specific logic - Babelfish temp table is implemented
629+
* using ENR which is not shared with parallel worker and parallel
630+
* operations are not allowed for temp table in Postgres. Babelfish
631+
* can skip permission check for such use cases under parallel worker
632+
* using this hook.
633+
* Note - This hook must not be used outside of Babelfish parallel worker
634+
*/
635+
if (ExecCheckRTEPerms_hook &&
636+
IsBabelfishParallelWorker() &&
637+
(*ExecCheckRTEPerms_hook)(rte))
638+
return true;
639+
624640
/*
625641
* userid to check as: current user unless we have a setuid indication.
626642
*
@@ -813,10 +829,9 @@ InitPlan(QueryDesc *queryDesc, int eflags)
813829
int i;
814830

815831
/*
816-
* Do permissions checks if not Babelfish parallel worker
832+
* Do permissions checks
817833
*/
818-
if (!IsBabelfishParallelWorker())
819-
ExecCheckRTPerms(rangeTable, true);
834+
ExecCheckRTPerms(rangeTable, true);
820835

821836
/*
822837
* initialize the node's execution state
@@ -3043,3 +3058,12 @@ EvalPlanQualEnd(EPQState *epqstate)
30433058
epqstate->relsubs_done = NULL;
30443059
epqstate->epqExtra->relsubs_blocked = NULL;
30453060
}
3061+
3062+
/*
3063+
* ExecCheckRTEPerms_wrapper - wrapper around ExecCheckRTEPerms
3064+
*/
3065+
bool
3066+
ExecCheckRTEPerms_wrapper(RangeTblEntry *rte)
3067+
{
3068+
return ExecCheckRTEPerms(rte);
3069+
}

src/backend/executor/execParallel.c

+19
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,9 @@ typedef struct ExecParallelInitializeDSMContext
124124
int nnodes;
125125
} ExecParallelInitializeDSMContext;
126126

127+
ExecInitParallelPlan_hook_type ExecInitParallelPlan_hook = NULL;
128+
ParallelQueryMain_hook_type ParallelQueryMain_hook = NULL;
129+
127130
/* Helper functions that run in the parallel leader. */
128131
static char *ExecSerializePlan(Plan *plan, EState *estate);
129132
static bool ExecParallelEstimate(PlanState *node,
@@ -684,6 +687,10 @@ ExecInitParallelPlan(PlanState *planstate, EState *estate,
684687
mul_size(PARALLEL_TUPLE_QUEUE_SIZE, pcxt->nworkers));
685688
shm_toc_estimate_keys(&pcxt->estimator, 1);
686689

690+
/* Let extension estimate a dynamic shared memory needed to communicate additional context */
691+
if (ExecInitParallelPlan_hook)
692+
(*ExecInitParallelPlan_hook)(estate, pcxt, true);
693+
687694
/*
688695
* Give parallel-aware nodes a chance to add to the estimates, and get a
689696
* count of how many PlanState nodes there are.
@@ -767,6 +774,12 @@ ExecInitParallelPlan(PlanState *planstate, EState *estate,
767774
shm_toc_insert(pcxt->toc, PARALLEL_KEY_WAL_USAGE, walusage_space);
768775
pei->wal_usage = walusage_space;
769776

777+
/* Give extension a chance to share additional context */
778+
if (ExecInitParallelPlan_hook)
779+
{
780+
(*ExecInitParallelPlan_hook)(estate, pcxt,false);
781+
}
782+
770783
/* Set up the tuple queues that the workers will write into. */
771784
pei->tqueue = ExecParallelSetupTupleQueues(pcxt, false);
772785

@@ -1427,6 +1440,12 @@ ParallelQueryMain(dsm_segment *seg, shm_toc *toc)
14271440
area_space = shm_toc_lookup(toc, PARALLEL_KEY_DSA, false);
14281441
area = dsa_attach_in_place(area_space, seg);
14291442

1443+
/* Give extension chance to retrieve additional context shared by leader node */
1444+
if (ParallelQueryMain_hook)
1445+
{
1446+
(*ParallelQueryMain_hook)(toc);
1447+
}
1448+
14301449
/* Start up the executor */
14311450
queryDesc->plannedstmt->jitFlags = fpes->jit_flags;
14321451
ExecutorStart(queryDesc, fpes->eflags);

src/backend/nodes/read.c

+18
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
#include <ctype.h>
2323

2424
#include "common/string.h"
25+
#include "nodes/bitmapset.h"
2526
#include "nodes/pg_list.h"
2627
#include "nodes/readfuncs.h"
2728
#include "nodes/value.h"
@@ -478,3 +479,20 @@ nodeRead(const char *token, int tok_len)
478479

479480
return (void *) result;
480481
}
482+
483+
/*
484+
* Helper function for Babelfish to build Bitmapset from string.
485+
*/
486+
Bitmapset *
487+
stringToBms(const char *str)
488+
{
489+
Bitmapset *ret;
490+
const char *save_strtok;
491+
492+
save_strtok = pg_strtok_ptr;
493+
pg_strtok_ptr = str; /* point pg_strtok at the string to read */
494+
495+
ret = readBitmapset();
496+
pg_strtok_ptr = save_strtok;
497+
return ret;
498+
}

src/include/access/parallel.h

-4
Original file line numberDiff line numberDiff line change
@@ -82,15 +82,11 @@ extern void ParallelWorkerMain(Datum main_arg);
8282
/* Below helpers are added to support parallel workers in Babelfish context */
8383
extern bool IsBabelfishParallelWorker(void);
8484

85-
/* Key for BabelfishFixedParallelState */
86-
#define BABELFISH_PARALLEL_KEY_FIXED UINT64CONST(0xBBF0000000000001)
87-
8885
/* Hooks for communicating babelfish related information to parallel worker */
8986
typedef void (*bbf_InitializeParallelDSM_hook_type)(ParallelContext *pcxt, bool estimate);
9087
extern PGDLLIMPORT bbf_InitializeParallelDSM_hook_type bbf_InitializeParallelDSM_hook;
9188

9289
typedef void (*bbf_ParallelWorkerMain_hook_type)(shm_toc *toc);
9390
extern PGDLLIMPORT bbf_ParallelWorkerMain_hook_type bbf_ParallelWorkerMain_hook;
9491

95-
9692
#endif /* PARALLEL_H */

src/include/executor/execParallel.h

+11
Original file line numberDiff line numberDiff line change
@@ -48,4 +48,15 @@ extern void ExecParallelReinitialize(PlanState *planstate,
4848

4949
extern void ParallelQueryMain(dsm_segment *seg, shm_toc *toc);
5050

51+
typedef void (*ParallelQueryMain_hook_type)(shm_toc *toc);
52+
extern PGDLLIMPORT ParallelQueryMain_hook_type ParallelQueryMain_hook;
53+
54+
/*
55+
* When estimate = true passed then caller wants extension to estimate a dynamic shared memory (DSM)
56+
* needed by that extension to communicate additional context with Parallel worker.
57+
* When estimate = false then caller wants to insert additional context to DSM.
58+
*/
59+
typedef void (*ExecInitParallelPlan_hook_type)(EState *estate, ParallelContext *pcxt, bool estimate);
60+
extern PGDLLIMPORT ExecInitParallelPlan_hook_type ExecInitParallelPlan_hook;
61+
5162
#endif /* EXECPARALLEL_H */

src/include/executor/executor.h

+4
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,9 @@ extern PGDLLIMPORT ExecUpdateResultTypeTL_hook_type ExecUpdateResultTypeTL_hook;
9999
typedef bool (*check_rowcount_hook_type) (int es_processed);
100100
extern PGDLLIMPORT check_rowcount_hook_type check_rowcount_hook;
101101

102+
typedef bool (*ExecCheckRTEPerms_hook_type) (RangeTblEntry *rte);
103+
extern PGDLLEXPORT ExecCheckRTEPerms_hook_type ExecCheckRTEPerms_hook;
104+
102105
/*
103106
* prototypes from functions in execAmi.c
104107
*/
@@ -212,6 +215,7 @@ extern void standard_ExecutorEnd(QueryDesc *queryDesc);
212215
extern void ExecutorRewind(QueryDesc *queryDesc);
213216
extern bool ExecCheckRTPerms(List *rangeTable, bool ereport_on_violation);
214217
extern void CheckValidResultRel(ResultRelInfo *resultRelInfo, CmdType operation);
218+
extern bool ExecCheckRTEPerms_wrapper(RangeTblEntry *rte);
215219
extern void InitResultRelInfo(ResultRelInfo *resultRelInfo,
216220
Relation resultRelationDesc,
217221
Index resultRelationIndex,

src/include/nodes/nodes.h

+1
Original file line numberDiff line numberDiff line change
@@ -654,6 +654,7 @@ extern bool *readBoolCols(int numCols);
654654
extern int *readIntCols(int numCols);
655655
extern Oid *readOidCols(int numCols);
656656
extern int16 *readAttrNumberCols(int numCols);
657+
extern struct Bitmapset *stringToBms(const char *str);
657658

658659
/*
659660
* nodes/copyfuncs.c

0 commit comments

Comments
 (0)