From 48bb6c2bbb001a7eb97e5e605bfbde931b661273 Mon Sep 17 00:00:00 2001 From: factosea <285808407@qq.com> Date: Sun, 21 Jan 2024 15:06:46 +0800 Subject: [PATCH 1/4] fix: time pseudo column used illegally --- include/util/taoserror.h | 1 + source/libs/planner/src/planPhysiCreater.c | 40 +++++++++++++++++++++- source/util/src/terror.c | 1 + 3 files changed, 41 insertions(+), 1 deletion(-) diff --git a/include/util/taoserror.h b/include/util/taoserror.h index b5389e60d3..5e27358166 100644 --- a/include/util/taoserror.h +++ b/include/util/taoserror.h @@ -521,6 +521,7 @@ int32_t* taosGetErrno(); #define TSDB_CODE_QRY_INVALID_INPUT TAOS_DEF_ERROR_CODE(0, 0x070F) // #define TSDB_CODE_QRY_INVALID_SCHEMA_VERSION TAOS_DEF_ERROR_CODE(0, 0x0710) // 2.x // #define TSDB_CODE_QRY_RESULT_TOO_LARGE TAOS_DEF_ERROR_CODE(0, 0x0711) // 2.x +#define TSDB_CODE_QRY_INVALID_WINDOW_CONDITION TAOS_DEF_ERROR_CODE(0, 0x0712) #define TSDB_CODE_QRY_SCH_NOT_EXIST TAOS_DEF_ERROR_CODE(0, 0x0720) #define TSDB_CODE_QRY_TASK_NOT_EXIST TAOS_DEF_ERROR_CODE(0, 0x0721) #define TSDB_CODE_QRY_TASK_ALREADY_EXIST TAOS_DEF_ERROR_CODE(0, 0x0722) diff --git a/source/libs/planner/src/planPhysiCreater.c b/source/libs/planner/src/planPhysiCreater.c index 21c637116f..301ba4fceb 100644 --- a/source/libs/planner/src/planPhysiCreater.c +++ b/source/libs/planner/src/planPhysiCreater.c @@ -1722,6 +1722,39 @@ static int32_t createStateWindowPhysiNode(SPhysiPlanContext* pCxt, SNodeList* pC return code; } +static EDealRes collectWindowsPseudocolumns(SNode* pNode, void* pContext) { + SNodeList* pCols = (SNodeList*)pContext; + if (QUERY_NODE_FUNCTION == nodeType(pNode)) { + SFunctionNode* pFunc = (SFunctionNode*)pNode; + if (FUNCTION_TYPE_WSTART == pFunc->funcType || FUNCTION_TYPE_WEND == pFunc->funcType || + FUNCTION_TYPE_WDURATION == pFunc->funcType) { + nodesListStrictAppend(pCols, nodesCloneNode(pNode)); + } + } + return DEAL_RES_CONTINUE; +} + +static int32_t checkWindowsConditonValid(SWindowLogicNode* pWindowLogicNode) { + int32_t code = TSDB_CODE_SUCCESS; + SNodeList* pCols = nodesMakeList(); + if (NULL == pCols) { + return TSDB_CODE_OUT_OF_MEMORY; + } + nodesWalkExpr(pWindowLogicNode->pStartCond, collectWindowsPseudocolumns, pCols); + if (pCols->length > 0) { + code = TSDB_CODE_QRY_INVALID_WINDOW_CONDITION; + } + if (TSDB_CODE_SUCCESS == code) { + nodesWalkExpr(pWindowLogicNode->pEndCond, collectWindowsPseudocolumns, pCols); + if (pCols->length > 0) { + code = TSDB_CODE_QRY_INVALID_WINDOW_CONDITION; + } + } + + nodesDestroyList(pCols); + return code; +} + static int32_t createEventWindowPhysiNode(SPhysiPlanContext* pCxt, SNodeList* pChildren, SWindowLogicNode* pWindowLogicNode, SPhysiNode** pPhyNode) { SEventWinodwPhysiNode* pEvent = (SEventWinodwPhysiNode*)makePhysiNode( @@ -1731,8 +1764,13 @@ static int32_t createEventWindowPhysiNode(SPhysiPlanContext* pCxt, SNodeList* pC return TSDB_CODE_OUT_OF_MEMORY; } + int32_t code = checkWindowsConditonValid(pWindowLogicNode); + if (TSDB_CODE_SUCCESS != code) { + return code; + } + SDataBlockDescNode* pChildTupe = (((SPhysiNode*)nodesListGetNode(pChildren, 0))->pOutputDataBlockDesc); - int32_t code = setNodeSlotId(pCxt, pChildTupe->dataBlockId, -1, pWindowLogicNode->pStartCond, &pEvent->pStartCond); + code = setNodeSlotId(pCxt, pChildTupe->dataBlockId, -1, pWindowLogicNode->pStartCond, &pEvent->pStartCond); if (TSDB_CODE_SUCCESS == code) { code = setNodeSlotId(pCxt, pChildTupe->dataBlockId, -1, pWindowLogicNode->pEndCond, &pEvent->pEndCond); } diff --git a/source/util/src/terror.c b/source/util/src/terror.c index 79b9e9bbed..9aa1b97190 100644 --- a/source/util/src/terror.c +++ b/source/util/src/terror.c @@ -426,6 +426,7 @@ TAOS_DEFINE_ERROR(TSDB_CODE_QRY_JSON_IN_GROUP_ERROR, "Json not support in g TAOS_DEFINE_ERROR(TSDB_CODE_QRY_JOB_NOT_EXIST, "Job not exist") TAOS_DEFINE_ERROR(TSDB_CODE_QRY_QWORKER_QUIT, "Vnode/Qnode is quitting") TAOS_DEFINE_ERROR(TSDB_CODE_QRY_GEO_NOT_SUPPORT_ERROR, "Geometry not support in this operator") +TAOS_DEFINE_ERROR(TSDB_CODE_QRY_INVALID_WINDOW_CONDITION, "The time pseudo column is illegally used in the condition of the event window.") TAOS_DEFINE_ERROR(TSDB_CODE_QRY_EXECUTOR_INTERNAL_ERROR, "Executor internal error") // grant From 119d64ecdc6b91c6383bc8fc5ad4cd580b06f473 Mon Sep 17 00:00:00 2001 From: factosea <285808407@qq.com> Date: Mon, 22 Jan 2024 11:11:06 +0800 Subject: [PATCH 2/4] add test case --- tests/system-test/2-query/group_partition.py | 32 +++++++++++++++++++- 1 file changed, 31 insertions(+), 1 deletion(-) diff --git a/tests/system-test/2-query/group_partition.py b/tests/system-test/2-query/group_partition.py index e228351f0e..a20b124c33 100644 --- a/tests/system-test/2-query/group_partition.py +++ b/tests/system-test/2-query/group_partition.py @@ -168,8 +168,37 @@ class TDTestCase: tdSql.query(f"select tbname, count(*) from {self.dbname}.{self.stable} partition by tbname event_window start with c1 >= 0 end with c2 = 9;") tdSql.checkRows(nonempty_tb_num) + def test_event_window(self, nonempty_tb_num): + tdSql.query(f"select tbname, count(*) from {self.dbname}.{self.stable} partition by tbname event_window start with c1 >= 0 end with c2 = 9 and 1=1;") + tdSql.checkRows(nonempty_tb_num) + + tdSql.query(f"select tbname, count(*) from {self.dbname}.{self.stable} partition by tbname event_window start with c1 >= 0 end with c2 = 9 and 1=0;") + tdSql.checkRows(0) + + tdSql.query(f"select tbname, count(*) from {self.dbname}.{self.stable} partition by tbname event_window start with c1 >= 0 end with c2 = 9 and tbname='sub_{self.stable}_0';") + tdSql.checkRows(1) + + tdSql.query(f"select tbname, count(*) from {self.dbname}.{self.stable} partition by tbname event_window start with c1 >= 0 end with c2 = 9 and t2=0;") + tdSql.checkRows(1) + + tdSql.query(f"select tbname, count(*) from {self.dbname}.{self.stable} partition by tbname event_window start with c1 >= 0 end with c2 = 9 and _rowts>0;") + tdSql.checkRows(nonempty_tb_num) + + tdSql.query(f"select tbname, count(*) from {self.dbname}.{self.stable} partition by tbname event_window start with c1 >= 0 end with c2 = 9 and _qstart>0;") + tdSql.checkRows(0) + tdSql.query(f"select tbname, count(*) from {self.dbname}.{self.stable} partition by tbname event_window start with c1 >= 0 end with c2 = 9 and _qstart<0;") + tdSql.checkRows(0) + tdSql.query(f"select tbname, count(*) from {self.dbname}.{self.stable} partition by tbname event_window start with c1 >= 0 end with c2 = 9 and _qstart<_qend;") + tdSql.checkRows(0) + + tdSql.error(f"select tbname, count(*) from {self.dbname}.{self.stable} partition by tbname event_window start with c1 >= 0 end with c2 = 9 and _wstart= 0 end with c2 = 9 and _wstart - q_start > 0;") + tdSql.error(f"select tbname, count(*) from {self.dbname}.{self.stable} partition by tbname event_window start with c1 >= 0 end with c2 = 9 and _irowts>0;") + tdSql.error(f"select tbname, count(*) from {self.dbname}.{self.stable} partition by tbname event_window start with c1 >= 0 and _wduration > 5s end with c2 = 9;") + tdSql.error(f"select tbname, count(*) from {self.dbname}.{self.stable} partition by tbname event_window start with c1 >= 0 end with c2 = 9 and _wstart > 1299845454;") + tdSql.error(f"select tbname, count(*) from {self.dbname}.{self.stable} partition by tbname event_window start with c1 >= 0 end with c2 = 9 and _wduration + 1s > 5s;") + tdSql.error(f"select tbname, count(*) from {self.dbname}.{self.stable} partition by tbname event_window start with c1 >= 0 end with c2 = 9 and count(*) > 10;") - def test_error(self): tdSql.error(f"select * from {self.dbname}.{self.stable} group by t2") tdSql.error(f"select t2, count(*) from {self.dbname}.{self.stable} group by t2 where t2 = 1") @@ -197,6 +226,7 @@ class TDTestCase: self.test_multi_group_key(self.tb_nums, nonempty_tb_num) self.test_multi_agg(self.tb_nums, nonempty_tb_num) self.test_window(nonempty_tb_num) + self.test_event_window(nonempty_tb_num) ## test old version before changed # self.test_groupby('group', 0, 0) From 6ef1d8b0cb460229e8d1e33764beddea8d1aa1e2 Mon Sep 17 00:00:00 2001 From: factosea <285808407@qq.com> Date: Mon, 22 Jan 2024 17:16:55 +0800 Subject: [PATCH 3/4] checkout pseudo columns --- source/libs/parser/src/parTranslater.c | 40 ++++++++++++++++++++++ source/libs/planner/src/planPhysiCreater.c | 40 +--------------------- 2 files changed, 41 insertions(+), 39 deletions(-) diff --git a/source/libs/parser/src/parTranslater.c b/source/libs/parser/src/parTranslater.c index 70e4744644..f8d1573df4 100644 --- a/source/libs/parser/src/parTranslater.c +++ b/source/libs/parser/src/parTranslater.c @@ -3981,6 +3981,42 @@ static int32_t translateSpecificWindow(STranslateContext* pCxt, SSelectStmt* pSe return TSDB_CODE_SUCCESS; } +static EDealRes collectWindowsPseudocolumns(SNode* pNode, void* pContext) { + SNodeList* pCols = (SNodeList*)pContext; + if (QUERY_NODE_FUNCTION == nodeType(pNode)) { + SFunctionNode* pFunc = (SFunctionNode*)pNode; + if (FUNCTION_TYPE_WSTART == pFunc->funcType || FUNCTION_TYPE_WEND == pFunc->funcType || + FUNCTION_TYPE_WDURATION == pFunc->funcType) { + nodesListStrictAppend(pCols, nodesCloneNode(pNode)); + } + } + return DEAL_RES_CONTINUE; +} + +static int32_t checkWindowsConditonValid(SNode* pNode) { + int32_t code = TSDB_CODE_SUCCESS; + if(QUERY_NODE_EVENT_WINDOW != nodeType(pNode)) return code; + + SEventWindowNode* pEventWindowNode = (SEventWindowNode*)pNode; + SNodeList* pCols = nodesMakeList(); + if (NULL == pCols) { + return TSDB_CODE_OUT_OF_MEMORY; + } + nodesWalkExpr(pEventWindowNode->pStartCond, collectWindowsPseudocolumns, pCols); + if (pCols->length > 0) { + code = TSDB_CODE_QRY_INVALID_WINDOW_CONDITION; + } + if (TSDB_CODE_SUCCESS == code) { + nodesWalkExpr(pEventWindowNode->pEndCond, collectWindowsPseudocolumns, pCols); + if (pCols->length > 0) { + code = TSDB_CODE_QRY_INVALID_WINDOW_CONDITION; + } + } + + nodesDestroyList(pCols); + return code; +} + static int32_t translateWindow(STranslateContext* pCxt, SSelectStmt* pSelect) { if (NULL == pSelect->pWindow) { return TSDB_CODE_SUCCESS; @@ -3994,6 +4030,10 @@ static int32_t translateWindow(STranslateContext* pCxt, SSelectStmt* pSelect) { if (TSDB_CODE_SUCCESS == code) { code = translateSpecificWindow(pCxt, pSelect); } + code = checkWindowsConditonValid(pSelect->pWindow); + if (TSDB_CODE_SUCCESS != code) { + return code; + } return code; } diff --git a/source/libs/planner/src/planPhysiCreater.c b/source/libs/planner/src/planPhysiCreater.c index 301ba4fceb..21c637116f 100644 --- a/source/libs/planner/src/planPhysiCreater.c +++ b/source/libs/planner/src/planPhysiCreater.c @@ -1722,39 +1722,6 @@ static int32_t createStateWindowPhysiNode(SPhysiPlanContext* pCxt, SNodeList* pC return code; } -static EDealRes collectWindowsPseudocolumns(SNode* pNode, void* pContext) { - SNodeList* pCols = (SNodeList*)pContext; - if (QUERY_NODE_FUNCTION == nodeType(pNode)) { - SFunctionNode* pFunc = (SFunctionNode*)pNode; - if (FUNCTION_TYPE_WSTART == pFunc->funcType || FUNCTION_TYPE_WEND == pFunc->funcType || - FUNCTION_TYPE_WDURATION == pFunc->funcType) { - nodesListStrictAppend(pCols, nodesCloneNode(pNode)); - } - } - return DEAL_RES_CONTINUE; -} - -static int32_t checkWindowsConditonValid(SWindowLogicNode* pWindowLogicNode) { - int32_t code = TSDB_CODE_SUCCESS; - SNodeList* pCols = nodesMakeList(); - if (NULL == pCols) { - return TSDB_CODE_OUT_OF_MEMORY; - } - nodesWalkExpr(pWindowLogicNode->pStartCond, collectWindowsPseudocolumns, pCols); - if (pCols->length > 0) { - code = TSDB_CODE_QRY_INVALID_WINDOW_CONDITION; - } - if (TSDB_CODE_SUCCESS == code) { - nodesWalkExpr(pWindowLogicNode->pEndCond, collectWindowsPseudocolumns, pCols); - if (pCols->length > 0) { - code = TSDB_CODE_QRY_INVALID_WINDOW_CONDITION; - } - } - - nodesDestroyList(pCols); - return code; -} - static int32_t createEventWindowPhysiNode(SPhysiPlanContext* pCxt, SNodeList* pChildren, SWindowLogicNode* pWindowLogicNode, SPhysiNode** pPhyNode) { SEventWinodwPhysiNode* pEvent = (SEventWinodwPhysiNode*)makePhysiNode( @@ -1764,13 +1731,8 @@ static int32_t createEventWindowPhysiNode(SPhysiPlanContext* pCxt, SNodeList* pC return TSDB_CODE_OUT_OF_MEMORY; } - int32_t code = checkWindowsConditonValid(pWindowLogicNode); - if (TSDB_CODE_SUCCESS != code) { - return code; - } - SDataBlockDescNode* pChildTupe = (((SPhysiNode*)nodesListGetNode(pChildren, 0))->pOutputDataBlockDesc); - code = setNodeSlotId(pCxt, pChildTupe->dataBlockId, -1, pWindowLogicNode->pStartCond, &pEvent->pStartCond); + int32_t code = setNodeSlotId(pCxt, pChildTupe->dataBlockId, -1, pWindowLogicNode->pStartCond, &pEvent->pStartCond); if (TSDB_CODE_SUCCESS == code) { code = setNodeSlotId(pCxt, pChildTupe->dataBlockId, -1, pWindowLogicNode->pEndCond, &pEvent->pEndCond); } From bde8c14b55eae9d91b960769208a3042c08daf7e Mon Sep 17 00:00:00 2001 From: factosea <285808407@qq.com> Date: Mon, 22 Jan 2024 20:54:24 +0800 Subject: [PATCH 4/4] fix: error code is overwritten --- source/libs/parser/src/parTranslater.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/source/libs/parser/src/parTranslater.c b/source/libs/parser/src/parTranslater.c index f8d1573df4..23396a499d 100644 --- a/source/libs/parser/src/parTranslater.c +++ b/source/libs/parser/src/parTranslater.c @@ -4030,9 +4030,8 @@ static int32_t translateWindow(STranslateContext* pCxt, SSelectStmt* pSelect) { if (TSDB_CODE_SUCCESS == code) { code = translateSpecificWindow(pCxt, pSelect); } - code = checkWindowsConditonValid(pSelect->pWindow); - if (TSDB_CODE_SUCCESS != code) { - return code; + if (TSDB_CODE_SUCCESS == code) { + code = checkWindowsConditonValid(pSelect->pWindow); } return code; }