From 5708d497ce204dc6b64b0a350eca99669fe8979c Mon Sep 17 00:00:00 2001 From: Xiaoyu Wang Date: Mon, 11 Jul 2022 10:07:11 +0800 Subject: [PATCH] fix: syntax issue with '(select ... limit) statement' --- source/libs/function/src/functionMgt.c | 2 +- source/libs/parser/inc/sql.y | 6 +++++- source/libs/parser/src/parAstCreater.c | 9 +++++++++ source/libs/parser/src/parTranslater.c | 16 ++++------------ source/libs/parser/src/sql.c | 9 +++++---- source/libs/parser/test/parSelectTest.cpp | 16 ++++++++++++---- 6 files changed, 36 insertions(+), 22 deletions(-) diff --git a/source/libs/function/src/functionMgt.c b/source/libs/function/src/functionMgt.c index 1da9d4a70f..ed82654f77 100644 --- a/source/libs/function/src/functionMgt.c +++ b/source/libs/function/src/functionMgt.c @@ -139,7 +139,7 @@ bool fmIsAggFunc(int32_t funcId) { return isSpecificClassifyFunc(funcId, FUNC_MG bool fmIsScalarFunc(int32_t funcId) { return isSpecificClassifyFunc(funcId, FUNC_MGT_SCALAR_FUNC); } -bool fmIsVectorFunc(int32_t funcId) { return !fmIsScalarFunc(funcId) && !fmIsScanPseudoColumnFunc(funcId); } +bool fmIsVectorFunc(int32_t funcId) { return !fmIsScalarFunc(funcId) && !fmIsPseudoColumnFunc(funcId); } bool fmIsSelectFunc(int32_t funcId) { return isSpecificClassifyFunc(funcId, FUNC_MGT_SELECT_FUNC); } diff --git a/source/libs/parser/inc/sql.y b/source/libs/parser/inc/sql.y index 4bcfcbf5c0..4758f3ae31 100644 --- a/source/libs/parser/inc/sql.y +++ b/source/libs/parser/inc/sql.y @@ -934,7 +934,11 @@ query_expression_body(A) ::= query_primary(A) ::= query_specification(B). { A = B; } query_primary(A) ::= NK_LP query_expression_body(B) - order_by_clause_opt slimit_clause_opt limit_clause_opt NK_RP. { A = B; } + order_by_clause_opt(C) slimit_clause_opt(D) limit_clause_opt(E) NK_RP. { + A = addOrderByClause(pCxt, B, C); + A = addSlimitClause(pCxt, A, D); + A = addLimitClause(pCxt, A, E); + } %type order_by_clause_opt { SNodeList* } %destructor order_by_clause_opt { nodesDestroyList($$); } diff --git a/source/libs/parser/src/parAstCreater.c b/source/libs/parser/src/parAstCreater.c index ab73204fb7..5521d1c382 100644 --- a/source/libs/parser/src/parAstCreater.c +++ b/source/libs/parser/src/parAstCreater.c @@ -665,6 +665,9 @@ SNode* addHavingClause(SAstCreateContext* pCxt, SNode* pStmt, SNode* pHaving) { SNode* addOrderByClause(SAstCreateContext* pCxt, SNode* pStmt, SNodeList* pOrderByList) { CHECK_PARSER_STATUS(pCxt); + if (NULL == pOrderByList) { + return pStmt; + } if (QUERY_NODE_SELECT_STMT == nodeType(pStmt)) { ((SSelectStmt*)pStmt)->pOrderByList = pOrderByList; } else { @@ -675,6 +678,9 @@ SNode* addOrderByClause(SAstCreateContext* pCxt, SNode* pStmt, SNodeList* pOrder SNode* addSlimitClause(SAstCreateContext* pCxt, SNode* pStmt, SNode* pSlimit) { CHECK_PARSER_STATUS(pCxt); + if (NULL == pSlimit) { + return pStmt; + } if (QUERY_NODE_SELECT_STMT == nodeType(pStmt)) { ((SSelectStmt*)pStmt)->pSlimit = (SLimitNode*)pSlimit; } @@ -683,6 +689,9 @@ SNode* addSlimitClause(SAstCreateContext* pCxt, SNode* pStmt, SNode* pSlimit) { SNode* addLimitClause(SAstCreateContext* pCxt, SNode* pStmt, SNode* pLimit) { CHECK_PARSER_STATUS(pCxt); + if (NULL == pLimit) { + return pStmt; + } if (QUERY_NODE_SELECT_STMT == nodeType(pStmt)) { ((SSelectStmt*)pStmt)->pLimit = (SLimitNode*)pLimit; } else { diff --git a/source/libs/parser/src/parTranslater.c b/source/libs/parser/src/parTranslater.c index 8e6798dcd2..e682f1abfd 100644 --- a/source/libs/parser/src/parTranslater.c +++ b/source/libs/parser/src/parTranslater.c @@ -1189,7 +1189,7 @@ static void setFuncClassification(SNode* pCurrStmt, SFunctionNode* pFunc) { if (fmIsSelectFunc(pFunc->funcId)) { pSelect->hasSelectFunc = true; ++(pSelect->selectFuncNum); - } else if (fmIsAggFunc(pFunc->funcId) || fmIsIndefiniteRowsFunc(pFunc->funcId)) { + } else if (fmIsVectorFunc(pFunc->funcId)) { pSelect->hasOtherVectorFunc = true; } pSelect->hasUniqueFunc = pSelect->hasUniqueFunc ? true : (FUNCTION_TYPE_UNIQUE == pFunc->funcType); @@ -1514,18 +1514,11 @@ static int32_t rewriteColsToSelectValFunc(STranslateContext* pCxt, SSelectStmt* typedef struct CheckAggColCoexistCxt { STranslateContext* pTranslateCxt; bool existCol; - int32_t selectFuncNum; - bool existOtherVectorFunc; } CheckAggColCoexistCxt; static EDealRes doCheckAggColCoexist(SNode** pNode, void* pContext) { CheckAggColCoexistCxt* pCxt = (CheckAggColCoexistCxt*)pContext; if (isVectorFunc(*pNode)) { - if (isSelectFunc(*pNode)) { - ++(pCxt->selectFuncNum); - } else { - pCxt->existOtherVectorFunc = true; - } return DEAL_RES_IGNORE_CHILD; } SNode* pPartKey = NULL; @@ -1542,16 +1535,15 @@ static EDealRes doCheckAggColCoexist(SNode** pNode, void* pContext) { static int32_t checkAggColCoexist(STranslateContext* pCxt, SSelectStmt* pSelect) { if (NULL != pSelect->pGroupByList || NULL != pSelect->pWindow || - (!pSelect->hasAggFuncs && !pSelect->hasIndefiniteRowsFunc)) { + (!pSelect->hasAggFuncs && !pSelect->hasIndefiniteRowsFunc && !pSelect->hasInterpFunc)) { return TSDB_CODE_SUCCESS; } - CheckAggColCoexistCxt cxt = { - .pTranslateCxt = pCxt, .existCol = false, .selectFuncNum = 0, .existOtherVectorFunc = false}; + CheckAggColCoexistCxt cxt = {.pTranslateCxt = pCxt, .existCol = false}; nodesRewriteExprs(pSelect->pProjectionList, doCheckAggColCoexist, &cxt); if (!pSelect->isDistinct) { nodesRewriteExprs(pSelect->pOrderByList, doCheckAggColCoexist, &cxt); } - if (1 == cxt.selectFuncNum && !cxt.existOtherVectorFunc) { + if (1 == pSelect->selectFuncNum && !pSelect->hasOtherVectorFunc) { return rewriteColsToSelectValFunc(pCxt, pSelect); } if (cxt.existCol) { diff --git a/source/libs/parser/src/sql.c b/source/libs/parser/src/sql.c index 029dae0311..ec67ad9efd 100644 --- a/source/libs/parser/src/sql.c +++ b/source/libs/parser/src/sql.c @@ -4658,10 +4658,11 @@ static YYACTIONTYPE yy_reduce( yymsp[-2].minor.yy652 = yylhsminor.yy652; break; case 465: /* query_primary ::= NK_LP query_expression_body order_by_clause_opt slimit_clause_opt limit_clause_opt NK_RP */ -{ yymsp[-5].minor.yy652 = yymsp[-4].minor.yy652; } - yy_destructor(yypParser,369,&yymsp[-3].minor); - yy_destructor(yypParser,370,&yymsp[-2].minor); - yy_destructor(yypParser,371,&yymsp[-1].minor); +{ + yymsp[-5].minor.yy652 = addOrderByClause(pCxt, yymsp[-4].minor.yy652, yymsp[-3].minor.yy210); + yymsp[-5].minor.yy652 = addSlimitClause(pCxt, yymsp[-5].minor.yy652, yymsp[-2].minor.yy652); + yymsp[-5].minor.yy652 = addLimitClause(pCxt, yymsp[-5].minor.yy652, yymsp[-1].minor.yy652); + } break; case 469: /* slimit_clause_opt ::= SLIMIT NK_INTEGER */ case 473: /* limit_clause_opt ::= LIMIT NK_INTEGER */ yytestcase(yyruleno==473); diff --git a/source/libs/parser/test/parSelectTest.cpp b/source/libs/parser/test/parSelectTest.cpp index c6d74c5305..8ad9feb536 100644 --- a/source/libs/parser/test/parSelectTest.cpp +++ b/source/libs/parser/test/parSelectTest.cpp @@ -245,13 +245,21 @@ TEST_F(ParserSelectTest, orderBy) { TEST_F(ParserSelectTest, distinct) { useDb("root", "test"); - // run("SELECT distinct c1, c2 FROM t1 WHERE c1 > 0 order by c1"); + run("SELECT distinct c1, c2 FROM t1 WHERE c1 > 0 order by c1"); - // run("SELECT distinct c1 + 10, c2 FROM t1 WHERE c1 > 0 order by c1 + 10, c2"); + run("SELECT distinct c1 + 10, c2 FROM t1 WHERE c1 > 0 order by c1 + 10, c2"); - // run("SELECT distinct c1 + 10 cc1, c2 cc2 FROM t1 WHERE c1 > 0 order by cc1, c2"); + run("SELECT distinct c1 + 10 cc1, c2 cc2 FROM t1 WHERE c1 > 0 order by cc1, c2"); - // run("SELECT distinct COUNT(c2) FROM t1 WHERE c1 > 0 GROUP BY c1 order by COUNT(c2)"); + run("SELECT distinct COUNT(c2) FROM t1 WHERE c1 > 0 GROUP BY c1 order by COUNT(c2)"); +} + +TEST_F(ParserSelectTest, limit) { + useDb("root", "test"); + + run("SELECT c1, c2 FROM t1 LIMIT 10"); + + run("(SELECT c1, c2 FROM t1 LIMIT 10)"); } // INTERVAL(interval_val [, interval_offset]) [SLIDING (sliding_val)] [FILL(fill_mod_and_val)]