From aa581b5dc3d88b4d0d07a224de4da5c96cf23aa3 Mon Sep 17 00:00:00 2001 From: David Hall Date: Mon, 2 Apr 2018 17:02:54 -0500 Subject: [PATCH] MCOL-1234 Nested CASE filters not processed Change the way buildCaseFunction() handles the ptWorkStack and the rcWorkStack. --- dbcon/execplan/arithmeticoperator.cpp | 2 +- dbcon/execplan/filter.cpp | 2 +- dbcon/execplan/logicoperator.cpp | 2 +- dbcon/execplan/predicateoperator.cpp | 2 +- dbcon/execplan/simplecolumn.cpp | 10 +- dbcon/execplan/treenodeimpl.cpp | 2 +- dbcon/mysql/ha_calpont_execplan.cpp | 146 +++++++++++--------------- 7 files changed, 73 insertions(+), 93 deletions(-) diff --git a/dbcon/execplan/arithmeticoperator.cpp b/dbcon/execplan/arithmeticoperator.cpp index b16b0dbeb..122f481ae 100644 --- a/dbcon/execplan/arithmeticoperator.cpp +++ b/dbcon/execplan/arithmeticoperator.cpp @@ -102,7 +102,7 @@ void ArithmeticOperator::unserialize(messageqcpp::ByteStream& b) bool ArithmeticOperator::operator==(const ArithmeticOperator& t) const { - if (fData == t.fData) + if (data() == t.data()) return true; return false; } diff --git a/dbcon/execplan/filter.cpp b/dbcon/execplan/filter.cpp index 386acd256..7368d4379 100644 --- a/dbcon/execplan/filter.cpp +++ b/dbcon/execplan/filter.cpp @@ -72,7 +72,7 @@ const string Filter::toString() const bool Filter::operator==(const Filter& t) const { - if (fData == t.fData) + if (data() == t.data()) return true; return false; } diff --git a/dbcon/execplan/logicoperator.cpp b/dbcon/execplan/logicoperator.cpp index 9c12d27ae..99a7d0656 100644 --- a/dbcon/execplan/logicoperator.cpp +++ b/dbcon/execplan/logicoperator.cpp @@ -106,7 +106,7 @@ void LogicOperator::unserialize(messageqcpp::ByteStream& b) bool LogicOperator::operator==(const LogicOperator& t) const { - if (fData == t.fData) + if (data() == t.data()) return true; return false; } diff --git a/dbcon/execplan/predicateoperator.cpp b/dbcon/execplan/predicateoperator.cpp index 38e7a2f54..2b47ec058 100644 --- a/dbcon/execplan/predicateoperator.cpp +++ b/dbcon/execplan/predicateoperator.cpp @@ -152,7 +152,7 @@ void PredicateOperator::unserialize(messageqcpp::ByteStream& b) bool PredicateOperator::operator==(const PredicateOperator& t) const { - if (fData == t.fData) + if (data() == t.data()) return true; return false; } diff --git a/dbcon/execplan/simplecolumn.cpp b/dbcon/execplan/simplecolumn.cpp index a3a4e0dac..895334fc9 100644 --- a/dbcon/execplan/simplecolumn.cpp +++ b/dbcon/execplan/simplecolumn.cpp @@ -370,16 +370,16 @@ bool SimpleColumn::operator==(const SimpleColumn& t) const return false; if (fColumnName != t.fColumnName) return false; - if (fIndexName != t.fIndexName) - return false; +// if (fIndexName != t.fIndexName) +// return false; if (fViewName != t.fViewName) return false; if (fOid != t.fOid) return false; - if (fData != t.fData) - return false; - if (fAlias != t.fAlias) + if (data() != t.data()) return false; +// if (fAlias != t.fAlias) +// return false; if (fTableAlias != t.fTableAlias) return false; if (fAsc != t.fAsc) diff --git a/dbcon/execplan/treenodeimpl.cpp b/dbcon/execplan/treenodeimpl.cpp index 6ffe2787a..50fa7d209 100644 --- a/dbcon/execplan/treenodeimpl.cpp +++ b/dbcon/execplan/treenodeimpl.cpp @@ -63,7 +63,7 @@ const string TreeNodeImpl::toString() const bool TreeNodeImpl::operator==(const TreeNodeImpl& t) const { - if (fData == t.fData) + if (data() == t.data()) return true; return false; } diff --git a/dbcon/mysql/ha_calpont_execplan.cpp b/dbcon/mysql/ha_calpont_execplan.cpp index 7e138792b..da80366b8 100755 --- a/dbcon/mysql/ha_calpont_execplan.cpp +++ b/dbcon/mysql/ha_calpont_execplan.cpp @@ -3190,107 +3190,85 @@ FunctionColumn* buildCaseFunction(Item_func* item, gp_walk_info& gwi, bool& nonS funcName = "case_searched"; funcParms.reserve(item->argument_count()); - if (gwi.clauseType == SELECT || gwi.clauseType == HAVING || gwi.clauseType == GROUP_BY) // select clause + // so buildXXXcolumn function will not pop stack. + ClauseType realClauseType = gwi.clauseType; + gwi.clauseType = SELECT; + + // We ought to be able to just build from the stack, and would + // be able to if there were any way to know which stack had the + // next case item. Unfortunately, parameters may have been pushed + // onto the ptWorkStack or rcWorkStack or neither, depending on type + // and position. We can't tell which at this point, so we + // rebuild the item from the arguments directly and then try to + // figure what to pop, if anything, in order to sync the stacks. + for (int32_t i = item->argument_count()-1; i >=0; i--) { - // the first argument - if (funcName == "case_searched") + // For case_searched, we know the items for the WHEN clause will + // not be ReturnedColumns. We do this separately just to save + // some cpu cycles trying to build a ReturnedColumn as below. + // Every even numbered arg is a WHEN. In between are the THEN. + // An odd number of args indicates an ELSE residing in the last spot. + if (funcName == "case_searched" && + i % 2 == 0 && uint(i) != item->argument_count()-1) { - for (uint32_t i = 0; i < item->argument_count(); i++) + sptp.reset(buildParseTree((Item_func*)(item->arguments()[i]), gwi, nonSupport)); + if (!gwi.ptWorkStack.empty() && *gwi.ptWorkStack.top()->data() == sptp->data()) { - if (i % 2 == 0 && i != 1 && i != item->argument_count()-1) - { - sptp.reset(buildParseTree((Item_func*)(item->arguments()[i]), gwi, nonSupport)); - funcParms.push_back(sptp); - } - else - { - ReturnedColumn* parm = buildReturnedColumn(item->arguments()[i], gwi, nonSupport); - if (parm) - { - sptp.reset(new ParseTree(parm)); - } - else - { - sptp.reset(buildParseTree((Item_func*)(item->arguments()[i]), gwi, nonSupport)); - } - funcParms.push_back(sptp); - } + gwi.ptWorkStack.pop(); } } else { - for (uint32_t i = 0; i < item->argument_count(); i++) + // First try building a ReturnedColumn. It may or may not succeed + // depending on the types involved. There's also little correlation + // between buildReturnedColumn and the existance of the item on + // rwWorkStack or ptWorkStack. + // For example, simple predicates, such as 1=1 or 1=0, land in the + // ptWorkStack but other stuff might land in the rwWorkStack + ReturnedColumn* parm = buildReturnedColumn(item->arguments()[i], gwi, nonSupport); + if (parm) { - ReturnedColumn* parm = buildReturnedColumn(item->arguments()[i], gwi, nonSupport); - if (parm) + sptp.reset(new ParseTree(parm)); + // We need to pop whichever stack is holding it, if any. + if ((!gwi.rcWorkStack.empty()) && + *gwi.rcWorkStack.top() == parm) { - sptp.reset(new ParseTree(parm)); + gwi.rcWorkStack.pop(); } else + if (!gwi.ptWorkStack.empty()) { - sptp.reset(buildParseTree((Item_func*)(item->arguments()[i]), gwi, nonSupport)); - } - funcParms.push_back(sptp); - } - } - } - else // where clause - { - // so buildXXXcolumn function will not pop stack. - gwi.clauseType = SELECT; - if (funcName == "case_searched") - { - for (int32_t i = item->argument_count()-1; i >=0; i--) - { - if (i % 2 == 0 && uint(i) != item->argument_count()-1) - { - // build item from arguments to avoid parm sequence complexity - sptp.reset(buildParseTree((Item_func*)(item->arguments()[i]), gwi, nonSupport)); - if (!gwi.ptWorkStack.empty()) + ReturnedColumn* ptrc = dynamic_cast(gwi.ptWorkStack.top()->data()); + if (ptrc && *ptrc == *parm) gwi.ptWorkStack.pop(); } - else - { - ReturnedColumn* parm = buildReturnedColumn(item->arguments()[i], gwi, nonSupport); - if (parm) - { - sptp.reset(new ParseTree(parm)); - if (!gwi.rcWorkStack.empty()) - gwi.rcWorkStack.pop(); - } - else - { - sptp.reset(buildParseTree((Item_func*)(item->arguments()[i]), gwi, nonSupport)); - if (!gwi.ptWorkStack.empty()) - gwi.ptWorkStack.pop(); - } - } - funcParms.insert(funcParms.begin(), sptp); } - } - else // simple_case - { - for (uint32_t i = 0; i < item->argument_count(); i++) + else { - ReturnedColumn* parm = buildReturnedColumn(item->arguments()[i], gwi, nonSupport); - if (parm) + sptp.reset(buildParseTree((Item_func*)(item->arguments()[i]), gwi, nonSupport)); + // We need to pop whichever stack is holding it, if any. + if ((!gwi.ptWorkStack.empty()) && + *gwi.ptWorkStack.top()->data() == sptp->data()) { - sptp.reset(new ParseTree(parm)); - if (!gwi.rcWorkStack.empty()) + gwi.ptWorkStack.pop(); + } + else + if (!gwi.rcWorkStack.empty()) + { + // Probably won't happen, but it might have been on the + // rcWorkStack all along. + ReturnedColumn* ptrc = dynamic_cast(sptp->data()); + if (ptrc && *ptrc == *gwi.rcWorkStack.top()) + { gwi.rcWorkStack.pop(); + } } - else - { - sptp.reset(buildParseTree((Item_func*)(item->arguments()[i]), gwi, nonSupport)); - if (!gwi.ptWorkStack.empty()) - gwi.ptWorkStack.pop(); - } - funcParms.push_back(sptp); } } - // recover clause type - gwi.clauseType = WHERE; + funcParms.insert(funcParms.begin(), sptp); } + // recover clause type + gwi.clauseType = realClauseType; if (gwi.fatalParseError) { @@ -4259,9 +4237,9 @@ void gp_walk(const Item *item, void *arg) } // bug 3137. If filter constant like 1=0, put it to ptWorkStack // MariaDB bug 750. Breaks if compare is an argument to a function. - if ((int32_t)gwip->rcWorkStack.size() <= (gwip->rcBookMarkStack.empty() ? 0 : gwip->rcBookMarkStack.top()) - && isPredicateFunction(ifp, gwip)) -// if (isPredicateFunction(ifp, gwip)) +// if ((int32_t)gwip->rcWorkStack.size() <= (gwip->rcBookMarkStack.empty() ? 0 : gwip->rcBookMarkStack.top()) +// && isPredicateFunction(ifp, gwip)) + if (isPredicateFunction(ifp, gwip)) gwip->ptWorkStack.push(new ParseTree(cc)); else gwip->rcWorkStack.push(cc); @@ -5505,7 +5483,9 @@ int getSelectPlan(gp_walk_info& gwi, SELECT_LEX& select_lex, SCSEP& csep, bool i // @bug 1706 String funcStr; ifp->print(&funcStr, QT_INFINIDB); - gwi.selectCols.push_back(string(funcStr.c_ptr()) + " `" + escapeBackTick(ifp->name) + "`"); + string valStr; + valStr.assign(funcStr.ptr(), funcStr.length()); + gwi.selectCols.push_back(valStr + " `" + escapeBackTick(ifp->name) + "`"); // clear the error set by buildFunctionColumn gwi.fatalParseError = false; gwi.parseErrorText = "";