From a2f919fdd9ad9602ece3ad91bc37faa5f863f8ba Mon Sep 17 00:00:00 2001 From: Andrew Hutchings Date: Tue, 25 Apr 2017 13:09:19 +0100 Subject: [PATCH] MCOL-677 Fix incompatible join detection If two tables have multiple joins and one of them was compatible then the incompatible join detection would fail. This patch moves the incompatible join detection so that every join is checked. It also removes the incompatible join detection from expressionstep as this is redundant and was causing some valid quries to fail. --- dbcon/joblist/expressionstep.cpp | 12 +-- dbcon/joblist/jlf_tuplejoblist.cpp | 131 +++++++++++++++-------------- 2 files changed, 69 insertions(+), 74 deletions(-) diff --git a/dbcon/joblist/expressionstep.cpp b/dbcon/joblist/expressionstep.cpp index 094ec321a..0a9453790 100644 --- a/dbcon/joblist/expressionstep.cpp +++ b/dbcon/joblist/expressionstep.cpp @@ -559,18 +559,8 @@ void ExpressionStep::functionJoinCheck(SimpleFilter* sf, JobInfo& jobInfo) fFunctionJoinInfo.reset(new FunctionJoinInfo); if ((parseFuncJoinColumn(sf->lhs(), jobInfo) == false) || (parseFuncJoinColumn(sf->rhs(), jobInfo) == false) || - (fFunctionJoinInfo->fTableKey[0] == fFunctionJoinInfo->fTableKey[1]) || - (!compatibleColumnTypes(sf->lhs()->resultType(), sf->rhs()->resultType(), true))) + (fFunctionJoinInfo->fTableKey[0] == fFunctionJoinInfo->fTableKey[1])) { - // for better error message - if (fFunctionJoinInfo->fTableKey.size() == 2) - { - uint32_t t1 = fFunctionJoinInfo->fTableKey[0]; - uint32_t t2 = fFunctionJoinInfo->fTableKey[1]; - jobInfo.incompatibleJoinMap[t1] = t2; - jobInfo.incompatibleJoinMap[t2] = t1; - } - // not convertible fFunctionJoinInfo.reset(); return; diff --git a/dbcon/joblist/jlf_tuplejoblist.cpp b/dbcon/joblist/jlf_tuplejoblist.cpp index 9a485052e..e018a3e8e 100644 --- a/dbcon/joblist/jlf_tuplejoblist.cpp +++ b/dbcon/joblist/jlf_tuplejoblist.cpp @@ -1578,6 +1578,70 @@ void spanningTreeCheck(TableInfoMap& tableInfoMap, JobStepVector joinSteps, JobI cout << endl; } + // Check that join is compatible + set views1; + set tables1; + string errStr; + + vector::iterator k = joinedTables.begin(); + + k = joinedTables.begin(); + for (; k != joinedTables.end(); k++) + { + if (jobInfo.keyInfo->tupleKeyVec[*k].fView.empty()) + tables1.insert(jobInfo.keyInfo->tupleKeyToName[*k]); + else + views1.insert(jobInfo.keyInfo->tupleKeyVec[*k].fView); + + if (jobInfo.incompatibleJoinMap.find(*k) != jobInfo.incompatibleJoinMap.end()) + { + errcode = ERR_INCOMPATIBLE_JOIN; + + uint32_t key2 = jobInfo.incompatibleJoinMap[*k]; + if (jobInfo.keyInfo->tupleKeyVec[*k].fView.length() > 0) + { + string view2 = jobInfo.keyInfo->tupleKeyVec[key2].fView; + if (jobInfo.keyInfo->tupleKeyVec[*k].fView == view2) + { + // same view + errStr += "Tables in '" + view2 + "' have"; + } + else if (view2.empty()) + { + // view and real table + errStr += "'" + jobInfo.keyInfo->tupleKeyVec[*k].fView + "' and '" + + jobInfo.keyInfo->tupleKeyToName[key2] + "' have"; + } + else + { + // two views + errStr += "'" + jobInfo.keyInfo->tupleKeyVec[*k].fView + "' and '" + + view2 + "' have"; + } + } + else + { + string view2 = jobInfo.keyInfo->tupleKeyVec[key2].fView; + if (view2.empty()) + { + // two real tables + errStr += "'" + jobInfo.keyInfo->tupleKeyToName[*k] + "' and '" + + jobInfo.keyInfo->tupleKeyToName[key2] + "' have"; + } + else + { + // real table and view + errStr += "'" + jobInfo.keyInfo->tupleKeyToName[*k] + "' and '" + + view2 + "' have"; + } + } + args.add(errStr); + spanningTree = false; + break; + } + + } + // 1c. check again if all tables are joined after pulling in function joins. if (joinedTables.size() < tableInfoMap.size()) { @@ -1588,74 +1652,15 @@ void spanningTreeCheck(TableInfoMap& tableInfoMap, JobStepVector joinSteps, JobI notJoinedTables.push_back(i->first); } - vector::iterator k = joinedTables.begin(); - set views1; - set tables1; - for (; k != joinedTables.end(); k++) - { - if (jobInfo.keyInfo->tupleKeyVec[*k].fView.empty()) - tables1.insert(jobInfo.keyInfo->tupleKeyToName[*k]); - else - views1.insert(jobInfo.keyInfo->tupleKeyVec[*k].fView); - } - - k = notJoinedTables.begin(); set views2; set tables2; - string errStr; - for (; k != notJoinedTables.end(); k++) + k = notJoinedTables.begin(); + for (; k != notJoinedTables.end(); k++) { if (jobInfo.keyInfo->tupleKeyVec[*k].fView.empty()) tables2.insert(jobInfo.keyInfo->tupleKeyToName[*k]); else views2.insert(jobInfo.keyInfo->tupleKeyVec[*k].fView); - - if (jobInfo.incompatibleJoinMap.find(*k) != jobInfo.incompatibleJoinMap.end()) - { - errcode = ERR_INCOMPATIBLE_JOIN; - - uint32_t key2 = jobInfo.incompatibleJoinMap[*k]; - if (jobInfo.keyInfo->tupleKeyVec[*k].fView.length() > 0) - { - string view2 = jobInfo.keyInfo->tupleKeyVec[key2].fView; - if (jobInfo.keyInfo->tupleKeyVec[*k].fView == view2) - { - // same view - errStr += "Tables in '" + view2 + "' have"; - } - else if (view2.empty()) - { - // view and real table - errStr += "'" + jobInfo.keyInfo->tupleKeyVec[*k].fView + "' and '" + - jobInfo.keyInfo->tupleKeyToName[key2] + "' have"; - } - else - { - // two views - errStr += "'" + jobInfo.keyInfo->tupleKeyVec[*k].fView + "' and '" + - view2 + "' have"; - } - } - else - { - string view2 = jobInfo.keyInfo->tupleKeyVec[key2].fView; - if (view2.empty()) - { - // two real tables - errStr += "'" + jobInfo.keyInfo->tupleKeyToName[*k] + "' and '" + - jobInfo.keyInfo->tupleKeyToName[key2] + "' have"; - } - else - { - // real table and view - errStr += "'" + jobInfo.keyInfo->tupleKeyToName[*k] + "' and '" + - view2 + "' have"; - } - } - - break; - } - } if (errStr.empty()) @@ -1717,11 +1722,11 @@ void spanningTreeCheck(TableInfoMap& tableInfoMap, JobStepVector joinSteps, JobI } errStr = set1 + "' and " + set2 + "' are"; + args.add(errStr); + spanningTree = false; } } - args.add(errStr); - spanningTree = false; } // 2. no cycles