From f0dc72af6be2801c3f920e179ec4022702bbbf32 Mon Sep 17 00:00:00 2001 From: HustonMmmavr Date: Thu, 26 Jan 2023 16:36:19 +0300 Subject: [PATCH 1/2] Fix EPQ for DML operations (backport of #14304) **Issue** Currently EPQ mechanism doesn't work correctly in ORCA. **Example** ```sql set gp_enable_global_deadlock_detector = on create table test as select 0 as i distributed randomly; -- Start transaction -- First session increments the value from 0 to 1 1: begin; 1: update test set i = i + 1; -- Second session attempts to delete the old value 0 2: delete from test where i = 0; 1: end; -- The updated row ends up being deleted -- This is not the expected behavior ``` **Root cause** As shown in the following plan tree, `MODIFYTABLE->resultRelations` indicates rte 1 is the result relation. However, `SEQSCAN->scanrelid` points to rte 2. This causes the wrong slot (in this case the cache slot with the old value) being handed for deletion operation. Whereas the rte links are correct in the query parsing stage (parse tree), the information isn't transferred to the algebrized query (logical expression). ``` {PLANNEDSTMT ... :planTree {MODIFYTABLE ... :resultRelations (i 1) ... :plans ( {SEQSCAN ... :plan_node_id 1 ... :scanrelid 2 ... } ) ... } :rtable ( {RTE :alias <> :eref {ALIAS :aliasname test :colnames (""i"") } :relid 16548 :requiredPerms 8 ... } {RTE :alias <> :eref {ALIAS :aliasname test :colnames (""i"") } :relid 16548 :requiredPerms 2 ... } ) :resultRelations (i 1) :relationOids (o 16548 16548 16548) ... } ``` **Solution** We assign a query id to each query structure, including subqueries. The query id would be attached to the table descriptors, and is used to direct us to the target relation of DML. The idea is to maintain a `query id --> rte index` map. A new rte would be added only if it's not already in the map. An old rte would be reused, potentially having its permission (SELECT/INSERT/UPDATE/DELETE) updated. Multiple query id's are allowed to be directed to the same rte. With this patch, the rte's in the plan tree are deduplicated. --- .../gpopt/translate/CContextDXLToPlStmt.cpp | 77 +++ .../gpopt/translate/CContextQueryToDXL.cpp | 8 + .../translate/CTranslatorDXLToPlStmt.cpp | 181 ++++--- .../gpopt/translate/CTranslatorQueryToDXL.cpp | 21 +- .../gpopt/translate/CTranslatorUtils.cpp | 8 +- ...lete-Check-AssignedQueryIdForTargetRel.mdp | 455 ++++++++++++++++++ .../include/gpopt/metadata/CTableDescriptor.h | 15 +- .../src/metadata/CTableDescriptor.cpp | 6 +- .../src/translate/CTranslatorDXLToExpr.cpp | 7 +- .../src/translate/CTranslatorExprToDXL.cpp | 6 +- .../naucrates/dxl/operators/CDXLTableDescr.h | 15 +- .../include/naucrates/dxl/xml/dxltokens.h | 1 + .../src/operators/CDXLOperatorFactory.cpp | 7 +- .../src/operators/CDXLTableDescr.cpp | 30 +- .../gporca/libnaucrates/src/xml/dxltokens.cpp | 2 + .../gporca/server/src/unittest/CTestUtils.cpp | 3 +- .../dxl/statistics/CStatisticsTest.cpp | 3 +- .../src/unittest/gpopt/minidump/CDMLTest.cpp | 1 + .../translate/CTranslatorDXLToExprTest.cpp | 3 +- .../gpopt/translate/CContextDXLToPlStmt.h | 13 + .../gpopt/translate/CContextQueryToDXL.h | 6 + .../gpopt/translate/CTranslatorDXLToPlStmt.h | 6 +- .../gpopt/translate/CTranslatorQueryToDXL.h | 4 + .../gpopt/translate/CTranslatorUtils.h | 10 +- .../modify_table_data_corrupt_optimizer.out | 2 +- .../uao/parallel_delete_optimizer.source | 6 +- .../uao/parallel_update_optimizer.source | 6 +- .../regress/expected/ao_locks_optimizer.out | 12 +- .../regress/expected/bfv_dml_optimizer.out | 4 +- .../regress/expected/partition_locking.out | 2 + .../expected/partition_locking_optimizer.out | 13 +- .../expected/updatable_views_optimizer.out | 8 +- .../regress/expected/update_optimizer.out | 2 +- src/test/regress/sql/partition_locking.sql | 2 + 34 files changed, 786 insertions(+), 159 deletions(-) create mode 100644 src/backend/gporca/data/dxl/minidump/Delete-Check-AssignedQueryIdForTargetRel.mdp diff --git a/src/backend/gpopt/translate/CContextDXLToPlStmt.cpp b/src/backend/gpopt/translate/CContextDXLToPlStmt.cpp index 2ee1f329c8ba..2462059a14b7 100644 --- a/src/backend/gpopt/translate/CContextDXLToPlStmt.cpp +++ b/src/backend/gpopt/translate/CContextDXLToPlStmt.cpp @@ -60,6 +60,7 @@ CContextDXLToPlStmt::CContextDXLToPlStmt( { m_cte_consumer_info = GPOS_NEW(m_mp) HMUlCTEConsumerInfo(m_mp); m_num_partition_selectors_array = GPOS_NEW(m_mp) ULongPtrArray(m_mp); + m_used_rte_indexes = GPOS_NEW(m_mp) HMUlIndex(m_mp); } //--------------------------------------------------------------------------- @@ -74,6 +75,7 @@ CContextDXLToPlStmt::~CContextDXLToPlStmt() { m_cte_consumer_info->Release(); m_num_partition_selectors_array->Release(); + m_used_rte_indexes->Release(); } //--------------------------------------------------------------------------- @@ -462,4 +464,79 @@ CContextDXLToPlStmt::GetDistributionHashFuncForType(Oid typid) return hashproc; } +RangeTblEntry * +CContextDXLToPlStmt::GetRTEByIndex(Index index) +{ + return (RangeTblEntry *) gpdb::ListNth(*(m_rtable_entries_list), + int(index - 1)); +} + +//--------------------------------------------------------------------------- +// @function: of associated +// CContextDXLToPlStmt::GetRTEIndexByTableDescr +// +// @doc: +// +// For given table descriptor this function returns index of rte in +// m_rtable_entries_list for furhter processing and set a flag that +// rte was processed. +// In case of DML operations there is more than one table descr pointing +// to the result relation and to detect position of already processed rte +// `assigned_query_id_for_target_rel` of table descriptor is used. +//--------------------------------------------------------------------------- +Index +CContextDXLToPlStmt::GetRTEIndexByTableDescr(const CDXLTableDescr *table_descr, + BOOL *is_rte_exists) +{ + *is_rte_exists = false; + + // `assigned_query_id_for_target_rel` is a "tag" of table descriptors, it + // shows id of query structure which contains result relation. If table + // descriptors have the same `assigned_query_id_for_target_rel` - these + // table descriptors point to the same result relation in `ModifyTable` + // operation. It's not zero (0) value (which equal to `UNASSIGNED_QUERYID` + // define) if: user query is a INSERT/UPDATE/DELETE (`ModifyTable` + // operation) and this table descriptor points to the result relation of + // operation, for ex.: + // ```sql + // create table b (i int, j int); + // create table c (i int); + // insert into b(i,j) values (1,2), (2,3), (3,4); + // insert into c(i) values (1), (2); + // delete from b where i in (select i from c); + // ``` + // where `b` is a result relation (table descriptors pointing to it + // will have the same `assigned_query_id_for_target_rel` > 0), and + // `c` is not (all table descriptors which points to `c` will have + // `assigned_query_id_for_target_rel`=0 (equal to `UNASSIGNED_QUERYID`) + ULONG assigned_query_id_for_target_rel = + table_descr->GetAssignedQueryIdForTargetRel(); + if (assigned_query_id_for_target_rel == UNASSIGNED_QUERYID) + { + return gpdb::ListLength(*(m_rtable_entries_list)) + 1; + } + + Index *usedIndex = + m_used_rte_indexes->Find(&assigned_query_id_for_target_rel); + + // `usedIndex` is a non NULL value in next case: table descriptor with + // the same `assigned_query_id_for_target_rel` was processed previously + // (so no need to create a new index for result relation like the relation + // itself) + if (usedIndex) + { + *is_rte_exists = true; + return *usedIndex; + } + + // `assigned_query_id_for_target_rel` of table descriptor which points to + // result relation wasn't previously processed - create a new index. + Index new_index = gpdb::ListLength(*(m_rtable_entries_list)) + 1; + m_used_rte_indexes->Insert(GPOS_NEW(m_mp) + ULONG(assigned_query_id_for_target_rel), + GPOS_NEW(m_mp) Index(new_index)); + + return new_index; +} + // EOF diff --git a/src/backend/gpopt/translate/CContextQueryToDXL.cpp b/src/backend/gpopt/translate/CContextQueryToDXL.cpp index 7cbd795fc742..7dc5630ff6f1 100644 --- a/src/backend/gpopt/translate/CContextQueryToDXL.cpp +++ b/src/backend/gpopt/translate/CContextQueryToDXL.cpp @@ -28,11 +28,19 @@ CContextQueryToDXL::CContextQueryToDXL(CMemoryPool *mp) { // map that stores gpdb att to optimizer col mapping m_colid_counter = GPOS_NEW(mp) CIdGenerator(GPDXL_COL_ID_START); + m_queryid_counter = GPOS_NEW(mp) CIdGenerator(GPDXL_QUERY_ID_START); m_cte_id_counter = GPOS_NEW(mp) CIdGenerator(GPDXL_CTE_ID_START); } CContextQueryToDXL::~CContextQueryToDXL() { + GPOS_DELETE(m_queryid_counter); GPOS_DELETE(m_colid_counter); GPOS_DELETE(m_cte_id_counter); } + +ULONG +CContextQueryToDXL::GetNextQueryId() +{ + return m_queryid_counter->next_id(); +} diff --git a/src/backend/gpopt/translate/CTranslatorDXLToPlStmt.cpp b/src/backend/gpopt/translate/CTranslatorDXLToPlStmt.cpp index 878f638d2180..66e77520e92f 100644 --- a/src/backend/gpopt/translate/CTranslatorDXLToPlStmt.cpp +++ b/src/backend/gpopt/translate/CTranslatorDXLToPlStmt.cpp @@ -484,19 +484,13 @@ CTranslatorDXLToPlStmt::TranslateDXLTblScan( // translation context for column mappings in the base relation CDXLTranslateContextBaseTable base_table_context(m_mp); - // we will add the new range table entry as the last element of the range table - Index index = - gpdb::ListLength(m_dxl_to_plstmt_context->GetRTableEntriesList()) + 1; - const CDXLTableDescr *dxl_table_descr = phy_tbl_scan_dxlop->GetDXLTableDescr(); const IMDRelation *md_rel = m_md_accessor->RetrieveRel(dxl_table_descr->MDId()); - RangeTblEntry *rte = TranslateDXLTblDescrToRangeTblEntry( - dxl_table_descr, index, &base_table_context); - GPOS_ASSERT(NULL != rte); - rte->requiredPerms |= ACL_SELECT; - m_dxl_to_plstmt_context->AddRTE(rte); + + Index index = + ProcessDXLTblDescr(dxl_table_descr, &base_table_context, ACL_SELECT); Plan *plan = NULL; Plan *plan_return = NULL; @@ -658,18 +652,13 @@ CTranslatorDXLToPlStmt::TranslateDXLIndexScan( // translation context for column mappings in the base relation CDXLTranslateContextBaseTable base_table_context(m_mp); - Index index = - gpdb::ListLength(m_dxl_to_plstmt_context->GetRTableEntriesList()) + 1; - - const IMDRelation *md_rel = m_md_accessor->RetrieveRel( - physical_idx_scan_dxlop->GetDXLTableDescr()->MDId()); + const CDXLTableDescr *dxl_table_descr = + physical_idx_scan_dxlop->GetDXLTableDescr(); + const IMDRelation *md_rel = + m_md_accessor->RetrieveRel(dxl_table_descr->MDId()); - RangeTblEntry *rte = TranslateDXLTblDescrToRangeTblEntry( - physical_idx_scan_dxlop->GetDXLTableDescr(), index, - &base_table_context); - GPOS_ASSERT(NULL != rte); - rte->requiredPerms |= ACL_SELECT; - m_dxl_to_plstmt_context->AddRTE(rte); + Index index = + ProcessDXLTblDescr(dxl_table_descr, &base_table_context, ACL_SELECT); IndexScan *index_scan = NULL; index_scan = MakeNode(IndexScan); @@ -800,18 +789,11 @@ CTranslatorDXLToPlStmt::TranslateDXLIndexOnlyScan( // translation context for column mappings in the base relation CDXLTranslateContextBaseTable base_table_context(m_mp); - Index index = - gpdb::ListLength(m_dxl_to_plstmt_context->GetRTableEntriesList()) + 1; - const IMDRelation *md_rel = m_md_accessor->RetrieveRel( physical_idx_scan_dxlop->GetDXLTableDescr()->MDId()); - RangeTblEntry *rte = TranslateDXLTblDescrToRangeTblEntry( - physical_idx_scan_dxlop->GetDXLTableDescr(), index, - &base_table_context); - GPOS_ASSERT(NULL != rte); - rte->requiredPerms |= ACL_SELECT; - m_dxl_to_plstmt_context->AddRTE(rte); + Index index = + ProcessDXLTblDescr(table_desc, &base_table_context, ACL_SELECT); IndexOnlyScan *index_scan = MakeNode(IndexOnlyScan); index_scan->scan.scanrelid = index; @@ -3944,16 +3926,8 @@ CTranslatorDXLToPlStmt::TranslateDXLDynTblScan( // translation context for column mappings in the base relation CDXLTranslateContextBaseTable base_table_context(m_mp); - // add the new range table entry as the last element of the range table - Index index = - gpdb::ListLength(m_dxl_to_plstmt_context->GetRTableEntriesList()) + 1; - - RangeTblEntry *rte = TranslateDXLTblDescrToRangeTblEntry( - dyn_tbl_scan_dxlop->GetDXLTableDescr(), index, &base_table_context); - GPOS_ASSERT(NULL != rte); - rte->requiredPerms |= ACL_SELECT; - - m_dxl_to_plstmt_context->AddRTE(rte); + Index index = ProcessDXLTblDescr(dyn_tbl_scan_dxlop->GetDXLTableDescr(), + &base_table_context, ACL_SELECT); // create dynamic scan node DynamicSeqScan *dyn_seq_scan = MakeNode(DynamicSeqScan); @@ -4011,16 +3985,11 @@ CTranslatorDXLToPlStmt::TranslateDXLDynIdxScan( // translation context for column mappings in the base relation CDXLTranslateContextBaseTable base_table_context(m_mp); - Index index = - gpdb::ListLength(m_dxl_to_plstmt_context->GetRTableEntriesList()) + 1; + const CDXLTableDescr *table_desc = dyn_index_scan_dxlop->GetDXLTableDescr(); + const IMDRelation *md_rel = m_md_accessor->RetrieveRel(table_desc->MDId()); - const IMDRelation *md_rel = m_md_accessor->RetrieveRel( - dyn_index_scan_dxlop->GetDXLTableDescr()->MDId()); - RangeTblEntry *rte = TranslateDXLTblDescrToRangeTblEntry( - dyn_index_scan_dxlop->GetDXLTableDescr(), index, &base_table_context); - GPOS_ASSERT(NULL != rte); - rte->requiredPerms |= ACL_SELECT; - m_dxl_to_plstmt_context->AddRTE(rte); + Index index = + ProcessDXLTblDescr(table_desc, &base_table_context, ACL_SELECT); DynamicIndexScan *dyn_idx_scan = MakeNode(DynamicIndexScan); @@ -4033,9 +4002,12 @@ CTranslatorDXLToPlStmt::TranslateDXLDynIdxScan( CMDIdGPDB::CastMdid(dyn_index_scan_dxlop->GetDXLIndexDescr()->MDId()); const IMDIndex *md_index = m_md_accessor->RetrieveIndex(mdid_index); Oid index_oid = mdid_index->Oid(); + RangeTblEntry *rte = m_dxl_to_plstmt_context->GetRTEByIndex(index); GPOS_ASSERT(InvalidOid != index_oid); dyn_idx_scan->indexscan.indexid = index_oid; + + GPOS_ASSERT(NULL != rte); dyn_idx_scan->logicalIndexInfo = gpdb::GetLogicalIndexInfo(rte->relid, index_oid); @@ -4161,22 +4133,17 @@ CTranslatorDXLToPlStmt::TranslateDXLDml( // translation context for column mappings in the base relation CDXLTranslateContextBaseTable base_table_context(m_mp); - // add the new range table entry as the last element of the range table - Index index = - gpdb::ListLength(m_dxl_to_plstmt_context->GetRTableEntriesList()) + 1; - dml->scanrelid = index; - - m_result_rel_list = gpdb::LAppendInt(m_result_rel_list, index); + CDXLTableDescr *table_descr = phy_dml_dxlop->GetDXLTableDescr(); const IMDRelation *md_rel = m_md_accessor->RetrieveRel(phy_dml_dxlop->GetDXLTableDescr()->MDId()); - CDXLTableDescr *table_descr = phy_dml_dxlop->GetDXLTableDescr(); - RangeTblEntry *rte = TranslateDXLTblDescrToRangeTblEntry( - table_descr, index, &base_table_context); - GPOS_ASSERT(NULL != rte); - rte->requiredPerms |= acl_mode; - m_dxl_to_plstmt_context->AddRTE(rte); + Index index = + ProcessDXLTblDescr(table_descr, &base_table_context, acl_mode); + + dml->scanrelid = index; + + m_result_rel_list = gpdb::LAppendInt(m_result_rel_list, index); CDXLNode *project_list_dxlnode = (*dml_dxlnode)[0]; CDXLNode *child_dxlnode = (*dml_dxlnode)[1]; @@ -4656,40 +4623,74 @@ CTranslatorDXLToPlStmt::TranslateDXLRowTrigger( //--------------------------------------------------------------------------- // @function: -// CTranslatorDXLToPlStmt::TranslateDXLTblDescrToRangeTblEntry +// CTranslatorDXLToPlStmt::ProcessDXLTblDescr // // @doc: -// Translates a DXL table descriptor into a range table entry. If an index -// descriptor is provided, we use the mapping from colids to index attnos -// instead of table attnos -// -//--------------------------------------------------------------------------- -RangeTblEntry * -CTranslatorDXLToPlStmt::TranslateDXLTblDescrToRangeTblEntry( - const CDXLTableDescr *table_descr, Index index, - CDXLTranslateContextBaseTable *base_table_context) +// Translates a DXL table descriptor into a range table entry and stores +// it in m_dxl_to_plstmt_context if it's needed (in case of DML operations +// there is more than one table descriptors which point to the result +// relation, so if rte was alredy translated, this rte will be updated and +// index of this rte at m_dxl_to_plstmt_context->m_rtable_entries_list +// (shortened as "rte_list"), will be returned, if the rte wasn't +// translated, the newly created rte will be appended to rte_list and it's +// index returned). Also this function fills base_table_context for the +// mapping from colids to index attnos instead of table attnos. +// Returns index of translated range table entry at the rte_list. +// +//--------------------------------------------------------------------------- +Index +CTranslatorDXLToPlStmt::ProcessDXLTblDescr( + const CDXLTableDescr *table_descr, + CDXLTranslateContextBaseTable *base_table_context, AclMode acl_mode) { GPOS_ASSERT(NULL != table_descr); + BOOL rte_was_translated = false; + Index index = m_dxl_to_plstmt_context->GetRTEIndexByTableDescr( + table_descr, &rte_was_translated); + const IMDRelation *md_rel = m_md_accessor->RetrieveRel(table_descr->MDId()); const ULONG num_of_non_sys_cols = CTranslatorUtils::GetNumNonSystemColumns(md_rel); - RangeTblEntry *rte = MakeNode(RangeTblEntry); - rte->rtekind = RTE_RELATION; - // get oid for table Oid oid = CMDIdGPDB::CastMdid(table_descr->MDId())->Oid(); GPOS_ASSERT(InvalidOid != oid); - rte->relid = oid; - rte->checkAsUser = table_descr->GetExecuteAsUserId(); - rte->requiredPerms |= ACL_NO_RIGHTS; - // save oid and range index in translation context base_table_context->SetOID(oid); base_table_context->SetRelIndex(index); + // save mapping col id -> index in translate context + const ULONG arity = table_descr->Arity(); + for (ULONG ul = 0; ul < arity; ++ul) + { + const CDXLColDescr *dxl_col_descr = table_descr->GetColumnDescrAt(ul); + GPOS_ASSERT(NULL != dxl_col_descr); + + INT attno = dxl_col_descr->AttrNum(); + GPOS_ASSERT(0 != attno); + + (void) base_table_context->InsertMapping(dxl_col_descr->Id(), attno); + } + + // descriptor was already processed, and translated RTE is stored at + // context rtable list (only update required perms of this rte is needed) + if (rte_was_translated) + { + RangeTblEntry *rte = m_dxl_to_plstmt_context->GetRTEByIndex(index); + GPOS_ASSERT(NULL != rte); + rte->requiredPerms |= acl_mode; + return index; + } + + // create a new RTE (and it's alias) and store it at context rtable list + RangeTblEntry *rte = MakeNode(RangeTblEntry); + rte->rtekind = RTE_RELATION; + rte->relid = oid; + rte->checkAsUser = table_descr->GetExecuteAsUserId(); + rte->requiredPerms |= acl_mode; + Alias *alias = MakeNode(Alias); alias->colnames = NIL; @@ -4698,19 +4699,12 @@ CTranslatorDXLToPlStmt::TranslateDXLTblDescrToRangeTblEntry( table_descr->MdName()->GetMDName()->GetBuffer()); // get column names - const ULONG arity = table_descr->Arity(); - INT last_attno = 0; - for (ULONG ul = 0; ul < arity; ++ul) { const CDXLColDescr *dxl_col_descr = table_descr->GetColumnDescrAt(ul); - GPOS_ASSERT(NULL != dxl_col_descr); - INT attno = dxl_col_descr->AttrNum(); - GPOS_ASSERT(0 != attno); - if (0 < attno) { // if attno > last_attno + 1, there were dropped attributes @@ -4732,9 +4726,6 @@ CTranslatorDXLToPlStmt::TranslateDXLTblDescrToRangeTblEntry( alias->colnames = gpdb::LAppend(alias->colnames, val_colname); last_attno = attno; } - - // save mapping col id -> index in translate context - (void) base_table_context->InsertMapping(dxl_col_descr->Id(), attno); } // if there are any dropped columns at the end, add those too to the RangeTblEntry @@ -4746,7 +4737,9 @@ CTranslatorDXLToPlStmt::TranslateDXLTblDescrToRangeTblEntry( rte->eref = alias; - return rte; + m_dxl_to_plstmt_context->AddRTE(rte); + + return index; } //--------------------------------------------------------------------------- @@ -5691,18 +5684,10 @@ CTranslatorDXLToPlStmt::TranslateDXLBitmapTblScan( // translation context for column mappings in the base relation CDXLTranslateContextBaseTable base_table_context(m_mp); - // add the new range table entry as the last element of the range table - Index index = - gpdb::ListLength(m_dxl_to_plstmt_context->GetRTableEntriesList()) + 1; - const IMDRelation *md_rel = m_md_accessor->RetrieveRel(table_descr->MDId()); - RangeTblEntry *rte = TranslateDXLTblDescrToRangeTblEntry( - table_descr, index, &base_table_context); - GPOS_ASSERT(NULL != rte); - rte->requiredPerms |= ACL_SELECT; - - m_dxl_to_plstmt_context->AddRTE(rte); + Index index = + ProcessDXLTblDescr(table_descr, &base_table_context, ACL_SELECT); BitmapHeapScan *bitmap_tbl_scan; diff --git a/src/backend/gpopt/translate/CTranslatorQueryToDXL.cpp b/src/backend/gpopt/translate/CTranslatorQueryToDXL.cpp index 2292c777e9de..fbbd19bcf7cc 100644 --- a/src/backend/gpopt/translate/CTranslatorQueryToDXL.cpp +++ b/src/backend/gpopt/translate/CTranslatorQueryToDXL.cpp @@ -117,6 +117,8 @@ CTranslatorQueryToDXL::CTranslatorQueryToDXL( GPOS_ASSERT(NULL != query); CheckSupportedCmdType(query); + m_query_id = m_context->GetNextQueryId(); + CheckRangeTable(query); // GPDB_94_MERGE_FIXME: WITH CHECK OPTION views are not supported yet. @@ -710,9 +712,10 @@ CTranslatorQueryToDXL::TranslateInsertQueryToDXL() m_query->rtable, m_query->resultRelation - 1); CDXLTableDescr *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, m_query_id, &m_context->m_has_distributed_tables, &m_context->m_has_replicated_tables); + const IMDRelation *md_rel = m_md_accessor->RetrieveRel(table_descr->MDId()); if (!optimizer_enable_dml_triggers && CTranslatorUtils::RelHasTriggers(m_mp, m_md_accessor, md_rel, @@ -1186,7 +1189,7 @@ CTranslatorQueryToDXL::TranslateDeleteQueryToDXL() m_query->rtable, m_query->resultRelation - 1); CDXLTableDescr *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, m_query_id, &m_context->m_has_distributed_tables, &m_context->m_has_replicated_tables); const IMDRelation *md_rel = m_md_accessor->RetrieveRel(table_descr->MDId()); @@ -1260,7 +1263,7 @@ CTranslatorQueryToDXL::TranslateUpdateQueryToDXL() m_query->rtable, m_query->resultRelation - 1); CDXLTableDescr *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, m_query_id, &m_context->m_has_distributed_tables, &m_context->m_has_replicated_tables); const IMDRelation *md_rel = m_md_accessor->RetrieveRel(table_descr->MDId()); @@ -3247,10 +3250,20 @@ CTranslatorQueryToDXL::TranslateRTEToDXLLogicalGet(const RangeTblEntry *rte, GPOS_WSZ_LIT("ONLY in the FROM clause")); } + // query_id_for_target_rel is used to tag table descriptors assigned to target + // (result) relations one. In case of possible nested DML subqueries it's + // field points to target relation of corresponding Query structure of subquery. + ULONG query_id_for_target_rel = UNASSIGNED_QUERYID; + if (m_query->resultRelation > 0 && + ULONG(m_query->resultRelation) == rt_index) + { + query_id_for_target_rel = m_query_id; + } + // 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_context->m_has_distributed_tables, + query_id_for_target_rel, &m_context->m_has_distributed_tables, &m_context->m_has_replicated_tables); CDXLLogicalGet *dxl_op = NULL; diff --git a/src/backend/gpopt/translate/CTranslatorUtils.cpp b/src/backend/gpopt/translate/CTranslatorUtils.cpp index 53709414eb0e..f1721a0420b5 100644 --- a/src/backend/gpopt/translate/CTranslatorUtils.cpp +++ b/src/backend/gpopt/translate/CTranslatorUtils.cpp @@ -109,8 +109,9 @@ CDXLTableDescr * CTranslatorUtils::GetTableDescr(CMemoryPool *mp, CMDAccessor *md_accessor, CIdGenerator *id_generator, const RangeTblEntry *rte, - BOOL *is_distributed_table, // output - BOOL *is_replicated_table // output + ULONG assigned_query_id_for_target_rel, + BOOL *is_distributed_table, // output + BOOL *is_replicated_table // output ) { // generate an MDId for the table desc. @@ -134,7 +135,8 @@ CTranslatorUtils::GetTableDescr(CMemoryPool *mp, CMDAccessor *md_accessor, CMDName *table_mdname = GPOS_NEW(mp) CMDName(mp, tablename); CDXLTableDescr *table_descr = - GPOS_NEW(mp) CDXLTableDescr(mp, mdid, table_mdname, rte->checkAsUser); + GPOS_NEW(mp) CDXLTableDescr(mp, mdid, table_mdname, rte->checkAsUser, + assigned_query_id_for_target_rel); const ULONG len = rel->ColumnCount(); diff --git a/src/backend/gporca/data/dxl/minidump/Delete-Check-AssignedQueryIdForTargetRel.mdp b/src/backend/gporca/data/dxl/minidump/Delete-Check-AssignedQueryIdForTargetRel.mdp new file mode 100644 index 000000000000..1e79ec3a93f5 --- /dev/null +++ b/src/backend/gporca/data/dxl/minidump/Delete-Check-AssignedQueryIdForTargetRel.mdp @@ -0,0 +1,455 @@ + + + 10); + + Physical plan: + + QUERY PLAN +--------------------------------------------------------------------------------------------------------- + Delete (cost=0.00..862.02 rows=1 width=1) + -> Result (cost=0.00..862.00 rows=1 width=18) + -> Hash Semi Join (cost=0.00..862.00 rows=1 width=14) + Hash Cond: (test.i = test_1.i) + -> Seq Scan on test (cost=0.00..431.00 rows=1 width=14) + -> Hash (cost=431.00..431.00 rows=1 width=4) + -> Broadcast Motion 3:3 (slice1; segments: 3) (cost=0.00..431.00 rows=1 width=4) + -> Seq Scan on test test_1 (cost=0.00..431.00 rows=1 width=4) + Filter: (i > 10) + Optimizer: Pivotal Optimizer (GPORCA) +(10 rows) + ]]> + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/src/backend/gporca/libgpopt/include/gpopt/metadata/CTableDescriptor.h b/src/backend/gporca/libgpopt/include/gpopt/metadata/CTableDescriptor.h index d18c204aa51f..04f7b3532da4 100644 --- a/src/backend/gporca/libgpopt/include/gpopt/metadata/CTableDescriptor.h +++ b/src/backend/gporca/libgpopt/include/gpopt/metadata/CTableDescriptor.h @@ -97,13 +97,20 @@ class CTableDescriptor : public CRefCount, // returns true if this table descriptor has partial indexes BOOL FDescriptorWithPartialIndexes(); + // identifier of query to which current table belongs. + // This field is used for assigning current table entry with + // target one within DML operation. If descriptor doesn't point + // to the target (result) relation it has value UNASSIGNED_QUERYID + ULONG m_assigned_query_id_for_target_rel; + public: // ctor CTableDescriptor(CMemoryPool *, IMDId *mdid, const CName &, BOOL convert_hash_to_random, IMDRelation::Ereldistrpolicy rel_distr_policy, IMDRelation::Erelstoragetype erelstoragetype, - ULONG ulExecuteAsUser); + ULONG ulExecuteAsUser, + ULONG assigned_query_id_for_target_rel); // dtor virtual ~CTableDescriptor(); @@ -239,6 +246,12 @@ class CTableDescriptor : public CRefCount, m_erelstoragetype == IMDRelation::ErelstorageAppendOnlyRows; } + ULONG + GetAssignedQueryIdForTargetRel() const + { + return m_assigned_query_id_for_target_rel; + } + }; // class CTableDescriptor } // namespace gpopt diff --git a/src/backend/gporca/libgpopt/src/metadata/CTableDescriptor.cpp b/src/backend/gporca/libgpopt/src/metadata/CTableDescriptor.cpp index d3c5bcb0425a..a5b11f9104ae 100644 --- a/src/backend/gporca/libgpopt/src/metadata/CTableDescriptor.cpp +++ b/src/backend/gporca/libgpopt/src/metadata/CTableDescriptor.cpp @@ -37,7 +37,8 @@ FORCE_GENERATE_DBGSTR(CTableDescriptor); CTableDescriptor::CTableDescriptor( CMemoryPool *mp, IMDId *mdid, const CName &name, BOOL convert_hash_to_random, IMDRelation::Ereldistrpolicy rel_distr_policy, - IMDRelation::Erelstoragetype erelstoragetype, ULONG ulExecuteAsUser) + IMDRelation::Erelstoragetype erelstoragetype, ULONG ulExecuteAsUser, + ULONG assigned_query_id_for_target_rel) : m_mp(mp), m_mdid(mdid), m_name(mp, name), @@ -51,7 +52,8 @@ CTableDescriptor::CTableDescriptor( m_pdrgpbsKeys(NULL), m_num_of_partitions(0), m_execute_as_user_id(ulExecuteAsUser), - m_fHasPartialIndexes(FDescriptorWithPartialIndexes()) + m_fHasPartialIndexes(FDescriptorWithPartialIndexes()), + m_assigned_query_id_for_target_rel(assigned_query_id_for_target_rel) { GPOS_ASSERT(NULL != mp); GPOS_ASSERT(mdid->IsValid()); diff --git a/src/backend/gporca/libgpopt/src/translate/CTranslatorDXLToExpr.cpp b/src/backend/gporca/libgpopt/src/translate/CTranslatorDXLToExpr.cpp index ef2edaa9015c..d9348b7beace 100644 --- a/src/backend/gporca/libgpopt/src/translate/CTranslatorDXLToExpr.cpp +++ b/src/backend/gporca/libgpopt/src/translate/CTranslatorDXLToExpr.cpp @@ -2099,7 +2099,8 @@ CTranslatorDXLToExpr::Ptabdesc(CDXLTableDescr *table_descr) mdid->AddRef(); CTableDescriptor *ptabdesc = GPOS_NEW(m_mp) CTableDescriptor( m_mp, mdid, CName(m_mp, &strName), pmdrel->ConvertHashToRandom(), - rel_distr_policy, rel_storage_type, table_descr->GetExecuteAsUserId()); + rel_distr_policy, rel_storage_type, table_descr->GetExecuteAsUserId(), + table_descr->GetAssignedQueryIdForTargetRel()); const ULONG ulColumns = table_descr->Arity(); for (ULONG ul = 0; ul < ulColumns; ul++) @@ -2298,8 +2299,8 @@ CTranslatorDXLToExpr::PtabdescFromCTAS(CDXLLogicalCTAS *pdxlopCTAS) CTableDescriptor *ptabdesc = GPOS_NEW(m_mp) CTableDescriptor( m_mp, mdid, CName(m_mp, &strName), pmdrel->ConvertHashToRandom(), rel_distr_policy, rel_storage_type, - 0 // TODO: - Mar 5, 2014; ulExecuteAsUser - ); + 0, // TODO: - Mar 5, 2014; ulExecuteAsUser + UNASSIGNED_QUERYID); // populate column information from the dxl table descriptor CDXLColDescrArray *dxl_col_descr_array = diff --git a/src/backend/gporca/libgpopt/src/translate/CTranslatorExprToDXL.cpp b/src/backend/gporca/libgpopt/src/translate/CTranslatorExprToDXL.cpp index 3748a6f6f589..b5c2a3b858bc 100644 --- a/src/backend/gporca/libgpopt/src/translate/CTranslatorExprToDXL.cpp +++ b/src/backend/gporca/libgpopt/src/translate/CTranslatorExprToDXL.cpp @@ -1365,7 +1365,8 @@ CTranslatorExprToDXL::PdxlnMultiExternalScan( m_mp, extpart_mdid, extpart->Mdname().GetMDName(), extpart->ConvertHashToRandom(), extpart->GetRelDistribution(), extpart->RetrieveRelStorageType(), - multi_extscan->Ptabdesc()->GetExecuteAsUserId()); + multi_extscan->Ptabdesc()->GetExecuteAsUserId(), + multi_extscan->Ptabdesc()->GetAssignedQueryIdForTargetRel()); // Each scan shares the same col descriptors as the parent partitioned table // FIXME: Dropped columns break the assumption above. Handle it correctly. @@ -7569,7 +7570,8 @@ CTranslatorExprToDXL::MakeDXLTableDescr(const CTableDescriptor *ptabdesc, mdid->AddRef(); CDXLTableDescr *table_descr = GPOS_NEW(m_mp) - CDXLTableDescr(m_mp, mdid, pmdnameTbl, ptabdesc->GetExecuteAsUserId()); + CDXLTableDescr(m_mp, mdid, pmdnameTbl, ptabdesc->GetExecuteAsUserId(), + ptabdesc->GetAssignedQueryIdForTargetRel()); const ULONG ulColumns = ptabdesc->ColumnCount(); // translate col descriptors diff --git a/src/backend/gporca/libnaucrates/include/naucrates/dxl/operators/CDXLTableDescr.h b/src/backend/gporca/libnaucrates/include/naucrates/dxl/operators/CDXLTableDescr.h index 360b5f8841f7..f247aa87965c 100644 --- a/src/backend/gporca/libnaucrates/include/naucrates/dxl/operators/CDXLTableDescr.h +++ b/src/backend/gporca/libnaucrates/include/naucrates/dxl/operators/CDXLTableDescr.h @@ -20,6 +20,9 @@ #include "naucrates/md/CMDName.h" #include "naucrates/md/IMDId.h" +// default value for m_assigned_query_id_for_target_rel - no assigned query for table descriptor +#define UNASSIGNED_QUERYID 0 + namespace gpdxl { using namespace gpmd; @@ -53,12 +56,19 @@ class CDXLTableDescr : public CRefCount // private copy ctor CDXLTableDescr(const CDXLTableDescr &); + // identifier of query to which current table belongs. + // This field is used for assigning current table entry with + // target one within DML operation. If descriptor doesn't point + // to the target (result) relation it has value UNASSIGNED_QUERYID + ULONG m_assigned_query_id_for_target_rel; + void SerializeMDId(CXMLSerializer *xml_serializer) const; public: // ctor/dtor CDXLTableDescr(CMemoryPool *mp, IMDId *mdid, CMDName *mdname, - ULONG ulExecuteAsUser); + ULONG ulExecuteAsUser, + ULONG assigned_query_id_for_target_rel = UNASSIGNED_QUERYID); virtual ~CDXLTableDescr(); @@ -84,6 +94,9 @@ class CDXLTableDescr : public CRefCount // serialize to dxl format void SerializeToDXL(CXMLSerializer *xml_serializer) const; + + // get assigned query id for target relation + ULONG GetAssignedQueryIdForTargetRel() const; }; } // namespace gpdxl diff --git a/src/backend/gporca/libnaucrates/include/naucrates/dxl/xml/dxltokens.h b/src/backend/gporca/libnaucrates/include/naucrates/dxl/xml/dxltokens.h index 8e3435be4841..4b90c6266d64 100644 --- a/src/backend/gporca/libnaucrates/include/naucrates/dxl/xml/dxltokens.h +++ b/src/backend/gporca/libnaucrates/include/naucrates/dxl/xml/dxltokens.h @@ -462,6 +462,7 @@ enum Edxltoken EdxltokenIsNull, EdxltokenLintValue, EdxltokenDoubleValue, + EdxltokenAssignedQueryIdForTargetRel, EdxltokenRelTemporary, EdxltokenRelHasOids, diff --git a/src/backend/gporca/libnaucrates/src/operators/CDXLOperatorFactory.cpp b/src/backend/gporca/libnaucrates/src/operators/CDXLOperatorFactory.cpp index 4a989408426a..18c0dd743301 100644 --- a/src/backend/gporca/libnaucrates/src/operators/CDXLOperatorFactory.cpp +++ b/src/backend/gporca/libnaucrates/src/operators/CDXLOperatorFactory.cpp @@ -1479,7 +1479,12 @@ CDXLOperatorFactory::MakeDXLTableDescr(CDXLMemoryManager *dxl_memory_manager, EdxltokenTableDescr); } - return GPOS_NEW(mp) CDXLTableDescr(mp, mdid, mdname, user_id); + ULONG assigned_query_id_for_target_rel = ExtractConvertAttrValueToUlong( + dxl_memory_manager, attrs, EdxltokenAssignedQueryIdForTargetRel, + EdxltokenTableDescr, true /* is_optional */, UNASSIGNED_QUERYID); + + return GPOS_NEW(mp) CDXLTableDescr(mp, mdid, mdname, user_id, + assigned_query_id_for_target_rel); } //--------------------------------------------------------------------------- diff --git a/src/backend/gporca/libnaucrates/src/operators/CDXLTableDescr.cpp b/src/backend/gporca/libnaucrates/src/operators/CDXLTableDescr.cpp index 302b8ef1cc1d..91027851ec7c 100644 --- a/src/backend/gporca/libnaucrates/src/operators/CDXLTableDescr.cpp +++ b/src/backend/gporca/libnaucrates/src/operators/CDXLTableDescr.cpp @@ -30,12 +30,14 @@ using namespace gpdxl; // //--------------------------------------------------------------------------- CDXLTableDescr::CDXLTableDescr(CMemoryPool *mp, IMDId *mdid, CMDName *mdname, - ULONG ulExecuteAsUser) + ULONG ulExecuteAsUser, + ULONG assigned_query_id_for_target_rel) : m_mp(mp), m_mdid(mdid), m_mdname(mdname), m_dxl_column_descr_array(NULL), - m_execute_as_user_id(ulExecuteAsUser) + m_execute_as_user_id(ulExecuteAsUser), + m_assigned_query_id_for_target_rel(assigned_query_id_for_target_rel) { GPOS_ASSERT(NULL != m_mdname); m_dxl_column_descr_array = GPOS_NEW(mp) CDXLColDescrArray(mp); @@ -205,6 +207,13 @@ CDXLTableDescr::SerializeToDXL(CXMLSerializer *xml_serializer) const m_execute_as_user_id); } + if (UNASSIGNED_QUERYID != m_assigned_query_id_for_target_rel) + { + xml_serializer->AddAttribute( + CDXLTokens::GetDXLTokenStr(EdxltokenAssignedQueryIdForTargetRel), + m_assigned_query_id_for_target_rel); + } + // serialize columns xml_serializer->OpenElement( CDXLTokens::GetDXLTokenStr(EdxltokenNamespacePrefix), @@ -227,4 +236,21 @@ CDXLTableDescr::SerializeToDXL(CXMLSerializer *xml_serializer) const CDXLTokens::GetDXLTokenStr(EdxltokenTableDescr)); } + +//--------------------------------------------------------------------------- +// @function: +// CDXLTableDescr::GetAssignedQueryIdForTargetRel +// +// @doc: +// Return id of query, to which TableDescr belongs to +// (if this descriptor points to a result (target) entry, +// else UNASSIGNED_QUERYID returned) +// +//--------------------------------------------------------------------------- +ULONG +CDXLTableDescr::GetAssignedQueryIdForTargetRel() const +{ + return m_assigned_query_id_for_target_rel; +} + // EOF diff --git a/src/backend/gporca/libnaucrates/src/xml/dxltokens.cpp b/src/backend/gporca/libnaucrates/src/xml/dxltokens.cpp index b6737f18a344..3b57f8265ab5 100644 --- a/src/backend/gporca/libnaucrates/src/xml/dxltokens.cpp +++ b/src/backend/gporca/libnaucrates/src/xml/dxltokens.cpp @@ -510,6 +510,8 @@ CDXLTokens::Init(CMemoryPool *mp) {EdxltokenIsNull, GPOS_WSZ_LIT("IsNull")}, {EdxltokenLintValue, GPOS_WSZ_LIT("LintValue")}, {EdxltokenDoubleValue, GPOS_WSZ_LIT("DoubleValue")}, + {EdxltokenAssignedQueryIdForTargetRel, + GPOS_WSZ_LIT("AssignedQueryIdForTargetRel")}, {EdxltokenRelTemporary, GPOS_WSZ_LIT("IsTemporary")}, {EdxltokenRelHasOids, GPOS_WSZ_LIT("HasOids")}, diff --git a/src/backend/gporca/server/src/unittest/CTestUtils.cpp b/src/backend/gporca/server/src/unittest/CTestUtils.cpp index 2e298aa58772..1004f1b4f7b7 100644 --- a/src/backend/gporca/server/src/unittest/CTestUtils.cpp +++ b/src/backend/gporca/server/src/unittest/CTestUtils.cpp @@ -196,7 +196,8 @@ CTestUtils::PtabdescPlainWithColNameFormat( mp, mdid, nameTable, false, // convert_hash_to_random IMDRelation::EreldistrRandom, IMDRelation::ErelstorageHeap, - 0 // ulExecuteAsUser + 0, // ulExecuteAsUser + 0 // UNASSIGNED_QUERYID ); for (ULONG i = 0; i < num_cols; i++) diff --git a/src/backend/gporca/server/src/unittest/dxl/statistics/CStatisticsTest.cpp b/src/backend/gporca/server/src/unittest/dxl/statistics/CStatisticsTest.cpp index ff2c4f5f1c9e..d167b9dd91ea 100644 --- a/src/backend/gporca/server/src/unittest/dxl/statistics/CStatisticsTest.cpp +++ b/src/backend/gporca/server/src/unittest/dxl/statistics/CStatisticsTest.cpp @@ -316,7 +316,8 @@ CStatisticsTest::PtabdescTwoColumnSource(CMemoryPool *mp, mp, GPOS_NEW(mp) CMDIdGPDB(GPOPT_TEST_REL_OID1, 1, 1), nameTable, false, // convert_hash_to_random IMDRelation::EreldistrRandom, IMDRelation::ErelstorageHeap, - 0 // ulExecuteAsUser + 0, // ulExecuteAsUser + 0 // UNASSIGNED_QUERYID ); for (ULONG i = 0; i < 2; i++) diff --git a/src/backend/gporca/server/src/unittest/gpopt/minidump/CDMLTest.cpp b/src/backend/gporca/server/src/unittest/gpopt/minidump/CDMLTest.cpp index c125388cf79a..cfcbef929c66 100644 --- a/src/backend/gporca/server/src/unittest/gpopt/minidump/CDMLTest.cpp +++ b/src/backend/gporca/server/src/unittest/gpopt/minidump/CDMLTest.cpp @@ -47,6 +47,7 @@ const CHAR *rgszDMLFileNames[] = { "../data/dxl/minidump/InsertSortDistributed2MasterOnly.mdp", "../data/dxl/minidump/InsertProjectSort.mdp", "../data/dxl/minidump/InsertAssertSort.mdp", + "../data/dxl/minidump/Delete-Check-AssignedQueryIdForTargetRel.mdp", "../data/dxl/minidump/UpdateRandomDistr.mdp", "../data/dxl/minidump/DeleteRandomDistr.mdp", "../data/dxl/minidump/DeleteRandomlyDistributedTableJoin.mdp", diff --git a/src/backend/gporca/server/src/unittest/gpopt/translate/CTranslatorDXLToExprTest.cpp b/src/backend/gporca/server/src/unittest/gpopt/translate/CTranslatorDXLToExprTest.cpp index 060e66e3ba8f..7e2a2b95987a 100644 --- a/src/backend/gporca/server/src/unittest/gpopt/translate/CTranslatorDXLToExprTest.cpp +++ b/src/backend/gporca/server/src/unittest/gpopt/translate/CTranslatorDXLToExprTest.cpp @@ -251,7 +251,8 @@ class GetBuilder m_ptabdesc = GPOS_NEW(mp) CTableDescriptor( mp, mdid, CName(&strTableName), convert_hash_to_random, CMDRelationGPDB::EreldistrMasterOnly, - CMDRelationGPDB::ErelstorageHeap, ulExecuteAsUser); + CMDRelationGPDB::ErelstorageHeap, ulExecuteAsUser, + 0 /* UNASSIGNED_QUERYID */); } void diff --git a/src/include/gpopt/translate/CContextDXLToPlStmt.h b/src/include/gpopt/translate/CContextDXLToPlStmt.h index 148798205613..28d2f826f0ed 100644 --- a/src/include/gpopt/translate/CContextDXLToPlStmt.h +++ b/src/include/gpopt/translate/CContextDXLToPlStmt.h @@ -90,6 +90,10 @@ class CContextDXLToPlStmt CleanupDelete > HMUlCTEConsumerInfo; + typedef CHashMap, gpos::Equals, + CleanupDelete, CleanupDelete > + HMUlIndex; + CMemoryPool *m_mp; // counter for generating plan ids @@ -128,6 +132,9 @@ class CContextDXLToPlStmt // CTAS distribution policy GpPolicy *m_distribution_policy; + // hash map of the queryid (of DML query) and the target relation index + HMUlIndex *m_used_rte_indexes; + public: // ctor/dtor CContextDXLToPlStmt(CMemoryPool *mp, CIdGenerator *plan_id_counter, @@ -215,6 +222,12 @@ class CContextDXLToPlStmt // based on decision made by DetermineDistributionHashOpclasses() Oid GetDistributionHashOpclassForType(Oid typid); Oid GetDistributionHashFuncForType(Oid typid); + + // get rte from m_rtable_entries_list by given index + RangeTblEntry *GetRTEByIndex(Index index); + + Index GetRTEIndexByTableDescr(const CDXLTableDescr *table_descr, + BOOL *is_rte_exists); }; } // namespace gpdxl diff --git a/src/include/gpopt/translate/CContextQueryToDXL.h b/src/include/gpopt/translate/CContextQueryToDXL.h index 03add95c2f9e..368d81f0ba02 100644 --- a/src/include/gpopt/translate/CContextQueryToDXL.h +++ b/src/include/gpopt/translate/CContextQueryToDXL.h @@ -22,6 +22,7 @@ #define GPDXL_CTE_ID_START 1 #define GPDXL_COL_ID_START 1 +#define GPDXL_QUERY_ID_START 1 namespace gpdxl { @@ -53,6 +54,9 @@ class CContextQueryToDXL // counter for generating unique CTE ids CIdGenerator *m_cte_id_counter; + // counter for upper-level query and its subqueries + CIdGenerator *m_queryid_counter; + // does the query have any distributed tables? BOOL m_has_distributed_tables; @@ -71,6 +75,8 @@ class CContextQueryToDXL // dtor ~CContextQueryToDXL(); + + ULONG GetNextQueryId(); }; } // namespace gpdxl #endif // GPDXL_CContextQueryToDXL_H diff --git a/src/include/gpopt/translate/CTranslatorDXLToPlStmt.h b/src/include/gpopt/translate/CTranslatorDXLToPlStmt.h index 3a8cf515c140..3e3ce23f458f 100644 --- a/src/include/gpopt/translate/CTranslatorDXLToPlStmt.h +++ b/src/include/gpopt/translate/CTranslatorDXLToPlStmt.h @@ -464,9 +464,9 @@ class CTranslatorDXLToPlStmt CDXLTranslateContextBaseTable *base_table_context); // create range table entry from a table descriptor - RangeTblEntry *TranslateDXLTblDescrToRangeTblEntry( - const CDXLTableDescr *table_descr, Index index, - CDXLTranslateContextBaseTable *base_table_context); + Index ProcessDXLTblDescr(const CDXLTableDescr *table_descr, + CDXLTranslateContextBaseTable *base_table_context, + AclMode acl_mode); // translate DXL projection list into a target list List *TranslateDXLProjList( diff --git a/src/include/gpopt/translate/CTranslatorQueryToDXL.h b/src/include/gpopt/translate/CTranslatorQueryToDXL.h index 9d5176a70e7c..8c5ba1d6f2a7 100644 --- a/src/include/gpopt/translate/CTranslatorQueryToDXL.h +++ b/src/include/gpopt/translate/CTranslatorQueryToDXL.h @@ -138,6 +138,10 @@ class CTranslatorQueryToDXL // CTE producer IDs defined at the current query level UlongBoolHashMap *m_cteid_at_current_query_level_map; + // id of current query (and for nested queries), it's used for correct assigning + // of relation links to target relation of DML query + ULONG m_query_id; + //ctor // private constructor, called from the public factory function QueryToDXLInstance CTranslatorQueryToDXL( diff --git a/src/include/gpopt/translate/CTranslatorUtils.h b/src/include/gpopt/translate/CTranslatorUtils.h index 9ba1d35b88ad..3fa5d2313d77 100644 --- a/src/include/gpopt/translate/CTranslatorUtils.h +++ b/src/include/gpopt/translate/CTranslatorUtils.h @@ -142,12 +142,10 @@ class CTranslatorUtils CMDAccessor *md_accessor, IMDId *mdid); // translate a RangeTableEntry into a CDXLTableDescr - static CDXLTableDescr *GetTableDescr(CMemoryPool *mp, - CMDAccessor *md_accessor, - CIdGenerator *id_generator, - const RangeTblEntry *rte, - BOOL *is_distributed_table = NULL, - BOOL *is_replicated_table = NULL); + static CDXLTableDescr *GetTableDescr( + CMemoryPool *mp, CMDAccessor *md_accessor, CIdGenerator *id_generator, + const RangeTblEntry *rte, ULONG assigned_query_id_for_target_rel, + BOOL *is_distributed_table = NULL, BOOL *is_replicated_table = NULL); // translate a RangeTableEntry into a CDXLLogicalTVF static CDXLLogicalTVF *ConvertToCDXLLogicalTVF(CMemoryPool *mp, diff --git a/src/test/isolation2/expected/modify_table_data_corrupt_optimizer.out b/src/test/isolation2/expected/modify_table_data_corrupt_optimizer.out index 25a23ba33967..370973709f75 100644 --- a/src/test/isolation2/expected/modify_table_data_corrupt_optimizer.out +++ b/src/test/isolation2/expected/modify_table_data_corrupt_optimizer.out @@ -200,7 +200,7 @@ explain (costs off) update tab1 set b = b + 1; Update -> Result -> Redistribute Motion 3:3 (slice1; segments: 3) - Hash Key: tab1.b + Hash Key: b -> Split -> Result -> Seq Scan on tab1 diff --git a/src/test/isolation2/output/uao/parallel_delete_optimizer.source b/src/test/isolation2/output/uao/parallel_delete_optimizer.source index 7646a9066bfb..832979779db1 100644 --- a/src/test/isolation2/output/uao/parallel_delete_optimizer.source +++ b/src/test/isolation2/output/uao/parallel_delete_optimizer.source @@ -19,15 +19,13 @@ DELETE 1 coalesce | mode | locktype | node ----------+---------------------+--------------------------+-------- ao | AccessExclusiveLock | append-only segment file | master - ao | AccessShareLock | relation | master ao | ExclusiveLock | relation | master -(3 rows) +(2 rows) 2: SELECT * FROM locktest_segments WHERE coalesce = 'ao'; coalesce | mode | locktype | node ----------+------------------+----------+------------ - ao | AccessShareLock | relation | n segments ao | RowExclusiveLock | relation | n segments -(2 rows) +(1 row) -- The case here should delete a tuple at the same seg with(2). -- Under jump hash, (2) and (3) are on the same seg(seg0). 1&: DELETE FROM ao WHERE a = 3; diff --git a/src/test/isolation2/output/uao/parallel_update_optimizer.source b/src/test/isolation2/output/uao/parallel_update_optimizer.source index 6423a5f8e055..48a155a35285 100644 --- a/src/test/isolation2/output/uao/parallel_update_optimizer.source +++ b/src/test/isolation2/output/uao/parallel_update_optimizer.source @@ -19,16 +19,14 @@ UPDATE 1 coalesce | mode | locktype | node ----------+---------------------+--------------------------+-------- ao | AccessExclusiveLock | append-only segment file | master - ao | AccessShareLock | relation | master ao | ExclusiveLock | relation | master -(3 rows) +(2 rows) 2: SELECT * FROM locktest_segments WHERE coalesce = 'ao'; coalesce | mode | locktype | node ----------+---------------------+--------------------------+------------ ao | AccessExclusiveLock | append-only segment file | 1 segment - ao | AccessShareLock | relation | n segments ao | RowExclusiveLock | relation | n segments -(3 rows) +(2 rows) -- The case here should update a tuple at the same seg with(2). -- Under jump hash, (2) and (3) are on the same seg(seg0). 1&: UPDATE ao SET b = 42 WHERE a = 3; diff --git a/src/test/regress/expected/ao_locks_optimizer.out b/src/test/regress/expected/ao_locks_optimizer.out index 6401372ac96a..11a0cf585ef4 100644 --- a/src/test/regress/expected/ao_locks_optimizer.out +++ b/src/test/regress/expected/ao_locks_optimizer.out @@ -78,9 +78,8 @@ SELECT * FROM locktest_master where coalesce = 'ao_locks_table' or coalesce | mode | locktype | node ----------------+---------------------+--------------------------+-------- ao_locks_table | AccessExclusiveLock | append-only segment file | master - ao_locks_table | AccessShareLock | relation | master ao_locks_table | ExclusiveLock | relation | master -(3 rows) +(2 rows) SELECT * FROM locktest_segments where coalesce = 'ao_locks_table' or coalesce like 'aovisimap%' or coalesce like 'aoseg%'; @@ -88,9 +87,8 @@ SELECT * FROM locktest_segments where coalesce = 'ao_locks_table' or -----------------+------------------+----------+------------ aovisimap index | RowExclusiveLock | relation | 1 segment ao_locks_table | RowExclusiveLock | relation | n segments - ao_locks_table | AccessShareLock | relation | n segments aovisimap table | RowExclusiveLock | relation | 1 segment -(4 rows) +(3 rows) COMMIT; BEGIN; @@ -100,19 +98,17 @@ SELECT * FROM locktest_master where coalesce = 'ao_locks_table' or coalesce | mode | locktype | node ----------------+---------------------+--------------------------+-------- ao_locks_table | AccessExclusiveLock | append-only segment file | master - ao_locks_table | AccessShareLock | relation | master ao_locks_table | ExclusiveLock | relation | master -(3 rows) +(2 rows) SELECT * FROM locktest_segments where coalesce = 'ao_locks_table' or coalesce like 'aovisimap%' or coalesce like 'aoseg%'; coalesce | mode | locktype | node -----------------+---------------------+--------------------------+------------ - ao_locks_table | AccessShareLock | relation | n segments aovisimap table | RowExclusiveLock | relation | 1 segment ao_locks_table | AccessExclusiveLock | append-only segment file | 1 segment aovisimap index | RowExclusiveLock | relation | 1 segment ao_locks_table | RowExclusiveLock | relation | n segments -(5 rows) +(4 rows) COMMIT; diff --git a/src/test/regress/expected/bfv_dml_optimizer.out b/src/test/regress/expected/bfv_dml_optimizer.out index 164d849c4eea..2598c3f63843 100644 --- a/src/test/regress/expected/bfv_dml_optimizer.out +++ b/src/test/regress/expected/bfv_dml_optimizer.out @@ -671,11 +671,11 @@ explain delete from foo using (select a from foo union all select b from bar) v; -> Result (cost=0.00..1765380.23 rows=67 width=18) -> Nested Loop (cost=0.00..1765380.23 rows=67 width=14) Join Filter: true - -> Seq Scan on foo foo_1 (cost=0.00..431.00 rows=4 width=14) + -> Seq Scan on foo (cost=0.00..431.00 rows=4 width=14) -> Materialize (cost=0.00..862.00 rows=20 width=1) -> Broadcast Motion 3:3 (slice1; segments: 3) (cost=0.00..862.00 rows=20 width=1) -> Append (cost=0.00..862.00 rows=7 width=1) - -> Seq Scan on foo (cost=0.00..431.00 rows=4 width=1) + -> Seq Scan on foo foo_1 (cost=0.00..431.00 rows=4 width=1) -> Seq Scan on bar (cost=0.00..431.00 rows=4 width=1) Optimizer: Pivotal Optimizer (GPORCA) (11 rows) diff --git a/src/test/regress/expected/partition_locking.out b/src/test/regress/expected/partition_locking.out index 5cc7b3511f48..0101c71e594e 100644 --- a/src/test/regress/expected/partition_locking.out +++ b/src/test/regress/expected/partition_locking.out @@ -498,6 +498,8 @@ select * from locktest_master where coalesce not like 'gp_%' and coalesce not li partlockt_1_prt_9_i_idx | RowExclusiveLock | relation | master (19 rows) +-- TODO: Seems RowExclusiveLock should be also applied on index of partlock table, +-- like it's done in vanila postgres (lock on indexes should be eqaul to locks on table). select * from locktest_segments where coalesce not like 'gp_%' and coalesce not like 'pg_%'; coalesce | mode | locktype | node -------------------+------------------+----------+----------- diff --git a/src/test/regress/expected/partition_locking_optimizer.out b/src/test/regress/expected/partition_locking_optimizer.out index 944246d9ff30..5d05dba64e95 100644 --- a/src/test/regress/expected/partition_locking_optimizer.out +++ b/src/test/regress/expected/partition_locking_optimizer.out @@ -471,13 +471,14 @@ select * from locktest_master where coalesce not like 'gp_%' and coalesce not li partlockt_1_prt_9 | ExclusiveLock | relation | master (10 rows) +-- TODO: Seems RowExclusiveLock should be also applied on index of partlock table, +-- like it's done in vanila postgres (lock on indexes should be eqaul to locks on table). select * from locktest_segments where coalesce not like 'gp_%' and coalesce not like 'pg_%'; - coalesce | mode | locktype | node --------------------------+------------------+----------+------------ - partlockt | RowExclusiveLock | relation | n segments - partlockt_1_prt_4 | AccessShareLock | relation | n segments - partlockt_1_prt_4_i_idx | AccessShareLock | relation | n segments -(3 rows) + coalesce | mode | locktype | node +-------------------+------------------+----------+------------ + partlockt | RowExclusiveLock | relation | n segments + partlockt_1_prt_4 | AccessShareLock | relation | n segments +(2 rows) commit; -- drop index diff --git a/src/test/regress/expected/updatable_views_optimizer.out b/src/test/regress/expected/updatable_views_optimizer.out index b814e9ad11f0..dd78b17f92d9 100644 --- a/src/test/regress/expected/updatable_views_optimizer.out +++ b/src/test/regress/expected/updatable_views_optimizer.out @@ -369,7 +369,7 @@ EXPLAIN (costs off) UPDATE rw_view1 SET a=6 WHERE a=5; -> Sort Sort Key: (DMLAction) -> Redistribute Motion 3:3 (slice1; segments: 3) - Hash Key: base_tbl.a + Hash Key: a -> Split -> Result -> Index Scan using base_tbl_pkey on base_tbl @@ -449,7 +449,7 @@ EXPLAIN (costs off) UPDATE rw_view2 SET aaa=5 WHERE aaa=4; -> Sort Sort Key: (DMLAction) -> Redistribute Motion 3:3 (slice1; segments: 3) - Hash Key: base_tbl.a + Hash Key: a -> Split -> Result -> Index Scan using base_tbl_pkey on base_tbl @@ -2079,7 +2079,7 @@ EXPLAIN (costs off) INSERT INTO rw_view1 VALUES (2, 'New row 2'); -> Result -> Nested Loop Semi Join Join Filter: true - -> Index Scan using base_tbl_pkey on base_tbl base_tbl_1 + -> Index Scan using base_tbl_pkey on base_tbl Index Cond: (id = 2) -> Materialize -> Broadcast Motion 1:3 (slice2; segments: 1) @@ -2087,7 +2087,7 @@ EXPLAIN (costs off) INSERT INTO rw_view1 VALUES (2, 'New row 2'); -> Result -> Limit -> Gather Motion 3:1 (slice1; segments: 3) - -> Index Scan using base_tbl_pkey on base_tbl + -> Index Scan using base_tbl_pkey on base_tbl base_tbl_1 Index Cond: (id = 2) Optimizer: Pivotal Optimizer (GPORCA) version 3.11.0 (38 rows) diff --git a/src/test/regress/expected/update_optimizer.out b/src/test/regress/expected/update_optimizer.out index cb9663bf26fb..7b72db84d3a1 100755 --- a/src/test/regress/expected/update_optimizer.out +++ b/src/test/regress/expected/update_optimizer.out @@ -341,7 +341,7 @@ EXPLAIN (COSTS OFF ) UPDATE tab3 SET C1 = C1 + 1, C5 = C5+1; Update -> Result -> Redistribute Motion 3:3 (slice1; segments: 3) - Hash Key: tab3.c1, tab3.c2, tab3.c3 + Hash Key: c1, c2, c3 -> Split -> Result -> Seq Scan on tab3 diff --git a/src/test/regress/sql/partition_locking.sql b/src/test/regress/sql/partition_locking.sql index d7df5a690a62..2e0cc78d432e 100644 --- a/src/test/regress/sql/partition_locking.sql +++ b/src/test/regress/sql/partition_locking.sql @@ -148,6 +148,8 @@ begin; delete from partlockt where i = 4; -- Known_opt_diff: MPP-20936 select * from locktest_master where coalesce not like 'gp_%' and coalesce not like 'pg_%'; +-- TODO: Seems RowExclusiveLock should be also applied on index of partlock table, +-- like it's done in vanila postgres (lock on indexes should be eqaul to locks on table). select * from locktest_segments where coalesce not like 'gp_%' and coalesce not like 'pg_%'; commit; From e838191af20ea3180945e690d4bab2284c34d70b Mon Sep 17 00:00:00 2001 From: HustonMmmavr Date: Thu, 1 Sep 2022 12:40:18 +0300 Subject: [PATCH 2/2] orca: use epqstate for DMLState like as ModifyTableState Propagate epqState, to use eqp mechanism, like at it's done at pg planner. Also add epqParam to `DML` node, like it's done at `ModifyTable`, and fill it like it's done at pg planner: `make_modifytable(..., SS_assign_special_param(root))` where SS_assign_special_param post-increments root->glob->nParamExec --- src/backend/executor/nodeDML.c | 7 +++- src/backend/executor/nodeModifyTable.c | 11 ------- .../translate/CTranslatorDXLToPlStmt.cpp | 1 + src/backend/nodes/copyfuncs.c | 1 + src/backend/nodes/outfuncs.c | 1 + src/backend/nodes/readfast.c | 1 + src/include/nodes/execnodes.h | 1 + src/include/nodes/plannodes.h | 2 +- .../expected/gdd/concurrent_update.out | 29 +++++++++++++---- .../gdd/concurrent_update_optimizer.out | 32 ++++++++++++++----- .../isolation2/sql/gdd/concurrent_update.sql | 22 +++++++++---- 11 files changed, 75 insertions(+), 33 deletions(-) diff --git a/src/backend/executor/nodeDML.c b/src/backend/executor/nodeDML.c index 33b2edf5387f..d7ba9cea7d15 100644 --- a/src/backend/executor/nodeDML.c +++ b/src/backend/executor/nodeDML.c @@ -57,6 +57,7 @@ ExecDML(DMLState *node) { return NULL; } + EvalPlanQualSetSlot(&node->dml_epqstate, slot); bool isnull = false; int action = DatumGetUInt32(slot_getattr(slot, plannode->actionColIdx, &isnull)); @@ -145,7 +146,7 @@ ExecDML(DMLState *node) segid, NULL, /* GPDB_91_MERGE_FIXME: oldTuple? */ node->cleanedUpSlot, - NULL /* DestReceiver */, + &node->dml_epqstate, node->ps.state, !isUpdate, /* GPDB_91_MERGE_FIXME: where to get canSetTag? */ PLANGEN_OPTIMIZER /* Plan origin */, @@ -177,6 +178,8 @@ ExecInitDML(DML *node, EState *estate, int eflags) CmdType operation = estate->es_plannedstmt->commandType; ResultRelInfo *resultRelInfo = estate->es_result_relation_info; + EvalPlanQualInit(&dmlstate->dml_epqstate, NULL, NULL, NIL, node->epqParam); + ExecInitResultTupleSlot(estate, &dmlstate->ps); dmlstate->ps.targetlist = (List *) @@ -184,6 +187,7 @@ ExecInitDML(DML *node, EState *estate, int eflags) (PlanState *) dmlstate); Plan *outerPlan = outerPlan(node); + EvalPlanQualSetPlan(&dmlstate->dml_epqstate, outerPlan, NULL); outerPlanState(dmlstate) = ExecInitNode(outerPlan, estate, eflags); /* @@ -279,6 +283,7 @@ ExecEndDML(DMLState *node) ExecFreeExprContext(&node->ps); ExecClearTuple(node->ps.ps_ResultTupleSlot); ExecClearTuple(node->cleanedUpSlot); + EvalPlanQualEnd(&node->dml_epqstate); ExecEndNode(outerPlanState(node)); EndPlanStateGpmonPkt(&node->ps); } diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c index 24ad14a3afde..e7eb74284405 100644 --- a/src/backend/executor/nodeModifyTable.c +++ b/src/backend/executor/nodeModifyTable.c @@ -961,17 +961,6 @@ ldelete:; { TupleTableSlot *epqslot; - /* - * TODO: DML node doesn't initialize `epqstate` parameter so - * we exclude EPQ routine for this type of modification and - * act as in RR and upper isolation levels. - */ - if (!epqstate) - ereport(ERROR, - (errcode(ERRCODE_T_R_SERIALIZATION_FAILURE), - errmsg("could not serialize access due to concurrent update"), - errhint("Use PostgreSQL Planner instead of Optimizer for this query via optimizer=off GUC setting"))); - epqslot = EvalPlanQual(estate, epqstate, resultRelationDesc, diff --git a/src/backend/gpopt/translate/CTranslatorDXLToPlStmt.cpp b/src/backend/gpopt/translate/CTranslatorDXLToPlStmt.cpp index 66e77520e92f..f3f8878aef2a 100644 --- a/src/backend/gpopt/translate/CTranslatorDXLToPlStmt.cpp +++ b/src/backend/gpopt/translate/CTranslatorDXLToPlStmt.cpp @@ -4201,6 +4201,7 @@ CTranslatorDXLToPlStmt::TranslateDXLDml( plan->lefttree = child_plan; plan->nMotionNodes = child_plan->nMotionNodes; plan->plan_node_id = m_dxl_to_plstmt_context->GetNextPlanId(); + dml->epqParam = m_dxl_to_plstmt_context->GetNextParamId(); if (CMD_INSERT == m_cmd_type && 0 == plan->nMotionNodes) { diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c index f865a436be80..dfdcd1dc6f7b 100644 --- a/src/backend/nodes/copyfuncs.c +++ b/src/backend/nodes/copyfuncs.c @@ -1337,6 +1337,7 @@ _copyDML(const DML *from) COPY_SCALAR_FIELD(actionColIdx); COPY_SCALAR_FIELD(ctidColIdx); COPY_SCALAR_FIELD(tupleoidColIdx); + COPY_SCALAR_FIELD(epqParam); return newnode; } diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c index 0dfe3360ab66..3ab88a337cab 100644 --- a/src/backend/nodes/outfuncs.c +++ b/src/backend/nodes/outfuncs.c @@ -1215,6 +1215,7 @@ _outDML(StringInfo str, const DML *node) WRITE_INT_FIELD(actionColIdx); WRITE_INT_FIELD(ctidColIdx); WRITE_INT_FIELD(tupleoidColIdx); + WRITE_INT_FIELD(epqParam); _outPlanInfo(str, (Plan *) node); } diff --git a/src/backend/nodes/readfast.c b/src/backend/nodes/readfast.c index de6c0c2c011a..0508552b6d95 100644 --- a/src/backend/nodes/readfast.c +++ b/src/backend/nodes/readfast.c @@ -2237,6 +2237,7 @@ _readDML(void) READ_INT_FIELD(actionColIdx); READ_INT_FIELD(ctidColIdx); READ_INT_FIELD(tupleoidColIdx); + READ_INT_FIELD(epqParam); readPlanInfo((Plan *)local_node); diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h index be1ee1b7f23a..66b868c3db1d 100644 --- a/src/include/nodes/execnodes.h +++ b/src/include/nodes/execnodes.h @@ -2829,6 +2829,7 @@ typedef struct DMLState JunkFilter *junkfilter; /* filter that removes junk and dropped attributes */ TupleTableSlot *cleanedUpSlot; /* holds 'final' tuple which matches the target relation schema */ AttrNumber segid_attno; /* attribute number of "gp_segment_id" */ + EPQState dml_epqstate; } DMLState; /* diff --git a/src/include/nodes/plannodes.h b/src/include/nodes/plannodes.h index 19e38a9e2166..6b11dc7b6987 100644 --- a/src/include/nodes/plannodes.h +++ b/src/include/nodes/plannodes.h @@ -1328,7 +1328,7 @@ typedef struct DML AttrNumber actionColIdx; /* index of action column into the target list */ AttrNumber ctidColIdx; /* index of ctid column into the target list */ AttrNumber tupleoidColIdx; /* index of tuple oid column into the target list */ - + int epqParam; /* ID of Param for EvalPlanQual re-eval */ } DML; /* diff --git a/src/test/isolation2/expected/gdd/concurrent_update.out b/src/test/isolation2/expected/gdd/concurrent_update.out index eb54eb7ac44c..c10b20617c03 100644 --- a/src/test/isolation2/expected/gdd/concurrent_update.out +++ b/src/test/isolation2/expected/gdd/concurrent_update.out @@ -251,10 +251,7 @@ DROP 1q: ... 2q: ... --- Currently the DML node (modification variant in ORCA) doesn't support EPQ --- routine so concurrent modification is not possible in this case. User have --- to be informed to fallback to postgres optimizer. --- This test makes sense just for ORCA +-- check that orca concurrent delete transaction won't delete tuple, updated in other transaction (which doesn't match predicate anymore) create table test as select 0 as i distributed randomly; CREATE 1 -- in session 1, turn off the optimizer so it will invoke heap_update @@ -267,8 +264,6 @@ UPDATE 1 -- in session 2, in case of ORCA DML invokes EPQ -- the following SQL will hang due to XID lock 2&: delete from test where i = 0; --- commit session1's transaction so the above session2 will continue and emit --- error about serialization failure in case of ORCA 1: end; END 2<: <... completed> @@ -278,6 +273,28 @@ DROP 1q: ... 2q: ... +-- check that orca concurrent delete transaction will delete tuple, updated in other transaction (which still matches predicate) +create table test as select 0 as i distributed randomly; +CREATE 1 +-- in session 1, turn off the optimizer so it will invoke heap_update +1: set optimizer = off; +SET +1: begin; +BEGIN +1: update test set i = i; +UPDATE 1 +-- in session 2, in case of ORCA DML invokes EPQ +-- the following SQL will hang due to XID lock +2&: delete from test where i = 0; +1: end; +END +2<: <... completed> +DELETE 1 +drop table test; +DROP +1q: ... +2q: ... + -- split update is to implement updating on hash keys, -- it deletes the tuple and insert a new tuple in a -- new segment, so it is not easy for other transaction diff --git a/src/test/isolation2/expected/gdd/concurrent_update_optimizer.out b/src/test/isolation2/expected/gdd/concurrent_update_optimizer.out index 5f3ed55c942e..89db75aedf17 100644 --- a/src/test/isolation2/expected/gdd/concurrent_update_optimizer.out +++ b/src/test/isolation2/expected/gdd/concurrent_update_optimizer.out @@ -251,10 +251,7 @@ DROP 1q: ... 2q: ... --- Currently the DML node (modification variant in ORCA) doesn't support EPQ --- routine so concurrent modification is not possible in this case. User have --- to be informed to fallback to postgres optimizer. --- This test makes sense just for ORCA +-- check that orca concurrent delete transaction won't delete tuple, updated in other transaction (which doesn't match predicate anymore) create table test as select 0 as i distributed randomly; CREATE 1 -- in session 1, turn off the optimizer so it will invoke heap_update @@ -267,13 +264,32 @@ UPDATE 1 -- in session 2, in case of ORCA DML invokes EPQ -- the following SQL will hang due to XID lock 2&: delete from test where i = 0; --- commit session1's transaction so the above session2 will continue and emit --- error about serialization failure in case of ORCA 1: end; END 2<: <... completed> -ERROR: could not serialize access due to concurrent update -HINT: Use PostgreSQL Planner instead of Optimizer for this query via optimizer=off GUC setting +DELETE 0 +drop table test; +DROP +1q: ... +2q: ... + +-- check that orca concurrent delete transaction will delete tuple, updated in other transaction (which still matches predicate) +create table test as select 0 as i distributed randomly; +CREATE 1 +-- in session 1, turn off the optimizer so it will invoke heap_update +1: set optimizer = off; +SET +1: begin; +BEGIN +1: update test set i = i; +UPDATE 1 +-- in session 2, in case of ORCA DML invokes EPQ +-- the following SQL will hang due to XID lock +2&: delete from test where i = 0; +1: end; +END +2<: <... completed> +DELETE 1 drop table test; DROP 1q: ... diff --git a/src/test/isolation2/sql/gdd/concurrent_update.sql b/src/test/isolation2/sql/gdd/concurrent_update.sql index f670df45456c..14d640b6c1ed 100644 --- a/src/test/isolation2/sql/gdd/concurrent_update.sql +++ b/src/test/isolation2/sql/gdd/concurrent_update.sql @@ -135,10 +135,7 @@ DROP TABLE t_concurrent_update; 1q: 2q: --- Currently the DML node (modification variant in ORCA) doesn't support EPQ --- routine so concurrent modification is not possible in this case. User have --- to be informed to fallback to postgres optimizer. --- This test makes sense just for ORCA +-- check that orca concurrent delete transaction won't delete tuple, updated in other transaction (which doesn't match predicate anymore) create table test as select 0 as i distributed randomly; -- in session 1, turn off the optimizer so it will invoke heap_update 1: set optimizer = off; @@ -147,8 +144,21 @@ create table test as select 0 as i distributed randomly; -- in session 2, in case of ORCA DML invokes EPQ -- the following SQL will hang due to XID lock 2&: delete from test where i = 0; --- commit session1's transaction so the above session2 will continue and emit --- error about serialization failure in case of ORCA +1: end; +2<: +drop table test; +1q: +2q: + +-- check that orca concurrent delete transaction will delete tuple, updated in other transaction (which still matches predicate) +create table test as select 0 as i distributed randomly; +-- in session 1, turn off the optimizer so it will invoke heap_update +1: set optimizer = off; +1: begin; +1: update test set i = i; +-- in session 2, in case of ORCA DML invokes EPQ +-- the following SQL will hang due to XID lock +2&: delete from test where i = 0; 1: end; 2<: drop table test;