From 28cd6afcef3a25d8f3fbddbde924f0225ebab42e Mon Sep 17 00:00:00 2001 From: Haojun Liao Date: Tue, 23 Nov 2021 10:18:08 +0800 Subject: [PATCH] [td-10564] Fix bug in parse orderby clause. --- source/libs/function/src/texpr.c | 10 ++ source/libs/parser/src/astGenerator.c | 3 +- source/libs/parser/src/astValidate.c | 109 +++++++++++++++++---- source/libs/parser/test/plannerTest.cpp | 9 +- source/libs/planner/src/planner.c | 121 +++++++----------------- 5 files changed, 142 insertions(+), 110 deletions(-) diff --git a/source/libs/function/src/texpr.c b/source/libs/function/src/texpr.c index 6970b85638..70a7e9973f 100644 --- a/source/libs/function/src/texpr.c +++ b/source/libs/function/src/texpr.c @@ -13,6 +13,7 @@ * along with this program. If not, see . */ +#include "function.h" #include "os.h" #include "exception.h" @@ -550,6 +551,15 @@ tExprNode* exprdup(tExprNode* pNode) { } else if (pNode->nodeType == TEXPR_COL_NODE) { pCloned->pSchema = calloc(1, sizeof(SSchema)); *pCloned->pSchema = *pNode->pSchema; + } else if (pNode->nodeType == TEXPR_FUNCTION_NODE) { + strcpy(pCloned->_function.functionName, pNode->_function.functionName); + + int32_t num = pNode->_function.num; + pCloned->_function.num = num; + pCloned->_function.pChild = calloc(num, POINTER_BYTES); + for(int32_t i = 0; i < num; ++i) { + pCloned->_function.pChild[i] = exprdup(pNode->_function.pChild[i]); + } } pCloned->nodeType = pNode->nodeType; diff --git a/source/libs/parser/src/astGenerator.c b/source/libs/parser/src/astGenerator.c index 53d05c87b3..f328486556 100644 --- a/source/libs/parser/src/astGenerator.c +++ b/source/libs/parser/src/astGenerator.c @@ -59,8 +59,7 @@ SArray *tListItemAppendToken(SArray *pList, SToken *pAliasToken, uint8_t sortOrd if (pAliasToken) { SListItem item; - assert(0); -// taosVariantCreate(&item.pVar, pAliasToken); + taosVariantCreate(&item.pVar, pAliasToken->z, pAliasToken->n, pAliasToken->type); item.sortOrder = sortOrder; taosArrayPush(pList, &item); diff --git a/source/libs/parser/src/astValidate.c b/source/libs/parser/src/astValidate.c index d9fd822b9b..7b6c423fb6 100644 --- a/source/libs/parser/src/astValidate.c +++ b/source/libs/parser/src/astValidate.c @@ -954,22 +954,32 @@ int32_t validateOrderbyNode(SQueryStmtInfo *pQueryInfo, SSqlNode* pSqlNode, SMsg } // handle the first part of order by + bool found = false; for(int32_t i = 0; i < taosArrayGetSize(pSortOrder); ++i) { - SVariant* pVar = taosArrayGet(pSortOrder, i); + SListItem* pItem = taosArrayGet(pSortOrder, i); + + SVariant* pVar = &pItem->pVar; if (pVar->nType == TSDB_DATA_TYPE_BINARY) { - SColumn c = {0}; + SOrder order = {0}; // find the orde column among the result field. for (int32_t j = 0; j < getNumOfFields(&pQueryInfo->fieldsInfo); ++j) { SInternalField* pInfo = taosArrayGet(pQueryInfo->fieldsInfo.internalField, j); SSchema* pSchema = &pInfo->pExpr->base.resSchema; if (strcasecmp(pVar->pz, pSchema->name) == 0) { - setColumn(&c, pTableMetaInfo->pTableMeta->uid, pTableMetaInfo->aliasName, TSDB_COL_TMP, &pSchema); - return TSDB_CODE_SUCCESS; + setColumn(&order.col, pTableMetaInfo->pTableMeta->uid, pTableMetaInfo->aliasName, TSDB_COL_TMP, pSchema); + + order.order = pItem->sortOrder; + taosArrayPush(pQueryInfo->order, &order); + found = true; + break; } } - return buildInvalidOperationMsg(pMsgBuf, "invalid order by column"); + if (!found) { + return buildInvalidOperationMsg(pMsgBuf, "invalid order by column"); + } + } else { // order by [1|2|3] if (pVar->i > getNumOfFields(&pQueryInfo->fieldsInfo)) { return buildInvalidOperationMsg(pMsgBuf, msg4); @@ -980,6 +990,7 @@ int32_t validateOrderbyNode(SQueryStmtInfo *pQueryInfo, SSqlNode* pSqlNode, SMsg SOrder c = {0}; setColumn(&c.col, pTableMetaInfo->pTableMeta->uid, pTableMetaInfo->aliasName, TSDB_COL_TMP, &pExprInfo->base.resSchema); + c.order = pItem->sortOrder; taosArrayPush(pQueryInfo->order, &c); } } @@ -1248,16 +1259,17 @@ int32_t checkForInvalidOrderby(SQueryStmtInfo *pQueryInfo, SSqlNode* pSqlNode, S #endif static int32_t checkFillQueryRange(SQueryStmtInfo* pQueryInfo, SMsgBuf* pMsgBuf) { - const char* msg3 = "start(end) time of time range required or time range too large"; + const char* msg1 = "start(end) time of time range required or time range too large"; if (pQueryInfo->interval.interval == 0) { return TSDB_CODE_SUCCESS; } - bool initialWindows = TSWINDOW_IS_EQUAL(pQueryInfo->window, TSWINDOW_INITIALIZER); - if (initialWindows) { - return buildInvalidOperationMsg(pMsgBuf, msg3); - } + // TODO disable this check temporarily +// bool initialWindows = TSWINDOW_IS_EQUAL(pQueryInfo->window, TSWINDOW_INITIALIZER); +// if (initialWindows) { +// return buildInvalidOperationMsg(pMsgBuf, msg1); +// } int64_t timeRange = ABS(pQueryInfo->window.skey - pQueryInfo->window.ekey); @@ -1267,7 +1279,7 @@ static int32_t checkFillQueryRange(SQueryStmtInfo* pQueryInfo, SMsgBuf* pMsgBuf) // number of result is not greater than 10,000,000 if ((timeRange == 0) || (timeRange / intervalRange) >= MAX_INTERVAL_TIME_WINDOW) { - return buildInvalidOperationMsg(pMsgBuf, msg3); + return buildInvalidOperationMsg(pMsgBuf, msg1); } } @@ -1384,7 +1396,8 @@ int32_t validateFillNode(SQueryStmtInfo *pQueryInfo, SSqlNode* pSqlNode, SMsgBuf return TSDB_CODE_SUCCESS; } -static void exprInfoPushDown(SQueryStmtInfo* pQueryInfo); +static void pushDownAggFuncExprInfo(SQueryStmtInfo* pQueryInfo); +static void addColumnNodeFromLowerLevel(SQueryStmtInfo* pQueryInfo); int32_t validateSqlNode(SSqlNode* pSqlNode, SQueryStmtInfo* pQueryInfo, SMsgBuf* pMsgBuf) { assert(pSqlNode != NULL && (pSqlNode->from == NULL || taosArrayGetSize(pSqlNode->from->list) > 0)); @@ -1575,7 +1588,8 @@ int32_t validateSqlNode(SSqlNode* pSqlNode, SQueryStmtInfo* pQueryInfo, SMsgBuf* } } - exprInfoPushDown(pQueryInfo); + pushDownAggFuncExprInfo(pQueryInfo); +// addColumnNodeFromLowerLevel(pQueryInfo); for(int32_t i = 0; i < 1; ++i) { SArray* functionList = extractFunctionList(pQueryInfo->exprList[i]); @@ -1629,15 +1643,29 @@ static bool isAllAggExpr(SArray* pList) { return true; } -static SExprInfo* doCreateColumnNodeFromAggFunc(SSchema* pSchema); +static bool isAllProjectExpr(SArray *pList) { + assert(pList != NULL); + + for(int32_t i = 0; i < taosArrayGetSize(pList); ++i) { + SExprInfo* p = taosArrayGetP(pList, i); + if (p->pExpr->nodeType == TEXPR_FUNCTION_NODE && !qIsAggregateFunction(p->pExpr->_function.functionName)) { + return false; + } + } -static void exprInfoPushDown(SQueryStmtInfo* pQueryInfo) { + return true; +} + +static SExprInfo* createColumnNodeFromAggFunc(SSchema* pSchema); + +static void pushDownAggFuncExprInfo(SQueryStmtInfo* pQueryInfo) { assert(pQueryInfo != NULL); size_t level = getExprFunctionLevel(pQueryInfo); for(int32_t i = 0; i < level - 1; ++i) { SArray* p = pQueryInfo->exprList[i]; + // If direct lower level expressions are all aggregate function, check if current function can be push down or not SArray* pNext = pQueryInfo->exprList[i + 1]; if (!isAllAggExpr(pNext)) { continue; @@ -1650,8 +1678,8 @@ static void exprInfoPushDown(SQueryStmtInfo* pQueryInfo) { bool canPushDown = true; for (int32_t k = 0; k < taosArrayGetSize(pNext); ++k) { SExprInfo* pNextLevelExpr = taosArrayGetP(pNext, k); + // pExpr depends on the output of the down level, so it can not be push downwards if (pExpr->base.pColumns->info.colId == pNextLevelExpr->base.resSchema.colId) { - // pExpr is dependent on the output of the under layer, so it can not be push downwards canPushDown = false; break; } @@ -1661,8 +1689,8 @@ static void exprInfoPushDown(SQueryStmtInfo* pQueryInfo) { taosArrayInsert(pNext, j, &pExpr); taosArrayRemove(p, j); - // todo add the project function in level of "i" - SExprInfo* pNew = doCreateColumnNodeFromAggFunc(&pExpr->base.resSchema); + // Add the project function of the current level, to output the calculated result + SExprInfo* pNew = createColumnNodeFromAggFunc(&pExpr->base.resSchema); taosArrayInsert(p, j, &pNew); } } @@ -1670,6 +1698,49 @@ static void exprInfoPushDown(SQueryStmtInfo* pQueryInfo) { } } +// todo change the logic plan data +static void addColumnNodeFromLowerLevel(SQueryStmtInfo* pQueryInfo) { + assert(pQueryInfo != NULL); + + size_t level = getExprFunctionLevel(pQueryInfo); + for (int32_t i = 0; i < level - 1; ++i) { + SArray* p = pQueryInfo->exprList[i]; + if (isAllAggExpr(p)) { + continue; + } + + // If direct lower level expressions are all aggregate function, check if current function can be push down or not + SArray* pNext = pQueryInfo->exprList[i + 1]; + if (isAllAggExpr(pNext)) { + continue; + } + + for (int32_t j = 0; j < taosArrayGetSize(pNext); ++j) { + SExprInfo* pExpr = taosArrayGetP(p, j); + + bool exists = false; + for (int32_t k = 0; k < taosArrayGetSize(p); ++k) { + SExprInfo* pNextLevelExpr = taosArrayGetP(pNext, k); + // pExpr depends on the output of the down level, so it can not be push downwards + if (pExpr->base.pColumns->info.colId == pNextLevelExpr->base.resSchema.colId) { + exists = true; + break; + } + } + + if (!exists) { + SExprInfo* pNew = calloc(1, sizeof(SExprInfo)); + pNew->pExpr = exprdup(pExpr->pExpr); + memcpy(&pNew->base, &pExpr->base, sizeof(SSqlExpr)); + + int32_t pos = taosArrayGetSize(p); + // Add the project function of the current level, to output the calculated result + taosArrayInsert(p, pos - 1, &pExpr); + } + } + } +} + int32_t checkForInvalidExpr(SQueryStmtInfo* pQueryInfo, SMsgBuf* pMsgBuf) { assert(pQueryInfo != NULL && pMsgBuf != NULL); @@ -3096,7 +3167,7 @@ static tExprNode* doCreateColumnNode(SQueryStmtInfo* pQueryInfo, SColumnIndex* p return pExpr; } -static SExprInfo* doCreateColumnNodeFromAggFunc(SSchema* pSchema) { +static SExprInfo* createColumnNodeFromAggFunc(SSchema* pSchema) { tExprNode* pExprNode = calloc(1, sizeof(tExprNode)); pExprNode->nodeType = TEXPR_COL_NODE; diff --git a/source/libs/parser/test/plannerTest.cpp b/source/libs/parser/test/plannerTest.cpp index cf8463f245..c86e687664 100644 --- a/source/libs/parser/test/plannerTest.cpp +++ b/source/libs/parser/test/plannerTest.cpp @@ -182,14 +182,17 @@ TEST(testCase, displayPlan) { generateLogicplan("select count(*), first(a), last(b) from `t.1abc` state_window(a)"); generateLogicplan("select count(*), first(a), last(b) from `t.1abc` session(ts, 20s)"); - // order by + group by column + limit offset + fill + // order by + group by column + limit offset generateLogicplan("select top(a, 20) k from `t.1abc` order by k asc limit 3 offset 1"); - // join + // fill + generateLogicplan("select min(a) from `t.1abc` where ts>now and ts= 0; --i) { SArray* p = pQueryInfo->exprList[i]; + size_t num = taosArrayGetSize(p); - size_t num = taosArrayGetSize(p); bool aggregateFunc = false; for(int32_t j = 0; j < num; ++j) { SExprInfo* pExpr = (SExprInfo*)taosArrayGetP(p, 0); @@ -265,10 +266,11 @@ static SQueryPlanNode* doCreateQueryPlanForSingleTableImpl(SQueryStmtInfo* pQuer if (pQueryInfo->fillType != TSDB_FILL_NONE) { SFillEssInfo* pInfo = calloc(1, sizeof(SFillEssInfo)); pInfo->fillType = pQueryInfo->fillType; - pInfo->val = calloc(pNode->numOfExpr, sizeof(int64_t)); + pInfo->val = calloc(pNode->numOfExpr, sizeof(int64_t)); memcpy(pInfo->val, pQueryInfo->fillVal, pNode->numOfExpr); - pNode = createQueryNode(QNODE_FILL, "Fill", &pNode, 1, NULL, 0, pInfo); + SArray* p = pQueryInfo->exprList[0]; // top expression in select clause + pNode = createQueryNode(QNODE_FILL, "Fill", &pNode, 1, p, taosArrayGetSize(p), pInfo); } if (pQueryInfo->order != NULL) { @@ -433,26 +435,11 @@ static int32_t doPrintPlan(char* buf, SQueryPlanNode* pQueryNode, int32_t level, } case QNODE_PROJECT: { - len1 = sprintf(buf + len, "cols: "); + len1 = sprintf(buf + len, "cols:"); assert(len1 > 0); - len += len1; - for (int32_t i = 0; i < pQueryNode->numOfExpr; ++i) { - SExprInfo* pExprInfo = taosArrayGetP(pQueryNode->pExpr, i); - - SSqlExpr* p = &pExprInfo->base; - len1 = sprintf(buf + len, "[%s #%d]", p->resSchema.name, p->resSchema.colId); - assert(len1 > 0); - - len += len1; - - if (i < pQueryNode->numOfExpr - 1) { - len1 = sprintf(buf + len, ", "); - len += len1; - } - } - + len = printExprInfo(buf, pQueryNode, len); len1 = sprintf(buf + len, ")"); len += len1; @@ -463,34 +450,15 @@ static int32_t doPrintPlan(char* buf, SQueryPlanNode* pQueryNode, int32_t level, } case QNODE_AGGREGATE: { - for (int32_t i = 0; i < pQueryNode->numOfExpr; ++i) { - SExprInfo* pExprInfo = taosArrayGetP(pQueryNode->pExpr, i); - - SSqlExpr* pExpr = &pExprInfo->base; - len += sprintf(buf + len, "%s [%s #%d]", pExpr->token, pExpr->resSchema.name, pExpr->resSchema.colId); - if (i < pQueryNode->numOfExpr - 1) { - len1 = sprintf(buf + len, ", "); - len += len1; - } - } - + len = printExprInfo(buf, pQueryNode, len); len1 = sprintf(buf + len, ")\n"); len += len1; + break; } case QNODE_TIMEWINDOW: { - for (int32_t i = 0; i < pQueryNode->numOfExpr; ++i) { - SExprInfo* pExprInfo = taosArrayGetP(pQueryNode->pExpr, i); - - SSqlExpr* pExpr = &pExprInfo->base; - len += sprintf(buf + len, "%s [%s #%d]", pExpr->token, pExpr->resSchema.name, pExpr->resSchema.colId); - if (i < pQueryNode->numOfExpr - 1) { - len1 = sprintf(buf + len, ", "); - len += len1; - } - } - + len = printExprInfo(buf, pQueryNode, len); len1 = sprintf(buf + len, ") "); len += len1; @@ -506,16 +474,7 @@ static int32_t doPrintPlan(char* buf, SQueryPlanNode* pQueryNode, int32_t level, } case QNODE_STATEWINDOW: { - for (int32_t i = 0; i < pQueryNode->numOfExpr; ++i) { - SExprInfo* pExprInfo = taosArrayGetP(pQueryNode->pExpr, i); - SSqlExpr* pExpr = &pExprInfo->base; - len += sprintf(buf + len, "%s [%s #%d]", pExpr->token, pExpr->resSchema.name, pExpr->resSchema.colId); - if (i < pQueryNode->numOfExpr - 1) { - len1 = sprintf(buf + len, ", "); - len += len1; - } - } - + len = printExprInfo(buf, pQueryNode, len); len1 = sprintf(buf + len, ") "); len += len1; @@ -526,15 +485,7 @@ static int32_t doPrintPlan(char* buf, SQueryPlanNode* pQueryNode, int32_t level, } case QNODE_SESSIONWINDOW: { - for (int32_t i = 0; i < pQueryNode->numOfExpr; ++i) { - SExprInfo* pExprInfo = taosArrayGetP(pQueryNode->pExpr, i); - SSqlExpr* pExpr = &pExprInfo->base; - len += sprintf(buf + len, "%s [%s #%d]", pExpr->token, pExpr->resSchema.name, pExpr->resSchema.colId); - if (i < pQueryNode->numOfExpr - 1) { - len1 = sprintf(buf + len, ", "); - len += len1; - } - } + len = printExprInfo(buf, pQueryNode, len); len1 = sprintf(buf + len, ") "); len += len1; @@ -546,18 +497,7 @@ static int32_t doPrintPlan(char* buf, SQueryPlanNode* pQueryNode, int32_t level, } case QNODE_GROUPBY: { - for (int32_t i = 0; i < pQueryNode->numOfExpr; ++i) { - SExprInfo* pExprInfo = taosArrayGetP(pQueryNode->pExpr, i); - - SSqlExpr* pExpr = &pExprInfo->base; - len1 = sprintf(buf + len, "%s [%s #%d]", pExpr->token, pExpr->resSchema.name, pExpr->resSchema.colId); - - len += len1; - if (i < pQueryNode->numOfExpr - 1) { - len1 = sprintf(buf + len, ", "); - len += len1; - } - } + len = printExprInfo(buf, pQueryNode, len); SGroupbyExpr* pGroupbyExpr = pQueryNode->pExtInfo; len1 = sprintf(buf + len, ") groupby_col: "); @@ -611,18 +551,7 @@ static int32_t doPrintPlan(char* buf, SQueryPlanNode* pQueryNode, int32_t level, len1 = sprintf(buf + len, "cols: "); len += len1; - for (int32_t i = 0; i < pQueryNode->numOfExpr; ++i) { - SExprInfo* pExprInfo = taosArrayGetP(pQueryNode->pExpr, i); - SSchema* resSchema = &pExprInfo->base.resSchema; - - len1 = sprintf(buf + len, "[%s #%d]", resSchema->name, resSchema->colId); - len += len1; - - if (i < pQueryNode->numOfExpr - 1) { - len1 = sprintf(buf + len, ", "); - len += len1; - } - } + len = printExprInfo(buf, pQueryNode, len); len1 = sprintf(buf + len, ")\n"); len += len1; @@ -658,6 +587,26 @@ static int32_t doPrintPlan(char* buf, SQueryPlanNode* pQueryNode, int32_t level, return len; } +int32_t printExprInfo(const char* buf, const SQueryPlanNode* pQueryNode, int32_t len) { + int32_t len1 = 0; + + for (int32_t i = 0; i < pQueryNode->numOfExpr; ++i) { + SExprInfo* pExprInfo = taosArrayGetP(pQueryNode->pExpr, i); + + SSqlExpr* pExpr = &pExprInfo->base; + len1 = sprintf(buf + len, "%s [%s #%d]", pExpr->token, pExpr->resSchema.name, pExpr->resSchema.colId); + assert(len1 > 0); + + len += len1; + if (i < pQueryNode->numOfExpr - 1) { + len1 = sprintf(buf + len, ", "); + len += len1; + } + } + + return len; +} + int32_t queryPlanToStringImpl(char* buf, SQueryPlanNode* pQueryNode, int32_t level, int32_t totalLen) { int32_t len = doPrintPlan(buf, pQueryNode, level, totalLen); -- GitLab