From c69be87dde510728fafdbe9520df2602573e9522 Mon Sep 17 00:00:00 2001 From: selvaganesang Date: Tue, 10 Nov 2015 17:16:46 +0000 Subject: [PATCH] [TRAFODION-1575] Avoid transforming update into delete and insert commands to improve performance of update statements. When a table has check constraint the update command is transformed into insert and delete. Added code to evaluate constraint expressions in all TCBs so that constraint checking is done correctly. But, the subtask [TRAFODION-1610] needs to implemented before this change is exercised. --- core/sql/comexe/ComTdbHbaseAccess.cpp | 18 +++++- core/sql/comexe/ComTdbHbaseAccess.h | 9 ++- core/sql/executor/ExHbaseAccess.cpp | 24 +++++++ core/sql/executor/ExHbaseAccess.h | 15 ++++- core/sql/executor/ExHbaseIUD.cpp | 91 +++++++++++++++++---------- core/sql/generator/GenRelUpdate.cpp | 39 ++++++++++-- core/sql/optimizer/BindRelExpr.cpp | 21 +++---- 7 files changed, 161 insertions(+), 56 deletions(-) diff --git a/core/sql/comexe/ComTdbHbaseAccess.cpp b/core/sql/comexe/ComTdbHbaseAccess.cpp index a85751f654..c3ca3f67dd 100644 --- a/core/sql/comexe/ComTdbHbaseAccess.cpp +++ b/core/sql/comexe/ComTdbHbaseAccess.cpp @@ -131,6 +131,8 @@ ComTdbHbaseAccess::ComTdbHbaseAccess( encodedKeyExpr_(encodedKeyExpr), keyColValExpr_(keyColValExpr), insDelPreCondExpr_(NULL), + insConstraintExpr_(NULL), + updConstraintExpr_(NULL), hbaseFilterExpr_(hbaseFilterExpr), asciiRowLen_(asciiRowLen), @@ -239,6 +241,8 @@ ComTdbHbaseAccess::ComTdbHbaseAccess( encodedKeyExpr_(NULL), keyColValExpr_(NULL), insDelPreCondExpr_(NULL), + insConstraintExpr_(NULL), + updConstraintExpr_(NULL), hbaseFilterExpr_(NULL), asciiRowLen_(0), @@ -311,7 +315,7 @@ ComTdbHbaseAccess::~ComTdbHbaseAccess() Int32 ComTdbHbaseAccess::numExpressions() const { - return(16); + return 18; } // Return the expression names of the explain TDB based on some @@ -353,6 +357,10 @@ ComTdbHbaseAccess::getExpressionName(Int32 expNum) const return "hbaseFilterExpr"; case 15: return "preCondExpr"; + case 16: + return "insConstraintExpr"; + case 17: + return "updConstraintExpr"; default: return 0; } @@ -397,6 +405,10 @@ ComTdbHbaseAccess::getExpressionNode(Int32 expNum) return hbaseFilterExpr_; case 15: return insDelPreCondExpr_; + case 16: + return insConstraintExpr_; + case 17: + return updConstraintExpr_; default: return NULL; } @@ -418,6 +430,8 @@ Long ComTdbHbaseAccess::pack(void * space) encodedKeyExpr_.pack(space); keyColValExpr_.pack(space); insDelPreCondExpr_.pack(space); + insConstraintExpr_.pack(space); + updConstraintExpr_.pack(space); hbaseFilterExpr_.pack(space); colFamNameList_.pack(space); workCriDesc_.pack(space); @@ -486,6 +500,8 @@ Lng32 ComTdbHbaseAccess::unpack(void * base, void * reallocator) if(encodedKeyExpr_.unpack(base, reallocator)) return -1; if(keyColValExpr_.unpack(base, reallocator)) return -1; if(insDelPreCondExpr_.unpack(base, reallocator)) return -1; + if(insConstraintExpr_.unpack(base, reallocator)) return -1; + if(updConstraintExpr_.unpack(base, reallocator)) return -1; if(hbaseFilterExpr_.unpack(base, reallocator)) return -1; if(colFamNameList_.unpack(base, reallocator)) return -1; if(workCriDesc_.unpack(base, reallocator)) return -1; diff --git a/core/sql/comexe/ComTdbHbaseAccess.h b/core/sql/comexe/ComTdbHbaseAccess.h index bf5bb429c2..b371199678 100644 --- a/core/sql/comexe/ComTdbHbaseAccess.h +++ b/core/sql/comexe/ComTdbHbaseAccess.h @@ -846,7 +846,12 @@ class ComTdbHbaseAccess : public ComTdb void setInsDelPreCondExpr(ExExprPtr exprPtr) { insDelPreCondExpr_ = exprPtr; } - + void setInsConstraintExpr(ExExprPtr exprPtr) { + insConstraintExpr_ = exprPtr; + } + void setUpdConstraintExpr(ExExprPtr exprPtr) { + updConstraintExpr_ = exprPtr; + } protected: enum { @@ -944,6 +949,8 @@ class ComTdbHbaseAccess : public ComTdb ExExprPtr keyColValExpr_; ExExprPtr insDelPreCondExpr_; ExExprPtr hbaseFilterExpr_; + ExExprPtr insConstraintExpr_; + ExExprPtr updConstraintExpr_; ExCriDescPtr workCriDesc_; diff --git a/core/sql/executor/ExHbaseAccess.cpp b/core/sql/executor/ExHbaseAccess.cpp index 8e98daa66b..65595871d5 100644 --- a/core/sql/executor/ExHbaseAccess.cpp +++ b/core/sql/executor/ExHbaseAccess.cpp @@ -338,6 +338,10 @@ ExHbaseAccessTcb::ExHbaseAccessTcb( keyColValExpr()->fixup(0, getExpressionMode(), this, space, heap, FALSE, glob); if (insDelPreCondExpr()) insDelPreCondExpr()->fixup(0, getExpressionMode(), this, space, heap, FALSE, glob); + if (insConstraintExpr()) + insConstraintExpr()->fixup(0, getExpressionMode(), this, space, heap, FALSE, glob); + if (updConstraintExpr()) + updConstraintExpr()->fixup(0, getExpressionMode(), this, space, heap, FALSE, glob); if (hbaseFilterValExpr()) hbaseFilterValExpr()->fixup(0, getExpressionMode(), this, space, heap, FALSE, glob); @@ -1834,6 +1838,26 @@ short ExHbaseAccessTcb::evalInsDelPreCondExpr() return 0; } +short ExHbaseAccessTcb::evalConstraintExpr(ex_expr *expr, UInt16 tuppIndex, + char * tuppRow) +{ + if (expr == NULL) { + return 1; + } + ex_queue_entry *pentry_down = qparent_.down->getHeadEntry(); + ex_expr::exp_return_type exprRetCode = ex_expr::EXPR_OK; + if ((tuppRow) && (tuppIndex > 0)) + workAtp_->getTupp(tuppIndex).setDataPointer(tuppRow); + else + workAtp_->getTupp(hbaseAccessTdb().convertTuppIndex_).setDataPointer(convertRow_); + exprRetCode = expr->eval(pentry_down->getAtp(), workAtp_); + if (exprRetCode == ex_expr::EXPR_ERROR) + return -1; + else if (exprRetCode == ex_expr::EXPR_TRUE) + return 1; + else + return 0; +} short ExHbaseAccessTcb::evalRowIdAsciiExpr(const char * inputRowIdVals, char * rowIdBuf, // input: buffer where rowid is created diff --git a/core/sql/executor/ExHbaseAccess.h b/core/sql/executor/ExHbaseAccess.h index 92712278ff..85c50beb91 100644 --- a/core/sql/executor/ExHbaseAccess.h +++ b/core/sql/executor/ExHbaseAccess.h @@ -235,6 +235,12 @@ class ExHbaseAccessTcb : public ex_tcb inline ex_expr *insDelPreCondExpr() const { return hbaseAccessTdb().insDelPreCondExpr_; } + inline ex_expr *insConstraintExpr() const + { return hbaseAccessTdb().insConstraintExpr_; } + + inline ex_expr *updConstraintExpr() const + { return hbaseAccessTdb().updConstraintExpr_; } + inline ex_expr *hbaseFilterValExpr() const { return hbaseAccessTdb().hbaseFilterExpr_; } @@ -299,6 +305,8 @@ class ExHbaseAccessTcb : public ex_tcb short extractColFamilyAndName(char * input, Text &colFam, Text &colName); short evalKeyColValExpr(HbaseStr &columnToCheck, HbaseStr &colValToCheck); short evalInsDelPreCondExpr(); + short evalConstraintExpr(ex_expr *expr, UInt16 tuppIndex = 0, + char * tuppRow = NULL); short evalEncodedKeyExpr(); short evalRowIdExpr(NABoolean noVarchar = FALSE); short evalRowIdAsciiExpr(NABoolean noVarchar = FALSE); @@ -954,7 +962,8 @@ class ExHbaseUMDtrafSubsetTaskTcb : public ExHbaseTaskTcb , NEXT_ROW , CREATE_FETCHED_ROW , CREATE_UPDATED_ROW - , EVAL_CONSTRAINT + , EVAL_INS_CONSTRAINT + , EVAL_UPD_CONSTRAINT , CREATE_MUTATIONS , APPLY_PRED , APPLY_MERGE_UPD_SCAN_PRED @@ -1021,7 +1030,8 @@ class ExHbaseUMDtrafUniqueTaskTcb : public ExHbaseTaskTcb , CREATE_UPDATED_ROW , CREATE_MERGE_INSERTED_ROW , CREATE_MUTATIONS - , EVAL_CONSTRAINT + , EVAL_INS_CONSTRAINT + , EVAL_UPD_CONSTRAINT , APPLY_PRED , APPLY_MERGE_UPD_SCAN_PRED , RETURN_ROW @@ -1133,6 +1143,7 @@ class ExHbaseAccessSQRowsetTcb : public ExHbaseAccessTcb , CREATE_UPDATED_ROW , PROCESS_DELETE , PROCESS_DELETE_AND_CLOSE + , EVAL_CONSTRAINT , PROCESS_UPDATE , PROCESS_UPDATE_AND_CLOSE , PROCESS_SELECT diff --git a/core/sql/executor/ExHbaseIUD.cpp b/core/sql/executor/ExHbaseIUD.cpp index 5e810a4ad7..962796cb54 100644 --- a/core/sql/executor/ExHbaseIUD.cpp +++ b/core/sql/executor/ExHbaseIUD.cpp @@ -476,12 +476,12 @@ ExWorkProcRetcode ExHbaseAccessInsertSQTcb::work() case EVAL_CONSTRAINT: { - rc = applyPred(scanExpr()); - if (rc == 1) // expr is true or no expr + rc = evalConstraintExpr(insConstraintExpr()); + if (rc == 1) step_ = CREATE_MUTATIONS; - else if (rc == 0) // expr is false + else if (rc == 0) step_ = INSERT_CLOSE; - else // error + else step_ = HANDLE_ERROR; } break; @@ -862,12 +862,12 @@ ExWorkProcRetcode ExHbaseAccessUpsertVsbbSQTcb::work() case EVAL_CONSTRAINT: { - rc = applyPred(scanExpr()); - if (rc == 1) // expr is true or no expr + rc = evalConstraintExpr(insConstraintExpr()); + if (rc == 1) step_ = CREATE_MUTATIONS; - else if (rc == 0) // expr is false + else if (rc == 0) step_ = INSERT_CLOSE; - else // error + else step_ = HANDLE_ERROR; } break; @@ -1957,18 +1957,19 @@ ExWorkProcRetcode ExHbaseUMDtrafUniqueTaskTcb::work(short &rc) } } - step_ = EVAL_CONSTRAINT; + step_ = EVAL_UPD_CONSTRAINT; } break; - case EVAL_CONSTRAINT: + case EVAL_UPD_CONSTRAINT: { - rc = tcb_->applyPred(tcb_->mergeUpdScanExpr()); - if (rc == 1) // expr is true or no expr + rc = tcb_->evalConstraintExpr(tcb_->updConstraintExpr(), tcb_->hbaseAccessTdb().updateTuppIndex_, + tcb_->updateRow_); + if (rc == 1) step_ = CREATE_MUTATIONS; - else if (rc == 0) // expr is false - step_ = NEXT_ROW_AFTER_UPDATE; - else // error + else if (rc == 0) + step_ = GET_CLOSE; + else step_ = HANDLE_ERROR; } break; @@ -2040,9 +2041,22 @@ ExWorkProcRetcode ExHbaseUMDtrafUniqueTaskTcb::work(short &rc) break; } } - - if (tcb_->hbaseAccessTdb().getAccessType() == ComTdbHbaseAccess::MERGE_) - rowUpdated_ = FALSE; + if (tcb_->hbaseAccessTdb().getAccessType() == ComTdbHbaseAccess::MERGE_) + rowUpdated_ = FALSE; + step_ = EVAL_INS_CONSTRAINT; + } + break; + case EVAL_INS_CONSTRAINT: + { + rc = tcb_->evalConstraintExpr(tcb_->insConstraintExpr()); + if (rc == 0) { + step_ = GET_CLOSE; + break; + } + else if (rc != 1) { + step_ = HANDLE_ERROR; + break; + } retcode = tcb_->createDirectRowBuffer( tcb_->hbaseAccessTdb().mergeInsertTuppIndex_, tcb_->mergeInsertRow_, @@ -2875,18 +2889,18 @@ ExWorkProcRetcode ExHbaseUMDtrafSubsetTaskTcb::work(short &rc) } } - step_ = EVAL_CONSTRAINT; + step_ = EVAL_UPD_CONSTRAINT; } break; - case EVAL_CONSTRAINT: + case EVAL_UPD_CONSTRAINT: { - rc = tcb_->applyPred(tcb_->mergeUpdScanExpr(), - tcb_->hbaseAccessTdb().updateTuppIndex_, tcb_->updateRow_); - if (rc == 1) // expr is true or no expr + rc = tcb_->evalConstraintExpr(tcb_->updConstraintExpr(), tcb_->hbaseAccessTdb().updateTuppIndex_, + tcb_->updateRow_); + if (rc == 1) step_ = CREATE_MUTATIONS; - else if (rc == 0) // expr is false - step_ = NEXT_ROW; + else if (rc == 0) + step_ = SCAN_CLOSE; else // error step_ = HANDLE_ERROR; } @@ -4129,8 +4143,7 @@ ExWorkProcRetcode ExHbaseAccessSQRowsetTcb::work() workAtp_->getTupp(hbaseAccessTdb().updateTuppIndex_) .setDataPointer(updateRow_); - if (updateExpr()) - { + if (updateExpr()) { ex_expr::exp_return_type evalRetCode = updateExpr()->eval(pentry_down->getAtp(), workAtp_); if (evalRetCode == ex_expr::EXPR_ERROR) @@ -4138,23 +4151,33 @@ ExWorkProcRetcode ExHbaseAccessSQRowsetTcb::work() step_ = HANDLE_ERROR; break; } - } - + } + step_ = EVAL_CONSTRAINT; + } + break; + case EVAL_CONSTRAINT: + { + rc = evalConstraintExpr(updConstraintExpr(), hbaseAccessTdb().updateTuppIndex_,updateRow_); + if (rc == 0) { + step_ = RS_CLOSE; + break; + } + else if (rc != 1) { + step_ = HANDLE_ERROR; + break; + } retcode = createDirectRowBuffer( hbaseAccessTdb().updateTuppIndex_, updateRow_, hbaseAccessTdb().listOfUpdatedColNames(), TRUE); - if (retcode == -1) - { + if (retcode == -1) { step_ = HANDLE_ERROR; break; - } - + } step_ = NEXT_ROW; } break; - case PROCESS_UPDATE: case PROCESS_UPDATE_AND_CLOSE: { diff --git a/core/sql/generator/GenRelUpdate.cpp b/core/sql/generator/GenRelUpdate.cpp index 4454568696..9c2c140751 100644 --- a/core/sql/generator/GenRelUpdate.cpp +++ b/core/sql/generator/GenRelUpdate.cpp @@ -427,6 +427,11 @@ static short genMergeInsertExpr( return 0; } +static short genUpdConstraintExpr(Generator *generator) +{ + return 0; +} + static short genHbaseUpdOrInsertExpr( Generator * generator, NABoolean isInsert, @@ -1381,6 +1386,8 @@ short HbaseUpdate::codeGen(Generator * generator) ex_expr *mergeInsertExpr = NULL; ex_expr *returnUpdateExpr = NULL; ex_expr * keyColValExpr = NULL; + ex_expr * insConstraintExpr = NULL; + ex_expr * updConstraintExpr = NULL; ex_cri_desc * givenDesc = generator->getCriDesc(Generator::DOWN); @@ -1761,6 +1768,18 @@ short HbaseUpdate::codeGen(Generator * generator) else if (getIndexDesc()->isClusteringIndex() && getCheckConstraints().entries()) { GenAssert(FALSE, "Should not reach here. This update should have been transformed to delete/insert"); + // To be uncommented when TRAFODION-1610 is implemented + // Need to generate insConstraintExpr also +/* + ItemExpr *constrTree = + getCheckConstraints().rebuildExprTree(ITM_AND, TRUE, TRUE); + + if (getTableDesc()->getNATable()->hasSerializedEncodedColumn()) + constrTree = generator->addCompDecodeForDerialization(constrTree); + + expGen->generateExpr(constrTree->getValueId(), ex_expr::exp_SCAN_PRED, + &updConstraintExpr); +*/ } if ((getTableDesc()->getNATable()->isSeabaseTable()) && @@ -2121,6 +2140,12 @@ short HbaseUpdate::codeGen(Generator * generator) if (getTableDesc()->getNATable()->isHbaseRowTable()) hbasescan_tdb->setRowwiseFormat(TRUE); + if (updConstraintExpr) + hbasescan_tdb->setUpdConstraintExpr(updConstraintExpr); + + if (insConstraintExpr) + hbasescan_tdb->setInsConstraintExpr(insConstraintExpr); + if (getTableDesc()->getNATable()->isSeabaseTable()) { hbasescan_tdb->setSQHbaseTable(TRUE); @@ -2381,7 +2406,7 @@ short HbaseInsert::codeGen(Generator *generator) // Only works for base tables because the constraint information is // stored with the table descriptor which doesn't exist for indexes. // - ex_expr * constraintExpr = NULL; + ex_expr * insConstraintExpr = NULL; Queue * listOfUpdatedColNames = NULL; Lng32 keyAttrPos = -1; ex_expr * rowIdExpr = NULL; @@ -2389,6 +2414,7 @@ short HbaseInsert::codeGen(Generator *generator) ValueIdList savedInputVIDlist; NAList savedInputAttrsList; + const ValueIdList &indexVIDlist = getIndexDesc()->getIndexColumns(); CollIndex jj = 0; for (CollIndex ii = 0; ii < newRecExprArray().entries(); ii++) @@ -2415,7 +2441,8 @@ short HbaseInsert::codeGen(Generator *generator) colAttr->copyLocationAttrs(castAttr); indexAttr->copyLocationAttrs(castAttr); - + // To be removed when TRAFODION-1610 is implemented + // ` // if any of the target column is also an input value to this operator, then // make the value id of that input point to the location of the target column. // This is done as the input column value will become the target after this @@ -2506,9 +2533,10 @@ short HbaseInsert::codeGen(Generator *generator) constrTree = generator->addCompDecodeForDerialization(constrTree); expGen->generateExpr(constrTree->getValueId(), ex_expr::exp_SCAN_PRED, - &constraintExpr); + &insConstraintExpr); // restore original attribute values + // To be removed when TRAFODION-1610 is implemented for (Lng32 i = 0; i < savedInputVIDlist.entries(); i++) { ValueId inputValId = savedInputVIDlist[i]; @@ -2716,7 +2744,7 @@ short HbaseInsert::codeGen(Generator *generator) t, tablename, insertExpr, - constraintExpr, + NULL, rowIdExpr, loggingDataExpr, // logging expr NULL, // mergeInsertExpr @@ -2794,6 +2822,9 @@ short HbaseInsert::codeGen(Generator *generator) if (preCondExpr) hbasescan_tdb->setInsDelPreCondExpr(preCondExpr); + if (insConstraintExpr) + hbasescan_tdb->setInsConstraintExpr(insConstraintExpr); + if (getTableDesc()->getNATable()->isSeabaseTable()) { hbasescan_tdb->setSQHbaseTable(TRUE); diff --git a/core/sql/optimizer/BindRelExpr.cpp b/core/sql/optimizer/BindRelExpr.cpp index 5e15d029a6..60f7d3e970 100644 --- a/core/sql/optimizer/BindRelExpr.cpp +++ b/core/sql/optimizer/BindRelExpr.cpp @@ -10535,7 +10535,7 @@ RelExpr *Update::bindNode(BindWA *bindWA) NABoolean transformUpdateKey = updatesClusteringKeyOrUniqueIndexKey(bindWA); if (bindWA->errStatus()) // error occurred in updatesCKOrUniqueIndexKey() return this; - + // To be removed when TRAFODION-1610 is implemented. NABoolean xnsfrmHbaseUpdate = FALSE; if ((hbaseOper()) && (NOT isMerge())) { @@ -10543,26 +10543,19 @@ RelExpr *Update::bindNode(BindWA *bindWA) { xnsfrmHbaseUpdate = TRUE; } - else if ((CmpCommon::getDefault(HBASE_TRANSFORM_UPDATE_TO_DELETE_INSERT) == DF_SYSTEM) && - (getTableDesc()->getNATable()->hasSecondaryIndexes())) - { - xnsfrmHbaseUpdate = TRUE; - } - else if (avoidHalloween()) - { - xnsfrmHbaseUpdate = TRUE; - } else if (getCheckConstraints().entries()) - { - xnsfrmHbaseUpdate = TRUE; - } + { + xnsfrmHbaseUpdate = TRUE; + } } if (xnsfrmHbaseUpdate) { boundExpr = transformHbaseUpdate(bindWA); } - else if ((transformUpdateKey) && (NOT isMerge())) + else + // till here and remove the function transformHbaseUpdate also + if ((transformUpdateKey) && (NOT isMerge())) { boundExpr = transformUpdatePrimaryKey(bindWA); }