From 3b3540acc68e6e8a81e79b980e7e820bc5279b3e Mon Sep 17 00:00:00 2001 From: Vinoth Veeraraghavan Date: Thu, 3 Sep 2020 17:01:26 +0800 Subject: [PATCH] Removed unnecessary and wrong reference to current row in MOT FDW state --- .../storage/mot/fdw_adapter/src/mot_fdw.cpp | 130 ++++++++---------- .../mot/fdw_adapter/src/mot_internal.cpp | 6 +- .../mot/fdw_adapter/src/mot_internal.h | 3 +- 3 files changed, 58 insertions(+), 81 deletions(-) diff --git a/src/gausskernel/storage/mot/fdw_adapter/src/mot_fdw.cpp b/src/gausskernel/storage/mot/fdw_adapter/src/mot_fdw.cpp index a1a8bc75a..f2c7a23a5 100644 --- a/src/gausskernel/storage/mot/fdw_adapter/src/mot_fdw.cpp +++ b/src/gausskernel/storage/mot/fdw_adapter/src/mot_fdw.cpp @@ -1002,13 +1002,13 @@ static TupleTableSlot* MOTIterateForeignScan(ForeignScanState* node) return nullptr; } + MOT::Row* currRow; MOTFdwStateSt* festate = (MOTFdwStateSt*)node->fdw_state; TupleTableSlot* slot = node->ss.ss_ScanTupleSlot; bool found = false; bool stopAtFirst = (festate->m_bestIx && festate->m_bestIx->m_ixOpers[0] == KEY_OPER::READ_KEY_EXACT && festate->m_bestIx->m_ix->GetUnique() == true); - festate->m_currRow = NULL; (void)ExecClearTuple(slot); if (stopAtFirst) { @@ -1018,18 +1018,18 @@ static TupleTableSlot* MOTIterateForeignScan(ForeignScanState* node) MOTAdaptor::CreateKeyBuffer(node->ss.ss_currentRelation, festate, 0); MOT::Sentinel* Sentinel = festate->m_bestIx->m_ix->IndexReadSentinel(&festate->m_stateKey[0], festate->m_currTxn->GetThdId()); - festate->m_currRow = festate->m_currTxn->RowLookup(festate->m_internalCmdOper, Sentinel, rc); + currRow = festate->m_currTxn->RowLookup(festate->m_internalCmdOper, Sentinel, rc); - if (festate->m_currRow != NULL) { + if (currRow != NULL) { MOTAdaptor::UnpackRow( - slot, festate->m_table, festate->m_attrsUsed, const_cast(festate->m_currRow->GetData())); + slot, festate->m_table, festate->m_attrsUsed, const_cast(currRow->GetData())); node->ss.is_scan_end = true; fscan->scan.scan_qual_optimized = true; ExecStoreVirtualTuple(slot); if (festate->m_ctidNum > 0) { HeapTuple resultTup = ExecFetchSlotTuple(slot); MOTRecConvertSt cv; - cv.m_u.m_ptr = (uint64_t)festate->m_currRow->GetPrimarySentinel(); + cv.m_u.m_ptr = (uint64_t)currRow->GetPrimarySentinel(); resultTup->t_self = cv.m_u.m_self; HeapTupleSetXmin(resultTup, InvalidTransactionId); HeapTupleSetXmax(resultTup, InvalidTransactionId); @@ -1083,10 +1083,8 @@ static TupleTableSlot* MOTIterateForeignScan(ForeignScanState* node) do { MOT::Sentinel* Sentinel = festate->m_cursor[0]->GetPrimarySentinel(); - - festate->m_currRow = festate->m_currTxn->RowLookup(festate->m_internalCmdOper, Sentinel, rc); - - if (festate->m_currRow == NULL) { + currRow = festate->m_currTxn->RowLookup(festate->m_internalCmdOper, Sentinel, rc); + if (currRow == NULL) { if (rc != MOT::RC_OK) { if (MOT_IS_SEVERE()) { MOT_REPORT_ERROR(MOT_ERROR_INTERNAL, "MOTIterateForeignScan", "Failed to lookup row"); @@ -1113,7 +1111,7 @@ static TupleTableSlot* MOTIterateForeignScan(ForeignScanState* node) } MOTAdaptor::UnpackRow( - slot, festate->m_table, festate->m_attrsUsed, const_cast(festate->m_currRow->GetData())); + slot, festate->m_table, festate->m_attrsUsed, const_cast(currRow->GetData())); found = true; festate->m_cursor[0]->Next(); @@ -1126,7 +1124,7 @@ static TupleTableSlot* MOTIterateForeignScan(ForeignScanState* node) if (festate->m_ctidNum > 0) { HeapTuple resultTup = ExecFetchSlotTuple(slot); MOTRecConvertSt cv; - cv.m_u.m_ptr = (uint64_t)festate->m_currRow->GetPrimarySentinel(); + cv.m_u.m_ptr = (uint64_t)currRow->GetPrimarySentinel(); resultTup->t_self = cv.m_u.m_self; HeapTupleSetXmin(resultTup, InvalidTransactionId); HeapTupleSetXmax(resultTup, InvalidTransactionId); @@ -1456,58 +1454,48 @@ static TupleTableSlot* MOTExecForeignUpdate( { MOTFdwStateSt* fdwState = (MOTFdwStateSt*)resultRelInfo->ri_FdwState; MOT::RC rc = MOT::RC_OK; - TupleTableSlot* dataSlot = slot; - bool cleanCurrRow = false; - + MOT::Row* currRow; + AttrNumber num = fdwState->m_ctidNum - 1; + MOTRecConvertSt cv; + if (MOTAdaptor::m_engine->IsSoftMemoryLimitReached()) { CleanQueryStatesOnError(fdwState->m_currTxn); } isMemoryLimitReached(); - if (fdwState->m_currRow == nullptr) { - AttrNumber num = fdwState->m_ctidNum - 1; - if (fdwState->m_ctidNum != 0 && planSlot->tts_nvalid >= fdwState->m_ctidNum && !planSlot->tts_isnull[num]) { - MOTRecConvertSt cv; - cv.m_u.m_ptr = 0; - cv.m_u.m_self = *(ItemPointerData*)planSlot->tts_values[num]; - - fdwState->m_currRow = - fdwState->m_currTxn->RowLookup(fdwState->m_internalCmdOper, (MOT::Sentinel*)cv.m_u.m_ptr, rc); - } - - if (fdwState->m_currRow == nullptr) { - CleanQueryStatesOnError(fdwState->m_currTxn); - report_pg_error(MOT::RC_ERROR, fdwState->m_currTxn); - return nullptr; - } - - cleanCurrRow = true; - if (slot->tts_nvalid == 0) - dataSlot = planSlot; + if (fdwState->m_ctidNum != 0 && planSlot->tts_nvalid >= fdwState->m_ctidNum && !planSlot->tts_isnull[num]) { + cv.m_u.m_ptr = 0; + cv.m_u.m_self = *(ItemPointerData*)planSlot->tts_values[num]; + currRow = fdwState->m_currTxn->RowLookup(fdwState->m_internalCmdOper, (MOT::Sentinel*)cv.m_u.m_ptr, rc); } else { - fdwState->m_currRow = - fdwState->m_currTxn->RowLookup(fdwState->m_internalCmdOper, fdwState->m_currRow->GetPrimarySentinel(), rc); + elog(ERROR, "MOTExecForeignUpdate failed to fetch row for update ctid %d nvalid %d %s", + num, planSlot->tts_nvalid, (planSlot->tts_isnull[num] ? "NULL" : "NOT NULL")); + CleanQueryStatesOnError(fdwState->m_currTxn); + report_pg_error(MOT::RC_ERROR, fdwState->m_currTxn); + return nullptr; } - if ((rc = MOTAdaptor::UpdateRow(fdwState, dataSlot)) == MOT::RC_OK) { - if (cleanCurrRow) - fdwState->m_currRow = nullptr; + if (currRow == nullptr) { + elog(ERROR, "MOTExecForeignUpdate failed to fetch row"); + CleanQueryStatesOnError(fdwState->m_currTxn); + report_pg_error(((rc == MOT::RC_OK) ? MOT::RC_ERROR : rc), fdwState->m_currTxn); + return nullptr; + } - if (resultRelInfo->ri_projectReturning) - return dataSlot; - else { + if ((rc = MOTAdaptor::UpdateRow(fdwState, planSlot, currRow)) == MOT::RC_OK) { + if (resultRelInfo->ri_projectReturning) { + return planSlot; + } else { estate->es_processed++; return nullptr; } } else { - if (cleanCurrRow) - fdwState->m_currRow = nullptr; - elog(DEBUG2, "Abort parent transaction from MOT update, id %lu", fdwState->m_txnId); CleanQueryStatesOnError(fdwState->m_currTxn); report_pg_error(rc, fdwState->m_currTxn, - (void*)(fdwState->m_currTxn->m_errIx != NULL ? fdwState->m_currTxn->m_errIx->GetName().c_str() : "unknown"), + (void*)(fdwState->m_currTxn->m_errIx != nullptr ? fdwState->m_currTxn->m_errIx->GetName().c_str() + : "unknown"), (void*)fdwState->m_currTxn->m_errMsgBuf); return nullptr; } @@ -1518,50 +1506,40 @@ static TupleTableSlot* MOTExecForeignDelete( { MOTFdwStateSt* fdwState = (MOTFdwStateSt*)resultRelInfo->ri_FdwState; MOT::RC rc = MOT::RC_OK; - bool cleanCurrRow = false; + MOT::Row* currRow; + AttrNumber num = fdwState->m_ctidNum - 1; + MOTRecConvertSt cv; - if (fdwState->m_currRow == nullptr) { - AttrNumber num = fdwState->m_ctidNum - 1; - if (fdwState->m_ctidNum != 0 && planSlot->tts_nvalid >= fdwState->m_ctidNum && !planSlot->tts_isnull[num]) { - MOTRecConvertSt cv; - cv.m_u.m_ptr = 0; - cv.m_u.m_self = *(ItemPointerData*)planSlot->tts_values[num]; - - fdwState->m_currRow = - fdwState->m_currTxn->RowLookup(fdwState->m_internalCmdOper, (MOT::Sentinel*)cv.m_u.m_ptr, rc); - } - - if (fdwState->m_currRow == nullptr) { - CleanQueryStatesOnError(fdwState->m_currTxn); - report_pg_error(MOT::RC_ERROR, fdwState->m_currTxn); - return nullptr; - } - - cleanCurrRow = true; + if (fdwState->m_ctidNum != 0 && planSlot->tts_nvalid >= fdwState->m_ctidNum && !planSlot->tts_isnull[num]) { + cv.m_u.m_ptr = 0; + cv.m_u.m_self = *(ItemPointerData*)planSlot->tts_values[num]; + currRow = fdwState->m_currTxn->RowLookup(fdwState->m_internalCmdOper, (MOT::Sentinel*)cv.m_u.m_ptr, rc); } else { - fdwState->m_currRow = - fdwState->m_currTxn->RowLookup(fdwState->m_internalCmdOper, fdwState->m_currRow->GetPrimarySentinel(), rc); + elog(ERROR, "MOTExecForeignDelete failed to fetch row for delete ctid %d nvalid %d %s", + num, planSlot->tts_nvalid, (planSlot->tts_isnull[num] ? "NULL" : "NOT NULL")); + CleanQueryStatesOnError(fdwState->m_currTxn); + report_pg_error(MOT::RC_ERROR, fdwState->m_currTxn); + return nullptr; + } + + if (currRow == nullptr) { + elog(ERROR, "MOTExecForeignDelete failed to fetch row"); + CleanQueryStatesOnError(fdwState->m_currTxn); + report_pg_error(((rc == MOT::RC_OK) ? MOT::RC_ERROR : rc), fdwState->m_currTxn); + return nullptr; } if ((rc = MOTAdaptor::DeleteRow(fdwState, slot)) == MOT::RC_OK) { if (resultRelInfo->ri_projectReturning) { MOTAdaptor::UnpackRow( - slot, fdwState->m_table, fdwState->m_attrsUsed, const_cast(fdwState->m_currRow->GetData())); + slot, fdwState->m_table, fdwState->m_attrsUsed, const_cast(currRow->GetData())); ExecStoreVirtualTuple(slot); - if (cleanCurrRow) - fdwState->m_currRow = nullptr; - return slot; } else { - if (cleanCurrRow) - fdwState->m_currRow = nullptr; estate->es_processed++; return nullptr; } } else { - if (cleanCurrRow) - fdwState->m_currRow = nullptr; - elog(DEBUG2, "Abort parent transaction from MOT delete, id %lu", fdwState->m_txnId); CleanQueryStatesOnError(fdwState->m_currTxn); report_pg_error(rc, diff --git a/src/gausskernel/storage/mot/fdw_adapter/src/mot_internal.cpp b/src/gausskernel/storage/mot/fdw_adapter/src/mot_internal.cpp index 3e5a112df..65e10e5cc 100644 --- a/src/gausskernel/storage/mot/fdw_adapter/src/mot_internal.cpp +++ b/src/gausskernel/storage/mot/fdw_adapter/src/mot_internal.cpp @@ -1138,7 +1138,7 @@ MOT::RC MOTAdaptor::InsertRow(MOTFdwStateSt* fdwState, TupleTableSlot* slot) return res; } -MOT::RC MOTAdaptor::UpdateRow(MOTFdwStateSt* fdwState, TupleTableSlot* slot) +MOT::RC MOTAdaptor::UpdateRow(MOTFdwStateSt* fdwState, TupleTableSlot* slot, MOT::Row* currRow) { EnsureSafeThreadAccessInline(); MOT::RC rc; @@ -1149,11 +1149,11 @@ MOT::RC MOTAdaptor::UpdateRow(MOTFdwStateSt* fdwState, TupleTableSlot* slot) if (rc != MOT::RC::RC_OK) { break; } - uint8_t* rowData = const_cast(fdwState->m_currRow->GetData()); + uint8_t* rowData = const_cast(currRow->GetData()); PackUpdateRow(slot, fdwState->m_table, fdwState->m_attrsModified, rowData); MOT::BitmapSet modified_columns(fdwState->m_attrsModified, fdwState->m_table->GetFieldCount() - 1); - rc = fdwState->m_currTxn->OverwriteRow(fdwState->m_currRow, modified_columns); + rc = fdwState->m_currTxn->OverwriteRow(currRow, modified_columns); } while (0); return rc; diff --git a/src/gausskernel/storage/mot/fdw_adapter/src/mot_internal.h b/src/gausskernel/storage/mot/fdw_adapter/src/mot_internal.h index 4c92b7b80..00f5d1bb6 100644 --- a/src/gausskernel/storage/mot/fdw_adapter/src/mot_internal.h +++ b/src/gausskernel/storage/mot/fdw_adapter/src/mot_internal.h @@ -273,7 +273,6 @@ struct MOTFdwState_St { MOT::Table* m_table; MOT::IndexIterator* m_cursor[2] = {nullptr, nullptr}; MOT::TxnManager* m_currTxn; - MOT::Row* m_currRow = nullptr; void* m_currItem = nullptr; uint32_t m_rowsFound = 0; bool m_cursorOpened = false; @@ -338,7 +337,7 @@ public: static MOT::RC RollbackPrepared(TransactionId tid); static MOT::RC FailedCommitPrepared(TransactionId tid); static MOT::RC InsertRow(MOTFdwStateSt* fdwState, TupleTableSlot* slot); - static MOT::RC UpdateRow(MOTFdwStateSt* fdwState, TupleTableSlot* slot); + static MOT::RC UpdateRow(MOTFdwStateSt* fdwState, TupleTableSlot* slot, MOT::Row* currRow); static MOT::RC DeleteRow(MOTFdwStateSt* fdwState, TupleTableSlot* slot); /* Convertors */