From 9fd6d0593f45745df4eab56a0b592198fb3b0d6a Mon Sep 17 00:00:00 2001 From: Haojun Liao Date: Tue, 29 Jun 2021 22:58:00 +0800 Subject: [PATCH 1/4] [td-4954]:update some error message strings. --- src/client/src/tscSQLParser.c | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/src/client/src/tscSQLParser.c b/src/client/src/tscSQLParser.c index 50c8aa0376..3ceb10520b 100644 --- a/src/client/src/tscSQLParser.c +++ b/src/client/src/tscSQLParser.c @@ -1309,15 +1309,8 @@ static bool validateTagParams(SArray* pTagsList, SArray* pFieldList, SSqlCmd* pC return false; } - /* timestamp in tag is not allowed */ for (int32_t i = 0; i < numOfTags; ++i) { TAOS_FIELD* p = taosArrayGet(pTagsList, i); - - //if (p->type == TSDB_DATA_TYPE_TIMESTAMP) { - // invalidOperationMsg(tscGetErrorMsgPayload(pCmd), msg4); - // return false; - //} - if (!isValidDataType(p->type)) { invalidOperationMsg(tscGetErrorMsgPayload(pCmd), msg5); return false; @@ -2174,7 +2167,7 @@ int32_t addExprAndResultField(SSqlCmd* pCmd, SQueryInfo* pQueryInfo, int32_t col const char* msg3 = "illegal column name"; const char* msg4 = "invalid table name"; const char* msg5 = "parameter is out of range [0, 100]"; - const char* msg6 = "function applied to tags not allowed"; + const char* msg6 = "functions applied to tags are not allowed"; const char* msg7 = "normal table can not apply this function"; const char* msg8 = "multi-columns selection does not support alias column name"; const char* msg9 = "diff/derivative can no be applied to unsigned numeric type"; @@ -3087,9 +3080,9 @@ void tscRestoreFuncForSTableQuery(SQueryInfo* pQueryInfo) { } bool hasUnsupportFunctionsForSTableQuery(SSqlCmd* pCmd, SQueryInfo* pQueryInfo) { - const char* msg1 = "TWA/Diff/Derivative/Irate not allowed to apply to super table directly"; + const char* msg1 = "TWA/Diff/Derivative/Irate are not allowed to apply to super table directly"; const char* msg2 = "TWA/Diff/Derivative/Irate only support group by tbname for super table query"; - const char* msg3 = "function not support for super table query"; + const char* msg3 = "functions not support for super table query"; // filter sql function not supported by metric query yet. size_t size = tscNumOfExprs(pQueryInfo); From e55053fc451995827950209bfa03266d9d136a68 Mon Sep 17 00:00:00 2001 From: Haojun Liao Date: Wed, 30 Jun 2021 10:31:40 +0800 Subject: [PATCH 2/4] [td-4966]:add more check in group by clause. --- src/client/src/tscSQLParser.c | 72 ++++++++++++++++++++--------------- 1 file changed, 41 insertions(+), 31 deletions(-) diff --git a/src/client/src/tscSQLParser.c b/src/client/src/tscSQLParser.c index 3ceb10520b..3c0c9ae6c3 100644 --- a/src/client/src/tscSQLParser.c +++ b/src/client/src/tscSQLParser.c @@ -3187,34 +3187,42 @@ int32_t validateGroupbyNode(SQueryInfo* pQueryInfo, SArray* pList, SSqlCmd* pCmd const char* msg2 = "invalid column name in group by clause"; const char* msg3 = "columns from one table allowed as group by columns"; const char* msg4 = "join query does not support group by"; + const char* msg5 = "not allowed column type for group by"; + const char* msg6 = "tags not allowed for table query"; const char* msg7 = "not support group by expression"; - const char* msg8 = "not allowed column type for group by"; - const char* msg9 = "tags not allowed for table query"; + const char* msg8 = "normal column can only locate at the end of group by clause"; // todo : handle two tables situation STableMetaInfo* pTableMetaInfo = NULL; - if (pList == NULL) { return TSDB_CODE_SUCCESS; } - if (pQueryInfo->colList == NULL) { - pQueryInfo->colList = taosArrayInit(4, POINTER_BYTES); - } - - pQueryInfo->groupbyExpr.numOfGroupCols = (int16_t)taosArrayGetSize(pList); - if (pQueryInfo->groupbyExpr.numOfGroupCols > TSDB_MAX_TAGS) { - return invalidOperationMsg(tscGetErrorMsgPayload(pCmd), msg1); - } - if (pQueryInfo->numOfTables > 1) { return invalidOperationMsg(tscGetErrorMsgPayload(pCmd), msg4); } - STableMeta* pTableMeta = NULL; - SSchema* pSchema = NULL; + SGroupbyExpr* pGroupExpr = &pQueryInfo->groupbyExpr; + if (pGroupExpr->columnInfo == NULL) { + pGroupExpr->columnInfo = taosArrayInit(4, sizeof(SColIndex)); + } - int32_t tableIndex = COLUMN_INDEX_INITIAL_VAL; + if (pQueryInfo->colList == NULL) { + pQueryInfo->colList = taosArrayInit(4, POINTER_BYTES); + } + + if (pGroupExpr->columnInfo == NULL || pQueryInfo->colList == NULL) { + return TSDB_CODE_TSC_OUT_OF_MEMORY; + } + + pGroupExpr->numOfGroupCols = (int16_t)taosArrayGetSize(pList); + if (pGroupExpr->numOfGroupCols > TSDB_MAX_TAGS) { + return invalidOperationMsg(tscGetErrorMsgPayload(pCmd), msg1); + } + + SSchema *pSchema = NULL; + int32_t tableIndex = COLUMN_INDEX_INITIAL_VAL; + int32_t numOfGroupCols = 0; size_t num = taosArrayGetSize(pList); for (int32_t i = 0; i < num; ++i) { @@ -3235,28 +3243,20 @@ int32_t validateGroupbyNode(SQueryInfo* pQueryInfo, SArray* pList, SSqlCmd* pCmd } pTableMetaInfo = tscGetMetaInfo(pQueryInfo, index.tableIndex); - pTableMeta = pTableMetaInfo->pTableMeta; + STableMeta* pTableMeta = pTableMetaInfo->pTableMeta; - int32_t numOfCols = tscGetNumOfColumns(pTableMeta); if (index.columnIndex == TSDB_TBNAME_COLUMN_INDEX) { pSchema = tGetTbnameColumnSchema(); } else { pSchema = tscGetTableColumnSchema(pTableMeta, index.columnIndex); } - bool groupTag = false; - if (index.columnIndex == TSDB_TBNAME_COLUMN_INDEX || index.columnIndex >= numOfCols) { - groupTag = true; - } + int32_t numOfCols = tscGetNumOfColumns(pTableMeta); + bool groupTag = (index.columnIndex == TSDB_TBNAME_COLUMN_INDEX || index.columnIndex >= numOfCols); - SGroupbyExpr* pGroupExpr = &pQueryInfo->groupbyExpr; - if (pGroupExpr->columnInfo == NULL) { - pGroupExpr->columnInfo = taosArrayInit(4, sizeof(SColIndex)); - } - if (groupTag) { if (!UTIL_TABLE_IS_SUPER_TABLE(pTableMetaInfo)) { - return invalidOperationMsg(tscGetErrorMsgPayload(pCmd), msg9); + return invalidOperationMsg(tscGetErrorMsgPayload(pCmd), msg6); } int32_t relIndex = index.columnIndex; @@ -3273,7 +3273,7 @@ int32_t validateGroupbyNode(SQueryInfo* pQueryInfo, SArray* pList, SSqlCmd* pCmd } else { // check if the column type is valid, here only support the bool/tinyint/smallint/bigint group by if (pSchema->type == TSDB_DATA_TYPE_TIMESTAMP || pSchema->type == TSDB_DATA_TYPE_FLOAT || pSchema->type == TSDB_DATA_TYPE_DOUBLE) { - return invalidOperationMsg(tscGetErrorMsgPayload(pCmd), msg8); + return invalidOperationMsg(tscGetErrorMsgPayload(pCmd), msg5); } tscColumnListInsert(pQueryInfo->colList, index.columnIndex, pTableMeta->id.uid, pSchema); @@ -3283,10 +3283,20 @@ int32_t validateGroupbyNode(SQueryInfo* pQueryInfo, SArray* pList, SSqlCmd* pCmd taosArrayPush(pGroupExpr->columnInfo, &colIndex); pQueryInfo->groupbyExpr.orderType = TSDB_ORDER_ASC; + numOfGroupCols++; + } + } - if (i == 0 && num > 1) { - return invalidOperationMsg(tscGetErrorMsgPayload(pCmd), msg7); - } + // 1. only one normal column allowed in the group by clause + // 2. the normal column in the group by clause can only located in the end position + if (numOfGroupCols) { + return invalidOperationMsg(tscGetErrorMsgPayload(pCmd), msg7); + } + + for(int32_t i = 0; i < num; ++i) { + SColIndex* pIndex = taosArrayGet(pGroupExpr->columnInfo, i); + if (TSDB_COL_IS_NORMAL_COL(pIndex->flag) && i != num - 1) { + return invalidOperationMsg(tscGetErrorMsgPayload(pCmd), msg8); } } From 1f670fc95a637512bf91872517cc352659689853 Mon Sep 17 00:00:00 2001 From: Haojun Liao Date: Wed, 30 Jun 2021 11:27:17 +0800 Subject: [PATCH 3/4] [td-225]update the sim. --- tests/script/general/parser/groupby.sim | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/tests/script/general/parser/groupby.sim b/tests/script/general/parser/groupby.sim index 6ae5d420d8..e47af5588e 100644 --- a/tests/script/general/parser/groupby.sim +++ b/tests/script/general/parser/groupby.sim @@ -781,4 +781,14 @@ if $data11 != 2 then return -1 endi +sql_error select count(*) from m1 group by tbname,k,f1; +sql_error select count(*) from m1 group by tbname,k,a; +sql_error select count(*) from m1 group by k, tbname; +sql_error select count(*) from m1 group by k,f1; +sql_error select count(*) from tm0 group by tbname; +sql_error select count(*) from tm0 group by a; +sql_error select count(*) from tm0 group by k,f1; + +sql_error select count(*),f1 from m1 group by tbname,k; + system sh/exec.sh -n dnode1 -s stop -x SIGINT From d9ba8b1662788d9a93bca79e9d7fb5caac3b2a54 Mon Sep 17 00:00:00 2001 From: Haojun Liao Date: Wed, 30 Jun 2021 13:06:51 +0800 Subject: [PATCH 4/4] [td-225] --- src/client/src/tscSQLParser.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/client/src/tscSQLParser.c b/src/client/src/tscSQLParser.c index 3c0c9ae6c3..ac4a0bc9d8 100644 --- a/src/client/src/tscSQLParser.c +++ b/src/client/src/tscSQLParser.c @@ -3289,7 +3289,7 @@ int32_t validateGroupbyNode(SQueryInfo* pQueryInfo, SArray* pList, SSqlCmd* pCmd // 1. only one normal column allowed in the group by clause // 2. the normal column in the group by clause can only located in the end position - if (numOfGroupCols) { + if (numOfGroupCols > 1) { return invalidOperationMsg(tscGetErrorMsgPayload(pCmd), msg7); } @@ -6283,7 +6283,7 @@ static void updateTagPrjFunction(SQueryInfo* pQueryInfo) { */ static int32_t checkUpdateTagPrjFunctions(SQueryInfo* pQueryInfo, char* msg) { const char* msg1 = "only one selectivity function allowed in presence of tags function"; - const char* msg3 = "aggregation function should not be mixed up with projection"; + const char* msg2 = "aggregation function should not be mixed up with projection"; bool tagTsColExists = false; int16_t numOfSelectivity = 0; @@ -6362,7 +6362,7 @@ static int32_t checkUpdateTagPrjFunctions(SQueryInfo* pQueryInfo, char* msg) { } else { if ((pQueryInfo->type & TSDB_QUERY_TYPE_PROJECTION_QUERY) != 0) { if (numOfAggregation > 0 && pQueryInfo->groupbyExpr.numOfGroupCols == 0) { - return invalidOperationMsg(msg, msg3); + return invalidOperationMsg(msg, msg2); } if (numOfAggregation > 0 || numOfSelectivity > 0) {