Repository: incubator-trafodion Updated Branches: refs/heads/master d2c464c8d -> 24a3ddb96
[TRAFODION-1850] First N operator no longer returns extra rows. This is the entire or a partial fix to the problem where a select [first n] * from udf(...) can return more than n rows. The problem was that neither FirstN nor the UDF operator fully implemented the GET_N protocol. While that may be ok for the UDF, the FirstN needed a fix. Project: http://git-wip-us.apache.org/repos/asf/incubator-trafodion/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-trafodion/commit/f614456a Tree: http://git-wip-us.apache.org/repos/asf/incubator-trafodion/tree/f614456a Diff: http://git-wip-us.apache.org/repos/asf/incubator-trafodion/diff/f614456a Branch: refs/heads/master Commit: f614456a56d665587b706da2fb6bb82b5dcc0000 Parents: fa430a0 Author: Hans Zeller <[email protected]> Authored: Mon Feb 29 23:12:58 2016 +0000 Committer: Hans Zeller <[email protected]> Committed: Mon Feb 29 23:12:58 2016 +0000 ---------------------------------------------------------------------- core/sql/executor/ExFirstN.cpp | 39 +++++++++++++++++++++++++++---------- core/sql/executor/ExFirstN.h | 4 +++- 2 files changed, 32 insertions(+), 11 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-trafodion/blob/f614456a/core/sql/executor/ExFirstN.cpp ---------------------------------------------------------------------- diff --git a/core/sql/executor/ExFirstN.cpp b/core/sql/executor/ExFirstN.cpp index c995ec6..20b1cb9 100644 --- a/core/sql/executor/ExFirstN.cpp +++ b/core/sql/executor/ExFirstN.cpp @@ -100,6 +100,8 @@ ExFirstNTcb::ExFirstNTcb(const ExFirstNTdb & firstn_tdb, workAtp_ = NULL; firstNParamVal_ = 0; + effectiveFirstN_ = -1; + returnedSoFar_ = 0; if (firstn_tdb.workCriDesc_) { workAtp_ = allocateAtp(firstn_tdb.workCriDesc_, space); @@ -197,13 +199,15 @@ short ExFirstNTcb::work() ex_queue_entry * centry = qchild_.down->getTailEntry(); - // firstNRows_ or firstNRowsParam is set to a positive number + // effectiveFirstN_ is set to a positive number // if FIRST N rows are requested. // It is set to 0 or a negative number, if last N rows are needed. // 0 means process all but don't return any rows. // -1 means get all rows. Should not reach this state. - // -ve number means return the last '-(N+2)' rows. - Lng32 firstNVal = firstnTdb().firstNRows(); + // <-1 means return the last '-(N+2)' rows. + effectiveFirstN_ = firstnTdb().firstNRows(); + returnedSoFar_ = 0; + if (firstnTdb().firstNRowsExpr_) { ex_expr::exp_return_type evalRetCode = @@ -215,10 +219,10 @@ short ExFirstNTcb::work() break; } - firstNVal = firstNParamVal_; + effectiveFirstN_ = firstNParamVal_; } - if (firstNVal >= 0) + if (effectiveFirstN_ >= 0) { centry->downState.request = ex_queue::GET_N; @@ -228,11 +232,11 @@ short ExFirstNTcb::work() // GET_N request value and firstNRows_ to my child. if ((pentry_down->downState.request != ex_queue::GET_N) || (pentry_down->downState.requestValue == firstnTdb().firstNRows())) - centry->downState.requestValue = firstNVal; + centry->downState.requestValue = effectiveFirstN_; else { centry->downState.requestValue = - MINOF(pentry_down->downState.requestValue, firstNVal); + MINOF(pentry_down->downState.requestValue, effectiveFirstN_); } step_ = PROCESS_FIRSTN_; @@ -243,7 +247,7 @@ short ExFirstNTcb::work() centry->downState.request = ex_queue::GET_ALL; centry->downState.requestValue = 11; - requestedLastNRows_ = -(firstNVal + 2); + requestedLastNRows_ = -(effectiveFirstN_ + 2); returnedLastNRows_ = 0; step_ = PROCESS_LASTN_; @@ -269,7 +273,19 @@ short ExFirstNTcb::work() { case ex_queue::Q_OK_MMORE: { - moveChildDataToParent(); + if (returnedSoFar_ < effectiveFirstN_) + { + moveChildDataToParent(); + returnedSoFar_++; + } + else + { + // looks like the child may not honor our + // GET_N request, so send a cancel, maybe + // that will work better + qchild_.down->cancelRequest(); + step_ = CANCEL_; + } } break; @@ -324,7 +340,10 @@ short ExFirstNTcb::work() { // We know that current entry is Q_OK_MMORE. // Need atleast 1 more entry than requested to process - // last N. + // last N. Note that there is a small chance that this + // will lead to a buffer deadlock (child's buffer pool + // is full, child expects us to consume a row before it + // can produce another one). if (qchild_.up->getLength() < 1 + 1) return WORK_OK; http://git-wip-us.apache.org/repos/asf/incubator-trafodion/blob/f614456a/core/sql/executor/ExFirstN.h ---------------------------------------------------------------------- diff --git a/core/sql/executor/ExFirstN.h b/core/sql/executor/ExFirstN.h index de4a022..007956e 100644 --- a/core/sql/executor/ExFirstN.h +++ b/core/sql/executor/ExFirstN.h @@ -134,7 +134,9 @@ class ExFirstNTcb : public ex_tcb Int64 returnedLastNRows_; atp_struct * workAtp_; - Lng32 firstNParamVal_; + Lng32 firstNParamVal_; // first N computed from parameter + Lng32 effectiveFirstN_; // effective first n (constant or param) + Lng32 returnedSoFar_; // number of rows returned so far // Stub to cancel() subtask used by scheduler. static ExWorkProcRetcode sCancel(ex_tcb *tcb)
