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");