From ff43adee778f771a80db1d4db8a90ef8c3e4d33f Mon Sep 17 00:00:00 2001 From: Xiaoyu Wang Date: Thu, 19 May 2022 18:09:47 +0800 Subject: [PATCH 1/4] fix: some problems of parser and planner --- include/libs/nodes/querynodes.h | 2 ++ include/util/taoserror.h | 1 + source/libs/parser/src/parTranslater.c | 25 ++++++++++++++++++++++++- source/libs/parser/src/parUtil.c | 3 +++ source/libs/planner/src/planSpliter.c | 6 +++--- 5 files changed, 33 insertions(+), 4 deletions(-) diff --git a/include/libs/nodes/querynodes.h b/include/libs/nodes/querynodes.h index ee986d5aab..44a034aa7b 100644 --- a/include/libs/nodes/querynodes.h +++ b/include/libs/nodes/querynodes.h @@ -236,6 +236,8 @@ typedef struct SSelectStmt { bool isTimeOrderQuery; bool hasAggFuncs; bool hasRepeatScanFuncs; + bool hasNonstdSQLFunc; + bool hasProjCol; } SSelectStmt; typedef enum ESetOperatorType { SET_OP_TYPE_UNION_ALL = 1, SET_OP_TYPE_UNION } ESetOperatorType; diff --git a/include/util/taoserror.h b/include/util/taoserror.h index fd3e008e67..5c251e7a27 100644 --- a/include/util/taoserror.h +++ b/include/util/taoserror.h @@ -649,6 +649,7 @@ int32_t* taosGetErrno(); #define TSDB_CODE_PAR_INVALID_TBNAME TAOS_DEF_ERROR_CODE(0, 0x264C) #define TSDB_CODE_PAR_INVALID_FUNCTION_NAME TAOS_DEF_ERROR_CODE(0, 0x264D) #define TSDB_CODE_PAR_COMMENT_TOO_LONG TAOS_DEF_ERROR_CODE(0, 0x264E) +#define TSDB_CODE_PAR_NOT_ALLOWED_FUNC TAOS_DEF_ERROR_CODE(0, 0x264F) //planner #define TSDB_CODE_PLAN_INTERNAL_ERROR TAOS_DEF_ERROR_CODE(0, 0x2700) diff --git a/source/libs/parser/src/parTranslater.c b/source/libs/parser/src/parTranslater.c index 074a5bc30c..e6eda57cab 100644 --- a/source/libs/parser/src/parTranslater.c +++ b/source/libs/parser/src/parTranslater.c @@ -53,6 +53,8 @@ static bool afterGroupBy(ESqlClause clause) { return clause > SQL_CLAUSE_GROUP_B static bool beforeHaving(ESqlClause clause) { return clause < SQL_CLAUSE_HAVING; } +static bool afterHaving(ESqlClause clause) { return clause > SQL_CLAUSE_HAVING; } + static int32_t addNamespace(STranslateContext* pCxt, void* pTable) { size_t currTotalLevel = taosArrayGetSize(pCxt->pNsLevel); if (currTotalLevel > pCxt->currLevel) { @@ -276,6 +278,10 @@ static bool isScanPseudoColumnFunc(const SNode* pNode) { return (QUERY_NODE_FUNCTION == nodeType(pNode) && fmIsScanPseudoColumnFunc(((SFunctionNode*)pNode)->funcId)); } +static bool isNonstandardSQLFunc(const SNode* pNode) { + return (QUERY_NODE_FUNCTION == nodeType(pNode) && fmIsNonstandardSQLFunc(((SFunctionNode*)pNode)->funcId)); +} + static bool isDistinctOrderBy(STranslateContext* pCxt) { return (SQL_CLAUSE_ORDER_BY == pCxt->currClause && pCxt->pCurrStmt->isDistinct); } @@ -433,6 +439,7 @@ static EDealRes translateColumnWithoutPrefix(STranslateContext* pCxt, SColumnNod SArray* pTables = taosArrayGetP(pCxt->pNsLevel, pCxt->currLevel); size_t nums = taosArrayGetSize(pTables); bool found = false; + bool isInternalPk = isInternalPrimaryKey(pCol); for (size_t i = 0; i < nums; ++i) { STableNode* pTable = taosArrayGetP(pTables, i); if (findAndSetColumn(pCol, pTable)) { @@ -440,10 +447,13 @@ static EDealRes translateColumnWithoutPrefix(STranslateContext* pCxt, SColumnNod return generateDealNodeErrMsg(pCxt, TSDB_CODE_PAR_AMBIGUOUS_COLUMN, pCol->colName); } found = true; + if (isInternalPk) { + break; + } } } if (!found) { - if (isInternalPrimaryKey(pCol)) { + if (isInternalPk) { return generateDealNodeErrMsg(pCxt, TSDB_CODE_PAR_INVALID_INTERNAL_PK); } else { return generateDealNodeErrMsg(pCxt, TSDB_CODE_PAR_INVALID_COLUMN, pCol->colName); @@ -481,6 +491,9 @@ static EDealRes translateColumn(STranslateContext* pCxt, SColumnNode* pCol) { } res = (found ? DEAL_RES_CONTINUE : translateColumnWithoutPrefix(pCxt, pCol)); } + if (afterHaving(pCxt->currClause)) { + pCxt->pCurrStmt->hasProjCol = true; + } return res; } @@ -783,6 +796,16 @@ static EDealRes translateFunction(STranslateContext* pCxt, SFunctionNode* pFunc) return generateDealNodeErrMsg(pCxt, TSDB_CODE_PAR_INVALID_TBNAME); } } + if (afterHaving(pCxt->currClause)) { + pCxt->pCurrStmt->hasProjCol = true; + } + } + if (TSDB_CODE_SUCCESS == pCxt->errCode && fmIsNonstandardSQLFunc(pFunc->funcId)) { + if (SQL_CLAUSE_SELECT != pCxt->currClause || pCxt->pCurrStmt->hasNonstdSQLFunc || pCxt->pCurrStmt->hasAggFuncs || + pCxt->pCurrStmt->hasProjCol) { + return generateDealNodeErrMsg(pCxt, TSDB_CODE_PAR_NOT_ALLOWED_FUNC, pFunc->functionName); + } + pCxt->pCurrStmt->hasNonstdSQLFunc = true; } return TSDB_CODE_SUCCESS == pCxt->errCode ? DEAL_RES_CONTINUE : DEAL_RES_ERROR; } diff --git a/source/libs/parser/src/parUtil.c b/source/libs/parser/src/parUtil.c index 7b9147beab..d6ecf16521 100644 --- a/source/libs/parser/src/parUtil.c +++ b/source/libs/parser/src/parUtil.c @@ -164,6 +164,9 @@ static char* getSyntaxErrFormat(int32_t errCode) { return "Invalid function name"; case TSDB_CODE_PAR_COMMENT_TOO_LONG: return "Comment too long"; + case TSDB_CODE_PAR_NOT_ALLOWED_FUNC: + return "%s are allowed only in the SELECT list of a query. " + "And, cannot be mixed with other non scalar functions or columns."; case TSDB_CODE_OUT_OF_MEMORY: return "Out of memory"; default: diff --git a/source/libs/planner/src/planSpliter.c b/source/libs/planner/src/planSpliter.c index fb716af3e5..a87c00bea9 100644 --- a/source/libs/planner/src/planSpliter.c +++ b/source/libs/planner/src/planSpliter.c @@ -24,9 +24,9 @@ #define SPLIT_FLAG_TEST_MASK(val, mask) (((val) & (mask)) != 0) typedef struct SSplitContext { - int32_t queryId; - int32_t groupId; - bool split; + uint64_t queryId; + int32_t groupId; + bool split; } SSplitContext; typedef int32_t (*FSplit)(SSplitContext* pCxt, SLogicSubplan* pSubplan); From d2da0c0c7f3721df4c85562b6f23d7aeb61d665d Mon Sep 17 00:00:00 2001 From: Xiaoyu Wang Date: Thu, 19 May 2022 19:19:30 +0800 Subject: [PATCH 2/4] fix: some problems of parser and planner --- include/libs/nodes/querynodes.h | 1 - source/libs/parser/src/parTranslater.c | 46 +++++++++++++++-------- source/libs/parser/src/parUtil.c | 2 +- source/libs/parser/test/parSelectTest.cpp | 20 ++++++++++ 4 files changed, 52 insertions(+), 17 deletions(-) diff --git a/include/libs/nodes/querynodes.h b/include/libs/nodes/querynodes.h index 44a034aa7b..b08e0aff3d 100644 --- a/include/libs/nodes/querynodes.h +++ b/include/libs/nodes/querynodes.h @@ -237,7 +237,6 @@ typedef struct SSelectStmt { bool hasAggFuncs; bool hasRepeatScanFuncs; bool hasNonstdSQLFunc; - bool hasProjCol; } SSelectStmt; typedef enum ESetOperatorType { SET_OP_TYPE_UNION_ALL = 1, SET_OP_TYPE_UNION } ESetOperatorType; diff --git a/source/libs/parser/src/parTranslater.c b/source/libs/parser/src/parTranslater.c index e6eda57cab..cd1c91f84c 100644 --- a/source/libs/parser/src/parTranslater.c +++ b/source/libs/parser/src/parTranslater.c @@ -491,9 +491,6 @@ static EDealRes translateColumn(STranslateContext* pCxt, SColumnNode* pCol) { } res = (found ? DEAL_RES_CONTINUE : translateColumnWithoutPrefix(pCxt, pCol)); } - if (afterHaving(pCxt->currClause)) { - pCxt->pCurrStmt->hasProjCol = true; - } return res; } @@ -716,10 +713,13 @@ static EDealRes translateOperator(STranslateContext* pCxt, SOperatorNode* pOp) { return DEAL_RES_CONTINUE; } -static EDealRes haveAggFunction(SNode* pNode, void* pContext) { +static EDealRes haveAggOrNonstdFunction(SNode* pNode, void* pContext) { if (isAggFunc(pNode)) { *((bool*)pContext) = true; return DEAL_RES_END; + } else if (isNonstandardSQLFunc(pNode)) { + *((bool*)pContext) = true; + return DEAL_RES_END; } return DEAL_RES_CONTINUE; } @@ -756,6 +756,12 @@ static int32_t rewriteCountStar(STranslateContext* pCxt, SFunctionNode* pCount) return code; } +static bool hasInvalidFuncNesting(SNodeList* pParameterList) { + bool hasInvalidFunc = false; + nodesWalkExprs(pParameterList, haveAggOrNonstdFunction, &hasInvalidFunc); + return hasInvalidFunc; +} + static EDealRes translateFunction(STranslateContext* pCxt, SFunctionNode* pFunc) { SFmGetFuncInfoParam param = {.pCtg = pCxt->pParseCxt->pCatalog, .pRpc = pCxt->pParseCxt->pTransporter, @@ -767,11 +773,12 @@ static EDealRes translateFunction(STranslateContext* pCxt, SFunctionNode* pFunc) if (beforeHaving(pCxt->currClause)) { return generateDealNodeErrMsg(pCxt, TSDB_CODE_PAR_ILLEGAL_USE_AGG_FUNCTION); } - bool haveAggFunc = false; - nodesWalkExprs(pFunc->pParameterList, haveAggFunction, &haveAggFunc); - if (haveAggFunc) { + if (hasInvalidFuncNesting(pFunc->pParameterList)) { return generateDealNodeErrMsg(pCxt, TSDB_CODE_PAR_AGG_FUNC_NESTING); } + if (pCxt->pCurrStmt->hasNonstdSQLFunc) { + return generateDealNodeErrMsg(pCxt, TSDB_CODE_PAR_NOT_ALLOWED_FUNC); + } pCxt->pCurrStmt->hasAggFuncs = true; pCxt->pCurrStmt->isTimeOrderQuery = false; @@ -796,14 +803,13 @@ static EDealRes translateFunction(STranslateContext* pCxt, SFunctionNode* pFunc) return generateDealNodeErrMsg(pCxt, TSDB_CODE_PAR_INVALID_TBNAME); } } - if (afterHaving(pCxt->currClause)) { - pCxt->pCurrStmt->hasProjCol = true; - } } if (TSDB_CODE_SUCCESS == pCxt->errCode && fmIsNonstandardSQLFunc(pFunc->funcId)) { - if (SQL_CLAUSE_SELECT != pCxt->currClause || pCxt->pCurrStmt->hasNonstdSQLFunc || pCxt->pCurrStmt->hasAggFuncs || - pCxt->pCurrStmt->hasProjCol) { - return generateDealNodeErrMsg(pCxt, TSDB_CODE_PAR_NOT_ALLOWED_FUNC, pFunc->functionName); + if (SQL_CLAUSE_SELECT != pCxt->currClause || pCxt->pCurrStmt->hasNonstdSQLFunc || pCxt->pCurrStmt->hasAggFuncs) { + return generateDealNodeErrMsg(pCxt, TSDB_CODE_PAR_NOT_ALLOWED_FUNC); + } + if (hasInvalidFuncNesting(pFunc->pParameterList)) { + return generateDealNodeErrMsg(pCxt, TSDB_CODE_PAR_AGG_FUNC_NESTING); } pCxt->pCurrStmt->hasNonstdSQLFunc = true; } @@ -975,6 +981,7 @@ typedef struct CheckAggColCoexistCxt { STranslateContext* pTranslateCxt; bool existAggFunc; bool existCol; + bool existNonstdFunc; int32_t selectFuncNum; } CheckAggColCoexistCxt; @@ -985,6 +992,10 @@ static EDealRes doCheckAggColCoexist(SNode* pNode, void* pContext) { pCxt->existAggFunc = true; return DEAL_RES_IGNORE_CHILD; } + if (isNonstandardSQLFunc(pNode)) { + pCxt->existNonstdFunc = true; + return DEAL_RES_IGNORE_CHILD; + } if (isScanPseudoColumnFunc(pNode) || QUERY_NODE_COLUMN == nodeType(pNode)) { pCxt->existCol = true; } @@ -995,16 +1006,21 @@ static int32_t checkAggColCoexist(STranslateContext* pCxt, SSelectStmt* pSelect) if (NULL != pSelect->pGroupByList) { return TSDB_CODE_SUCCESS; } - CheckAggColCoexistCxt cxt = {.pTranslateCxt = pCxt, .existAggFunc = false, .existCol = false}; + CheckAggColCoexistCxt cxt = { + .pTranslateCxt = pCxt, .existAggFunc = false, .existCol = false, .existNonstdFunc = false}; nodesWalkExprs(pSelect->pProjectionList, doCheckAggColCoexist, &cxt); if (!pSelect->isDistinct) { nodesWalkExprs(pSelect->pOrderByList, doCheckAggColCoexist, &cxt); } if (1 == cxt.selectFuncNum) { return rewriteColsToSelectValFunc(pCxt, pSelect); - } else if ((cxt.selectFuncNum > 1 || cxt.existAggFunc || NULL != pSelect->pWindow) && cxt.existCol) { + } + if ((cxt.selectFuncNum > 1 || cxt.existAggFunc || NULL != pSelect->pWindow) && cxt.existCol) { return generateSyntaxErrMsg(&pCxt->msgBuf, TSDB_CODE_PAR_NOT_SINGLE_GROUP); } + if (cxt.existNonstdFunc && cxt.existCol) { + return generateSyntaxErrMsg(&pCxt->msgBuf, TSDB_CODE_PAR_NOT_ALLOWED_FUNC); + } return TSDB_CODE_SUCCESS; } diff --git a/source/libs/parser/src/parUtil.c b/source/libs/parser/src/parUtil.c index d6ecf16521..11884bc10d 100644 --- a/source/libs/parser/src/parUtil.c +++ b/source/libs/parser/src/parUtil.c @@ -165,7 +165,7 @@ static char* getSyntaxErrFormat(int32_t errCode) { case TSDB_CODE_PAR_COMMENT_TOO_LONG: return "Comment too long"; case TSDB_CODE_PAR_NOT_ALLOWED_FUNC: - return "%s are allowed only in the SELECT list of a query. " + return "Some functions are allowed only in the SELECT list of a query. " "And, cannot be mixed with other non scalar functions or columns."; case TSDB_CODE_OUT_OF_MEMORY: return "Out of memory"; diff --git a/source/libs/parser/test/parSelectTest.cpp b/source/libs/parser/test/parSelectTest.cpp index 2da50babd6..ca72d8e8b6 100644 --- a/source/libs/parser/test/parSelectTest.cpp +++ b/source/libs/parser/test/parSelectTest.cpp @@ -121,6 +121,26 @@ TEST_F(ParserSelectTest, selectFunc) { run("SELECT MAX(c1), c2 FROM t1 STATE_WINDOW(c3)"); } +TEST_F(ParserSelectTest, nonstdFunc) { + useDb("root", "test"); + + run("SELECT DIFF(c1) FROM t1"); + + // run("SELECT DIFF(c1) FROM t1 INTERVAL(10s)"); +} + +TEST_F(ParserSelectTest, nonstdFuncSemanticCheck) { + useDb("root", "test"); + + run("SELECT DIFF(c1), c2 FROM t1", TSDB_CODE_PAR_NOT_ALLOWED_FUNC, PARSER_STAGE_TRANSLATE); + + run("SELECT DIFF(c1), tbname FROM t1", TSDB_CODE_PAR_NOT_ALLOWED_FUNC, PARSER_STAGE_TRANSLATE); + + run("SELECT DIFF(c1), count(*) FROM t1", TSDB_CODE_PAR_NOT_ALLOWED_FUNC, PARSER_STAGE_TRANSLATE); + + run("SELECT DIFF(c1), CSUM(c1) FROM t1", TSDB_CODE_PAR_NOT_ALLOWED_FUNC, PARSER_STAGE_TRANSLATE); +} + TEST_F(ParserSelectTest, clause) { useDb("root", "test"); From beb181337ccd17dcec52be1aab4de390d39aba77 Mon Sep 17 00:00:00 2001 From: Xiaoyu Wang Date: Thu, 19 May 2022 20:07:20 +0800 Subject: [PATCH 3/4] fix: some problems of parser and planner --- tests/system-test/0-others/udfTest.py | 16 ++++++++-------- tests/system-test/2-query/nestedQuery.py | 2 +- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/tests/system-test/0-others/udfTest.py b/tests/system-test/0-others/udfTest.py index 0a998aee2b..0e67d3ed89 100644 --- a/tests/system-test/0-others/udfTest.py +++ b/tests/system-test/0-others/udfTest.py @@ -329,14 +329,14 @@ class TDTestCase: # # bug need fix - tdSql.query("select udf1(num1) , csum(num1) from tb;") - tdSql.checkRows(9) - tdSql.query("select ceil(num1) , csum(num1) from tb;") - tdSql.checkRows(9) - tdSql.query("select udf1(c1) , csum(c1) from stb1;") - tdSql.checkRows(22) - tdSql.query("select floor(c1) , csum(c1) from stb1;") - tdSql.checkRows(22) + #tdSql.query("select udf1(num1) , csum(num1) from tb;") + #tdSql.checkRows(9) + #tdSql.query("select ceil(num1) , csum(num1) from tb;") + #tdSql.checkRows(9) + #tdSql.query("select udf1(c1) , csum(c1) from stb1;") + #tdSql.checkRows(22) + #tdSql.query("select floor(c1) , csum(c1) from stb1;") + #tdSql.checkRows(22) # stable with compute functions tdSql.query("select udf1(c1) , abs(c1) from stb1;") diff --git a/tests/system-test/2-query/nestedQuery.py b/tests/system-test/2-query/nestedQuery.py index 1f1766f8e5..11f156c7a4 100755 --- a/tests/system-test/2-query/nestedQuery.py +++ b/tests/system-test/2-query/nestedQuery.py @@ -736,7 +736,7 @@ class TDTestCase: sql += ")" tdLog.info(sql) tdLog.info(len(sql)) - tdSql.error(sql) + #tdSql.error(sql) #TD-15610 tdSql.query(sql) # tdSql.checkRows(100) From d8cd3f02862b4b49996f948e9858ba07e2d7ecbb Mon Sep 17 00:00:00 2001 From: Xiaoyu Wang Date: Thu, 19 May 2022 20:30:56 +0800 Subject: [PATCH 4/4] fix: some problems of parser and planner --- tests/system-test/0-others/udfTest.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/system-test/0-others/udfTest.py b/tests/system-test/0-others/udfTest.py index 0e069d0c38..af3245df3d 100644 --- a/tests/system-test/0-others/udfTest.py +++ b/tests/system-test/0-others/udfTest.py @@ -477,15 +477,15 @@ class TDTestCase: "select c1 , udf1(c1) ,c2 ,udf1(c2), c3 ,udf1(c3), c4 ,udf1(c4) from stb1 order by c1" , "select udf1(num1) , max(num1) from tb;" , "select udf1(num1) , min(num1) from tb;" , - "select udf1(num1) , top(num1,1) from tb;" , - "select udf1(num1) , bottom(num1,1) from tb;" , + #"select udf1(num1) , top(num1,1) from tb;" , + #"select udf1(num1) , bottom(num1,1) from tb;" , "select udf1(c1) , max(c1) from stb1;" , "select udf1(c1) , min(c1) from stb1;" , - "select udf1(c1) , top(c1 ,1) from stb1;" , - "select udf1(c1) , bottom(c1,1) from stb1;" , + #"select udf1(c1) , top(c1 ,1) from stb1;" , + #"select udf1(c1) , bottom(c1,1) from stb1;" , "select udf1(num1) , abs(num1) from tb;" , - "select udf1(num1) , csum(num1) from tb;" , - "select udf1(c1) , csum(c1) from stb1;" , + #"select udf1(num1) , csum(num1) from tb;" , + #"select udf1(c1) , csum(c1) from stb1;" , "select udf1(c1) , abs(c1) from stb1;" , "select abs(udf1(c1)) , abs(ceil(c1)) from stb1 order by ts;" , "select abs(udf1(c1)) , abs(ceil(c1)) from ct1 order by ts;" ,