This is an automated email from the ASF dual-hosted git repository. reshke pushed a commit to branch cbdb-postgres-merge in repository https://gitbox.apache.org/repos/asf/cloudberry.git
commit bc2833e00611af9c42a4e6f89e51401a4ea3761a Author: reshke <[email protected]> AuthorDate: Sat Dec 27 19:52:32 2025 +0000 Adapt GPORCA with new query relation permission checking Resolve with repsect to a61b1f74823c9c4f79c95226a461f1e7a367764b. Store requiredPerms and checkAsUser info in separare list inside CContextDXLToPlStmt. --- src/backend/gpopt/gpdbwrappers.cpp | 15 +++++++-- .../gpopt/translate/CContextDXLToPlStmt.cpp | 26 ++++++++++++++++ .../gpopt/translate/CTranslatorDXLToPlStmt.cpp | 36 ++++++++++++++++------ .../gpopt/translate/CTranslatorQueryToDXL.cpp | 26 +++++++++++----- src/backend/gpopt/translate/CTranslatorUtils.cpp | 9 +++--- src/backend/optimizer/plan/orca.c | 6 +++- src/include/executor/executor.h | 1 - src/include/gpopt/gpdbwrappers.h | 5 ++- src/include/gpopt/translate/CContextDXLToPlStmt.h | 16 ++++++++++ .../gpopt/translate/CTranslatorQueryToDXL.h | 2 +- src/include/gpopt/translate/CTranslatorUtils.h | 3 +- 11 files changed, 117 insertions(+), 28 deletions(-) diff --git a/src/backend/gpopt/gpdbwrappers.cpp b/src/backend/gpopt/gpdbwrappers.cpp index bf1f6cd9d0b..92f7b8bacb0 100644 --- a/src/backend/gpopt/gpdbwrappers.cpp +++ b/src/backend/gpopt/gpdbwrappers.cpp @@ -2157,11 +2157,11 @@ gpdb::CdbHashRandomSeg(int num_segments) // check permissions on range table void -gpdb::CheckRTPermissions(List *rtable) +gpdb::CheckRTPermissions(List *rtable, List *rteperminfos) { GP_WRAP_START; { - ExecCheckRTPerms(rtable, true); + ExecCheckPermissions(rtable, rteperminfos, true); return; } GP_WRAP_END; @@ -2816,4 +2816,15 @@ gpdb::TestexprIsHashable(Node *testexpr, List *param_ids) return false; } +RTEPermissionInfo * +gpdb::GetRTEPermissionInfo(List *rteperminfos, + const RangeTblEntry *rte) +{ + GP_WRAP_START; + { + return getRTEPermissionInfo(rteperminfos, (RangeTblEntry *) rte); + } + GP_WRAP_END; +} + // EOF diff --git a/src/backend/gpopt/translate/CContextDXLToPlStmt.cpp b/src/backend/gpopt/translate/CContextDXLToPlStmt.cpp index b0d7caef331..16eec1ba5a6 100644 --- a/src/backend/gpopt/translate/CContextDXLToPlStmt.cpp +++ b/src/backend/gpopt/translate/CContextDXLToPlStmt.cpp @@ -51,6 +51,7 @@ CContextDXLToPlStmt::CContextDXLToPlStmt( m_param_types_list(NIL), m_distribution_hashops(distribution_hashops), m_rtable_entries_list(nullptr), + m_perminfo_list(nullptr), m_subplan_entries_list(nullptr), m_subplan_sliceids_list(nullptr), m_slices_list(nullptr), @@ -590,4 +591,29 @@ CContextDXLToPlStmt::GetRTEIndexByAssignedQueryId( return gpdb::ListLength(m_rtable_entries_list) + 1; } + +//--------------------------------------------------------------------------- +// @function: +// CContextDXLToPlStmt::AddPerfmInfo +// +// @doc: +// Add a Perfission Info list entry +// +//--------------------------------------------------------------------------- +void +CContextDXLToPlStmt::AddPerfmInfo(RTEPermissionInfo *pi) +{ + // add rte to rtable entries list + m_perminfo_list = gpdb::LAppend(m_perminfo_list, pi); +} + + +RTEPermissionInfo * +CContextDXLToPlStmt::GetPermInfoByIndex(Index index) +{ + + return (RTEPermissionInfo *) gpdb::ListNth(m_perminfo_list, + int(index - 1)); +} + // EOF diff --git a/src/backend/gpopt/translate/CTranslatorDXLToPlStmt.cpp b/src/backend/gpopt/translate/CTranslatorDXLToPlStmt.cpp index f862e4c549a..0d16ea490e6 100644 --- a/src/backend/gpopt/translate/CTranslatorDXLToPlStmt.cpp +++ b/src/backend/gpopt/translate/CTranslatorDXLToPlStmt.cpp @@ -244,6 +244,7 @@ CTranslatorDXLToPlStmt::GetPlannedStmtFromDXL(const CDXLNode *dxlnode, planned_stmt->planGen = PLANGEN_OPTIMIZER; planned_stmt->rtable = m_dxl_to_plstmt_context->GetRTableEntriesList(); + planned_stmt->permInfos = m_dxl_to_plstmt_context->GetPermInfosList(); planned_stmt->subplans = m_dxl_to_plstmt_context->GetSubplanEntriesList(); planned_stmt->planTree = plan; @@ -668,8 +669,8 @@ CTranslatorDXLToPlStmt::TranslateDXLTblScan( else { SeqScan *seq_scan = MakeNode(SeqScan); - seq_scan->scanrelid = index; - plan = &(seq_scan->plan); + seq_scan->scan.scanrelid = index; + plan = &(seq_scan->scan.plan); plan_return = (Plan *) seq_scan; plan->targetlist = targetlist; @@ -1846,6 +1847,7 @@ CTranslatorDXLToPlStmt::TranslateDXLTvfToRangeTblEntry( rte->functions = ListMake1(rtfunc); rte->inFromCl = true; + rte->perminfoindex = 0; rte->eref = alias; return rte; @@ -1868,8 +1870,8 @@ CTranslatorDXLToPlStmt::TranslateDXLValueScanToRangeTblEntry( rte->rtekind = RTE_VALUES; rte->inh = false; /* never true for values RTEs */ rte->inFromCl = true; - rte->requiredPerms = 0; - rte->checkAsUser = InvalidOid; + /* No permission checks */ + rte->perminfoindex = 0; Alias *alias = MakeNode(Alias); alias->colnames = NIL; @@ -4271,7 +4273,7 @@ CTranslatorDXLToPlStmt::TranslateDXLDynTblScan( // create dynamic scan node DynamicSeqScan *dyn_seq_scan = MakeNode(DynamicSeqScan); - dyn_seq_scan->seqscan.scanrelid = index; + dyn_seq_scan->seqscan.scan.scanrelid = index; const CDXLTableDescr *dxl_table_descr = dyn_tbl_scan_dxlop->GetDXLTableDescr(); @@ -4293,7 +4295,7 @@ CTranslatorDXLToPlStmt::TranslateDXLDynTblScan( TranslateJoinPruneParamids(dyn_tbl_scan_dxlop->GetSelectorIds(), oid_type, m_dxl_to_plstmt_context); - Plan *plan = &(dyn_seq_scan->seqscan.plan); + Plan *plan = &(dyn_seq_scan->seqscan.scan.plan); plan->plan_node_id = m_dxl_to_plstmt_context->GetNextPlanId(); //plan->nMotionNodes = 0; @@ -4510,7 +4512,7 @@ RemapAttrsFromTupDesc(TupleDesc fromDesc, TupleDesc toDesc, Index index, List *qual, List *targetlist) { AttrMap *attMap; - attMap = build_attrmap_by_name_if_req(toDesc, fromDesc); + attMap = build_attrmap_by_name_if_req(toDesc, fromDesc, false); /* If attribute remapping is not necessary, then do not change the varattno */ if (attMap) @@ -5266,16 +5268,24 @@ CTranslatorDXLToPlStmt::ProcessDXLTblDescr( { RangeTblEntry *rte = m_dxl_to_plstmt_context->GetRTEByIndex(index); GPOS_ASSERT(nullptr != rte); - rte->requiredPerms |= required_perms; + + if (rte->perminfoindex != 0) + { + RTEPermissionInfo *pi = m_dxl_to_plstmt_context->GetPermInfoByIndex(rte->perminfoindex); + pi->requiredPerms |= required_perms; + } + return index; } // create a new RTE (and it's alias) and store it at context rtable list RangeTblEntry *rte = MakeNode(RangeTblEntry); + // A perm info entry corresponding this rte. + RTEPermissionInfo *pi = MakeNode(RTEPermissionInfo); rte->rtekind = RTE_RELATION; rte->relid = oid; - rte->checkAsUser = table_descr->GetExecuteAsUserId(); - rte->requiredPerms |= required_perms; + pi->checkAsUser = table_descr->GetExecuteAsUserId(); + pi->requiredPerms |= required_perms; rte->rellockmode = table_descr->LockMode(); Alias *alias = MakeNode(Alias); @@ -5325,6 +5335,12 @@ CTranslatorDXLToPlStmt::ProcessDXLTblDescr( rte->eref = alias; rte->alias = alias; + m_dxl_to_plstmt_context->AddPerfmInfo(pi); + + // set up rte <> perm info link. + rte->perminfoindex = gpdb::ListLength( + m_dxl_to_plstmt_context->GetPermInfosList()); + // A new RTE is added to the range table entries list if it's not found in the look // up table. However, it is only added to the look up table if it's a result relation // This is because the look up table is our way of merging duplicate result relations diff --git a/src/backend/gpopt/translate/CTranslatorQueryToDXL.cpp b/src/backend/gpopt/translate/CTranslatorQueryToDXL.cpp index 392f650664e..700b596c93b 100644 --- a/src/backend/gpopt/translate/CTranslatorQueryToDXL.cpp +++ b/src/backend/gpopt/translate/CTranslatorQueryToDXL.cpp @@ -608,7 +608,7 @@ CTranslatorQueryToDXL::TranslateSelectQueryToDXL() // In Orca, we only keep range table entries for the base tables in the planned statement, but not for the view itself. // Since permissions are only checked during ExecutorStart, we lose track of the permissions required for the view and the select goes through successfully. // We therefore need to check permissions before we go into optimization for all RTEs, including the ones not explicitly referred in the query, e.g. views. - CTranslatorUtils::CheckRTEPermissions(m_query->rtable); + CTranslatorUtils::CheckRTEPermissions(m_query->rtable, m_query->rteperminfos); if (m_query->hasForUpdate) { @@ -861,8 +861,11 @@ CTranslatorQueryToDXL::TranslateInsertQueryToDXL() GPOS_RAISE(gpdxl::ExmaDXL, gpdxl::ExmiQuery2DXLUnsupportedFeature, GPOS_WSZ_LIT("Inserts with foreign tables")); } + const RTEPermissionInfo *perminfo = gpdb::GetRTEPermissionInfo( + m_query->rtable, rte); + CDXLTableDescr *table_descr = CTranslatorUtils::GetTableDescr( - m_mp, m_md_accessor, m_context->m_colid_counter, rte, m_query_id, + m_mp, m_md_accessor, m_context->m_colid_counter, rte, perminfo, m_query_id, &m_context->m_has_distributed_tables); const IMDRelation *md_rel = m_md_accessor->RetrieveRel(table_descr->MDId()); @@ -1184,8 +1187,11 @@ CTranslatorQueryToDXL::TranslateDeleteQueryToDXL() GPOS_RAISE(gpdxl::ExmaDXL, gpdxl::ExmiQuery2DXLUnsupportedFeature, GPOS_WSZ_LIT("Deletes with foreign tables")); } + const RTEPermissionInfo *perminfo = gpdb::GetRTEPermissionInfo( + m_query->rtable, rte); + CDXLTableDescr *table_descr = CTranslatorUtils::GetTableDescr( - m_mp, m_md_accessor, m_context->m_colid_counter, rte, m_query_id, + m_mp, m_md_accessor, m_context->m_colid_counter, rte, perminfo, m_query_id, &m_context->m_has_distributed_tables); const IMDRelation *md_rel = m_md_accessor->RetrieveRel(table_descr->MDId()); @@ -1261,14 +1267,16 @@ CTranslatorQueryToDXL::TranslateUpdateQueryToDXL() CDXLNode *query_dxlnode = TranslateSelectQueryToDXL(); const RangeTblEntry *rte = (RangeTblEntry *) gpdb::ListNth( m_query->rtable, m_query->resultRelation - 1); - if (rte->relkind == 'f') { GPOS_RAISE(gpdxl::ExmaDXL, gpdxl::ExmiQuery2DXLUnsupportedFeature, GPOS_WSZ_LIT("Updates with foreign tables")); } + const RTEPermissionInfo *perminfo = gpdb::GetRTEPermissionInfo( + m_query->rtable, rte); + CDXLTableDescr *table_descr = CTranslatorUtils::GetTableDescr( - m_mp, m_md_accessor, m_context->m_colid_counter, rte, m_query_id, + m_mp, m_md_accessor, m_context->m_colid_counter, rte, perminfo, m_query_id, &m_context->m_has_distributed_tables); const IMDRelation *md_rel = m_md_accessor->RetrieveRel(table_descr->MDId()); @@ -3258,6 +3266,9 @@ CTranslatorQueryToDXL::TranslateFromClauseToDXL(Node *node) GPOS_WSZ_LIT("WITH ORDINALITY")); } + const RTEPermissionInfo *perminfo = gpdb::GetRTEPermissionInfo(m_query->rtable, + rte); + switch (rte->rtekind) { default: @@ -3268,7 +3279,7 @@ CTranslatorQueryToDXL::TranslateFromClauseToDXL(Node *node) } case RTE_RELATION: { - return TranslateRTEToDXLLogicalGet(rte, rt_index, + return TranslateRTEToDXLLogicalGet(rte, perminfo, rt_index, m_query_level); } case RTE_VALUES: @@ -3356,6 +3367,7 @@ CTranslatorQueryToDXL::UnsupportedRTEKind(RTEKind rtekind) //--------------------------------------------------------------------------- CDXLNode * CTranslatorQueryToDXL::TranslateRTEToDXLLogicalGet(const RangeTblEntry *rte, + const RTEPermissionInfo *perminfo, ULONG rt_index, ULONG //current_query_level ) @@ -3384,7 +3396,7 @@ CTranslatorQueryToDXL::TranslateRTEToDXLLogicalGet(const RangeTblEntry *rte, // construct table descriptor for the scan node from the range table entry CDXLTableDescr *dxl_table_descr = CTranslatorUtils::GetTableDescr( - m_mp, m_md_accessor, m_context->m_colid_counter, rte, + m_mp, m_md_accessor, m_context->m_colid_counter, rte, perminfo, query_id_for_target_rel, &m_context->m_has_distributed_tables); CDXLLogicalGet *dxl_op = nullptr; diff --git a/src/backend/gpopt/translate/CTranslatorUtils.cpp b/src/backend/gpopt/translate/CTranslatorUtils.cpp index 0887c72725c..e87e8636d56 100644 --- a/src/backend/gpopt/translate/CTranslatorUtils.cpp +++ b/src/backend/gpopt/translate/CTranslatorUtils.cpp @@ -114,6 +114,7 @@ CDXLTableDescr * CTranslatorUtils::GetTableDescr(CMemoryPool *mp, CMDAccessor *md_accessor, CIdGenerator *id_generator, const RangeTblEntry *rte, + const RTEPermissionInfo *perminfo, ULONG assigned_query_id_for_target_rel, BOOL *is_distributed_table // output ) @@ -131,9 +132,9 @@ CTranslatorUtils::GetTableDescr(CMemoryPool *mp, CMDAccessor *md_accessor, GPOS_NEW(mp) CWStringConst(mp, rte->alias->aliasname), true) : GPOS_NEW(mp) CMDName(mp, rel->Mdname().GetMDName()); - ULONG required_perms = static_cast<ULONG>(rte->requiredPerms); + ULONG required_perms = static_cast<ULONG>(perminfo->requiredPerms); CDXLTableDescr *table_descr = GPOS_NEW(mp) CDXLTableDescr( - mp, mdid, table_mdname, rte->checkAsUser, rte->rellockmode, + mp, mdid, table_mdname, perminfo->checkAsUser, rte->rellockmode, required_perms, assigned_query_id_for_target_rel); const ULONG len = rel->ColumnCount(); @@ -2173,9 +2174,9 @@ CTranslatorUtils::CreateDXLProjElemConstNULL(CMemoryPool *mp, // //--------------------------------------------------------------------------- void -CTranslatorUtils::CheckRTEPermissions(List *range_table_list) +CTranslatorUtils::CheckRTEPermissions(List *range_table_list, List *rteperminfos) { - gpdb::CheckRTPermissions(range_table_list); + gpdb::CheckRTPermissions(range_table_list, rteperminfos); } diff --git a/src/backend/optimizer/plan/orca.c b/src/backend/optimizer/plan/orca.c index 0d706129d58..ffebbc8d8d7 100644 --- a/src/backend/optimizer/plan/orca.c +++ b/src/backend/optimizer/plan/orca.c @@ -518,6 +518,7 @@ transformGroupedWindows(Node *node, void *context) Query *subq; RangeTblEntry *rte; + RTEPermissionInfo *perminfo; RangeTblRef *ref; Alias *alias; bool hadSubLinks = qry->hasSubLinks; @@ -576,7 +577,9 @@ transformGroupedWindows(Node *node, void *context) rte->alias = NULL; /* fill in later */ rte->eref = NULL; /* fill in later */ rte->inFromCl = true; - rte->requiredPerms = ACL_SELECT; + + perminfo = makeNode(RTEPermissionInfo); + perminfo->requiredPerms = ACL_SELECT; /* * Default? rte->inh = 0; rte->checkAsUser = 0; @@ -602,6 +605,7 @@ transformGroupedWindows(Node *node, void *context) /* Core of outer query input table expression: */ qry->rtable = list_make1(rte); + qry->rteperminfos = list_make1(perminfo); qry->jointree = (FromExpr *) makeNode(FromExpr); qry->jointree->fromlist = list_make1(ref); qry->jointree->quals = NULL; diff --git a/src/include/executor/executor.h b/src/include/executor/executor.h index f675df1dfcb..466097c94c7 100644 --- a/src/include/executor/executor.h +++ b/src/include/executor/executor.h @@ -235,7 +235,6 @@ extern void standard_ExecutorFinish(QueryDesc *queryDesc); extern void ExecutorEnd(QueryDesc *queryDesc); extern void standard_ExecutorEnd(QueryDesc *queryDesc); extern void ExecutorRewind(QueryDesc *queryDesc); -extern bool ExecCheckRTPerms(List *rangeTable, bool ereport_on_violation); extern void CheckValidResultRel(ResultRelInfo *resultRelInfo, CmdType operation, ModifyTableState *mtstate); extern bool ExecCheckPermissions(List *rangeTable, List *rteperminfos, bool ereport_on_violation); diff --git a/src/include/gpopt/gpdbwrappers.h b/src/include/gpopt/gpdbwrappers.h index 188b6834b7b..feb9a0273fe 100644 --- a/src/include/gpopt/gpdbwrappers.h +++ b/src/include/gpopt/gpdbwrappers.h @@ -574,7 +574,7 @@ int32 CdbHashConstList(List *constants, int num_segments, Oid *hashfuncs); unsigned int CdbHashRandomSeg(int num_segments); // check permissions on range table -void CheckRTPermissions(List *rtable); +void CheckRTPermissions(List *rtable, List *rteperminfos); // throw an error if table has update triggers. bool HasUpdateTriggers(Oid relid); @@ -683,6 +683,9 @@ IndexAmRoutine *GetIndexAmRoutineFromAmHandler(Oid am_handler); bool TestexprIsHashable(Node *testexpr, List *param_ids); +RTEPermissionInfo * +GetRTEPermissionInfo(List *rteperminfos, const RangeTblEntry *rte); + gpos::BOOL WalkQueryTree(Query *query, bool (*walker)(), void *context, int flags); diff --git a/src/include/gpopt/translate/CContextDXLToPlStmt.h b/src/include/gpopt/translate/CContextDXLToPlStmt.h index f10d45456f4..1b88fb56711 100644 --- a/src/include/gpopt/translate/CContextDXLToPlStmt.h +++ b/src/include/gpopt/translate/CContextDXLToPlStmt.h @@ -109,6 +109,9 @@ private: // list of all rtable entries List *m_rtable_entries_list; + // list of all rtable entries + List *m_perminfo_list; + // list of all subplan entries List *m_subplan_entries_list; List *m_subplan_sliceids_list; @@ -170,6 +173,13 @@ public: return m_rtable_entries_list; } + // return list of perfinfos + List * + GetPermInfosList() const + { + return m_perminfo_list; + } + List * GetSubplanEntriesList() const { @@ -238,6 +248,12 @@ public: Index GetRTEIndexByAssignedQueryId(ULONG assigned_query_id_for_target_rel, BOOL *is_rte_exists); + + // add a perm info. + void AddPerfmInfo(RTEPermissionInfo *pi); + + // get perm info from m_perminfo_list by given index + RTEPermissionInfo *GetPermInfoByIndex(Index index); }; } // namespace gpdxl diff --git a/src/include/gpopt/translate/CTranslatorQueryToDXL.h b/src/include/gpopt/translate/CTranslatorQueryToDXL.h index c74892e5df2..d5fa59bc025 100644 --- a/src/include/gpopt/translate/CTranslatorQueryToDXL.h +++ b/src/include/gpopt/translate/CTranslatorQueryToDXL.h @@ -303,7 +303,7 @@ private: ULONG current_query_level); // translate a base table range table entry into a logical get - CDXLNode *TranslateRTEToDXLLogicalGet(const RangeTblEntry *rte, ULONG rti, + CDXLNode *TranslateRTEToDXLLogicalGet(const RangeTblEntry *rte, const RTEPermissionInfo *perminfo, ULONG rti, ULONG //current_query_level ); diff --git a/src/include/gpopt/translate/CTranslatorUtils.h b/src/include/gpopt/translate/CTranslatorUtils.h index b6187ad099b..5d72dfe6cbf 100644 --- a/src/include/gpopt/translate/CTranslatorUtils.h +++ b/src/include/gpopt/translate/CTranslatorUtils.h @@ -154,6 +154,7 @@ public: CMDAccessor *md_accessor, CIdGenerator *id_generator, const RangeTblEntry *rte, + const RTEPermissionInfo *perminfo, ULONG assigned_query_id_for_target_rel, BOOL *is_distributed_table = nullptr); @@ -340,7 +341,7 @@ public: const IMDColumn *col); // check required permissions for the range table - static void CheckRTEPermissions(List *range_table_list); + static void CheckRTEPermissions(List *range_table_list, List *rteperminfos); // check if given column ids are outer references in the tree rooted by given node static void MarkOuterRefs(ULONG *colid, BOOL *is_outer_ref, --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
