From 272494d1c520af6ba1ba156636cf2350258cd7b8 Mon Sep 17 00:00:00 2001 From: Haojun Liao Date: Sat, 3 Apr 2021 23:45:56 +0800 Subject: [PATCH 1/8] [td-3662] : fix invalid write caused by top/bottom query. --- src/client/inc/tscUtil.h | 2 ++ src/client/src/tscLocalMerge.c | 11 +++++++++-- src/client/src/tscUtil.c | 35 ++++++++++++++++++++++++++++++++++ 3 files changed, 46 insertions(+), 2 deletions(-) diff --git a/src/client/inc/tscUtil.h b/src/client/inc/tscUtil.h index 9ef13cccbf..0eda49b1f4 100644 --- a/src/client/inc/tscUtil.h +++ b/src/client/inc/tscUtil.h @@ -129,6 +129,8 @@ bool tscIsPointInterpQuery(SQueryInfo* pQueryInfo); bool tscIsTWAQuery(SQueryInfo* pQueryInfo); bool tscIsSecondStageQuery(SQueryInfo* pQueryInfo); bool tscGroupbyColumn(SQueryInfo* pQueryInfo); +bool tscIsTopbotQuery(SQueryInfo* pQueryInfo); +int32_t tscGetTopbotQueryParam(SQueryInfo* pQueryInfo); bool tscNonOrderedProjectionQueryOnSTable(SQueryInfo *pQueryInfo, int32_t tableIndex); bool tscOrderedProjectionQueryOnSTable(SQueryInfo* pQueryInfo, int32_t tableIndex); diff --git a/src/client/src/tscLocalMerge.c b/src/client/src/tscLocalMerge.c index 8a301d1820..26e21170ac 100644 --- a/src/client/src/tscLocalMerge.c +++ b/src/client/src/tscLocalMerge.c @@ -338,12 +338,19 @@ void tscCreateLocalMerger(tExtMemBuffer **pMemBuffer, int32_t numOfBuffer, tOrde pReducer->resColModel->capacity = pReducer->nResultBufSize; pReducer->finalModel = pFFModel; + int32_t expandFactor = 1; if (finalmodel->rowSize > 0) { - pReducer->resColModel->capacity /= finalmodel->rowSize; + bool topBotQuery = tscIsTopbotQuery(pQueryInfo); + if (topBotQuery) { + expandFactor = tscGetTopbotQueryParam(pQueryInfo); + pReducer->resColModel->capacity /= (finalmodel->rowSize * expandFactor); + } else { + pReducer->resColModel->capacity /= finalmodel->rowSize; + } } assert(finalmodel->rowSize > 0 && finalmodel->rowSize <= pReducer->rowSize); - pReducer->pFinalRes = calloc(1, pReducer->rowSize * pReducer->resColModel->capacity); + pReducer->pFinalRes = calloc(1, pReducer->rowSize * pReducer->resColModel->capacity * expandFactor); if (pReducer->pTempBuffer == NULL || pReducer->discardData == NULL || pReducer->pResultBuf == NULL || pReducer->pFinalRes == NULL || pReducer->prevRowOfInput == NULL) { diff --git a/src/client/src/tscUtil.c b/src/client/src/tscUtil.c index 386352bdc7..c6c0c9324e 100644 --- a/src/client/src/tscUtil.c +++ b/src/client/src/tscUtil.c @@ -271,6 +271,41 @@ bool tscIsTWAQuery(SQueryInfo* pQueryInfo) { return false; } +bool tscIsTopbotQuery(SQueryInfo* pQueryInfo) { + size_t numOfExprs = tscSqlExprNumOfExprs(pQueryInfo); + for (int32_t i = 0; i < numOfExprs; ++i) { + SSqlExpr* pExpr = tscSqlExprGet(pQueryInfo, i); + if (pExpr == NULL) { + continue; + } + + int32_t functionId = pExpr->functionId; + if (functionId == TSDB_FUNC_TOP || functionId == TSDB_FUNC_BOTTOM) { + return true; + } + } + + return false; +} + +int32_t tscGetTopbotQueryParam(SQueryInfo* pQueryInfo) { + size_t numOfExprs = tscSqlExprNumOfExprs(pQueryInfo); + for (int32_t i = 0; i < numOfExprs; ++i) { + SSqlExpr* pExpr = tscSqlExprGet(pQueryInfo, i); + if (pExpr == NULL) { + continue; + } + + int32_t functionId = pExpr->functionId; + if (functionId == TSDB_FUNC_TOP || functionId == TSDB_FUNC_BOTTOM) { + return pExpr->param[0].i64; + } + } + + return 0; +} + + void tscClearInterpInfo(SQueryInfo* pQueryInfo) { if (!tscIsPointInterpQuery(pQueryInfo)) { return; From 32590cb8037b2d28c04d85daa8c9823851289603 Mon Sep 17 00:00:00 2001 From: Haojun Liao Date: Sun, 4 Apr 2021 00:17:33 +0800 Subject: [PATCH 2/8] [td-3662]fix compiler error. --- src/client/src/tscUtil.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/client/src/tscUtil.c b/src/client/src/tscUtil.c index c6c0c9324e..438e5618df 100644 --- a/src/client/src/tscUtil.c +++ b/src/client/src/tscUtil.c @@ -298,7 +298,7 @@ int32_t tscGetTopbotQueryParam(SQueryInfo* pQueryInfo) { int32_t functionId = pExpr->functionId; if (functionId == TSDB_FUNC_TOP || functionId == TSDB_FUNC_BOTTOM) { - return pExpr->param[0].i64; + return (int32_t) pExpr->param[0].i64; } } From 4d57ac1e85da03a960e18da77664d00382f759b5 Mon Sep 17 00:00:00 2001 From: Haojun Liao Date: Sun, 4 Apr 2021 01:05:10 +0800 Subject: [PATCH 3/8] [td-3662]fix false filter. --- src/query/src/qExecutor.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/query/src/qExecutor.c b/src/query/src/qExecutor.c index 7ba629cfc2..e6f5eabd1c 100644 --- a/src/query/src/qExecutor.c +++ b/src/query/src/qExecutor.c @@ -2624,6 +2624,21 @@ int32_t loadDataBlockOnDemand(SQueryRuntimeEnv* pRuntimeEnv, STableScanInfo* pTa tsdbRetrieveDataBlockStatisInfo(pTableScanInfo->pQueryHandle, &pBlock->pBlockStatis); if (pQuery->topBotQuery && pBlock->pBlockStatis != NULL) { + { // set previous window + if (QUERY_IS_INTERVAL_QUERY(pQuery)) { + SResultRow* pResult = NULL; + + bool masterScan = IS_MASTER_SCAN(pRuntimeEnv); + TSKEY k = ascQuery? pBlock->info.window.skey : pBlock->info.window.ekey; + + STimeWindow win = getActiveTimeWindow(pTableScanInfo->pResultRowInfo, k, pQuery); + if (setWindowOutputBufByKey(pRuntimeEnv, pTableScanInfo->pResultRowInfo, &win, masterScan, &pResult, groupId, + pTableScanInfo->pCtx, pTableScanInfo->numOfOutput, + pTableScanInfo->rowCellInfoOffset) != TSDB_CODE_SUCCESS) { + longjmp(pRuntimeEnv->env, TSDB_CODE_QRY_OUT_OF_MEMORY); + } + } + } bool load = false; for (int32_t i = 0; i < pQuery->numOfOutput; ++i) { int32_t functionId = pTableScanInfo->pCtx[i].functionId; From 3aa657e3e72b11a926ab05bd19b7b92048c20a2f Mon Sep 17 00:00:00 2001 From: Haojun Liao Date: Sun, 4 Apr 2021 10:43:33 +0800 Subject: [PATCH 4/8] [td-3662]fix bug found by regression test. --- src/client/src/tscLocalMerge.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/client/src/tscLocalMerge.c b/src/client/src/tscLocalMerge.c index 26e21170ac..a5b2f46eed 100644 --- a/src/client/src/tscLocalMerge.c +++ b/src/client/src/tscLocalMerge.c @@ -344,13 +344,14 @@ void tscCreateLocalMerger(tExtMemBuffer **pMemBuffer, int32_t numOfBuffer, tOrde if (topBotQuery) { expandFactor = tscGetTopbotQueryParam(pQueryInfo); pReducer->resColModel->capacity /= (finalmodel->rowSize * expandFactor); + pReducer->resColModel->capacity *= expandFactor; } else { pReducer->resColModel->capacity /= finalmodel->rowSize; } } assert(finalmodel->rowSize > 0 && finalmodel->rowSize <= pReducer->rowSize); - pReducer->pFinalRes = calloc(1, pReducer->rowSize * pReducer->resColModel->capacity * expandFactor); + pReducer->pFinalRes = calloc(1, pReducer->rowSize * pReducer->resColModel->capacity); if (pReducer->pTempBuffer == NULL || pReducer->discardData == NULL || pReducer->pResultBuf == NULL || pReducer->pFinalRes == NULL || pReducer->prevRowOfInput == NULL) { @@ -1157,7 +1158,7 @@ static void fillMultiRowsOfTagsVal(SQueryInfo *pQueryInfo, int32_t numOfRes, SLo memset(buf, 0, (size_t)maxBufSize); memcpy(buf, pCtx->pOutput, (size_t)pCtx->outputBytes); - for (int32_t i = 0; i < inc; ++i) { + for (int32_t i = 1; i < inc; ++i) { pCtx->pOutput += pCtx->outputBytes; memcpy(pCtx->pOutput, buf, (size_t)pCtx->outputBytes); } From 3fda7a864733a91538bb667882eaebf6f4323a2e Mon Sep 17 00:00:00 2001 From: Haojun Liao Date: Sun, 4 Apr 2021 11:16:15 +0800 Subject: [PATCH 5/8] [td-3662]fix bug found by regression test. --- src/client/src/tscLocalMerge.c | 3 +++ tests/script/general/parser/select_with_tags.sim | 3 +++ 2 files changed, 6 insertions(+) diff --git a/src/client/src/tscLocalMerge.c b/src/client/src/tscLocalMerge.c index a5b2f46eed..704254b0cb 100644 --- a/src/client/src/tscLocalMerge.c +++ b/src/client/src/tscLocalMerge.c @@ -351,6 +351,8 @@ void tscCreateLocalMerger(tExtMemBuffer **pMemBuffer, int32_t numOfBuffer, tOrde } assert(finalmodel->rowSize > 0 && finalmodel->rowSize <= pReducer->rowSize); + printf("------xxxx:%d\n", pReducer->rowSize * pReducer->resColModel->capacity); + pReducer->pFinalRes = calloc(1, pReducer->rowSize * pReducer->resColModel->capacity); if (pReducer->pTempBuffer == NULL || pReducer->discardData == NULL || pReducer->pResultBuf == NULL || @@ -942,6 +944,7 @@ static void genFinalResWithoutFill(SSqlRes* pRes, SLocalMerger *pLocalMerge, SQu savePrevRecordAndSetupFillInfo(pLocalMerge, pQueryInfo, pLocalMerge->pFillInfo); } + printf("size: %d\n", pRes->numOfRows * pLocalMerge->finalModel->rowSize); memcpy(pRes->data, pBeforeFillData->data, (size_t)(pRes->numOfRows * pLocalMerge->finalModel->rowSize)); pRes->numOfClauseTotal += pRes->numOfRows; diff --git a/tests/script/general/parser/select_with_tags.sim b/tests/script/general/parser/select_with_tags.sim index 38a514a51b..286949421e 100644 --- a/tests/script/general/parser/select_with_tags.sim +++ b/tests/script/general/parser/select_with_tags.sim @@ -159,6 +159,9 @@ if $data03 != @abc15@ then return -1 endi +sql select top(c6, 3) from select_tags_mt0 interval(10a) + + sql select top(c1, 100), tbname, t1, t2 from select_tags_mt0; if $rows != 100 then return -1 From 859054fd48d5989b25b24c86f855dbbe08c4d545 Mon Sep 17 00:00:00 2001 From: Haojun Liao Date: Sun, 4 Apr 2021 11:16:50 +0800 Subject: [PATCH 6/8] [td-3662]fix bug found by regression test. --- tests/script/general/parser/select_with_tags.sim | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/script/general/parser/select_with_tags.sim b/tests/script/general/parser/select_with_tags.sim index 286949421e..ab6a0272bc 100644 --- a/tests/script/general/parser/select_with_tags.sim +++ b/tests/script/general/parser/select_with_tags.sim @@ -159,7 +159,7 @@ if $data03 != @abc15@ then return -1 endi -sql select top(c6, 3) from select_tags_mt0 interval(10a) +#sql select top(c6, 3) from select_tags_mt0 interval(10a) sql select top(c1, 100), tbname, t1, t2 from select_tags_mt0; From 00c1a59abbb0fb05e3bf1ddee49ff87c1a515b2c Mon Sep 17 00:00:00 2001 From: Haojun Liao Date: Sun, 4 Apr 2021 11:18:28 +0800 Subject: [PATCH 7/8] [td-3662] --- src/client/src/tscLocalMerge.c | 2 -- tests/script/general/parser/select_with_tags.sim | 2 +- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/src/client/src/tscLocalMerge.c b/src/client/src/tscLocalMerge.c index 704254b0cb..c28302895e 100644 --- a/src/client/src/tscLocalMerge.c +++ b/src/client/src/tscLocalMerge.c @@ -351,7 +351,6 @@ void tscCreateLocalMerger(tExtMemBuffer **pMemBuffer, int32_t numOfBuffer, tOrde } assert(finalmodel->rowSize > 0 && finalmodel->rowSize <= pReducer->rowSize); - printf("------xxxx:%d\n", pReducer->rowSize * pReducer->resColModel->capacity); pReducer->pFinalRes = calloc(1, pReducer->rowSize * pReducer->resColModel->capacity); @@ -944,7 +943,6 @@ static void genFinalResWithoutFill(SSqlRes* pRes, SLocalMerger *pLocalMerge, SQu savePrevRecordAndSetupFillInfo(pLocalMerge, pQueryInfo, pLocalMerge->pFillInfo); } - printf("size: %d\n", pRes->numOfRows * pLocalMerge->finalModel->rowSize); memcpy(pRes->data, pBeforeFillData->data, (size_t)(pRes->numOfRows * pLocalMerge->finalModel->rowSize)); pRes->numOfClauseTotal += pRes->numOfRows; diff --git a/tests/script/general/parser/select_with_tags.sim b/tests/script/general/parser/select_with_tags.sim index ab6a0272bc..6d22332fb2 100644 --- a/tests/script/general/parser/select_with_tags.sim +++ b/tests/script/general/parser/select_with_tags.sim @@ -160,7 +160,7 @@ if $data03 != @abc15@ then endi #sql select top(c6, 3) from select_tags_mt0 interval(10a) - +#sql select top(c6, 3) from select_tags_mt0 interval(10a) group by tbname; sql select top(c1, 100), tbname, t1, t2 from select_tags_mt0; if $rows != 100 then From ecfe8ecabb32b12da52e1b28f7d2b97dcb182fed Mon Sep 17 00:00:00 2001 From: Haojun Liao Date: Sun, 4 Apr 2021 15:33:18 +0800 Subject: [PATCH 8/8] [td-3662] --- src/client/src/tscLocalMerge.c | 9 +++++++-- tests/script/general/parser/select_with_tags.sim | 5 +++++ 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/src/client/src/tscLocalMerge.c b/src/client/src/tscLocalMerge.c index c28302895e..89642a067d 100644 --- a/src/client/src/tscLocalMerge.c +++ b/src/client/src/tscLocalMerge.c @@ -1159,7 +1159,7 @@ static void fillMultiRowsOfTagsVal(SQueryInfo *pQueryInfo, int32_t numOfRes, SLo memset(buf, 0, (size_t)maxBufSize); memcpy(buf, pCtx->pOutput, (size_t)pCtx->outputBytes); - for (int32_t i = 1; i < inc; ++i) { + for (int32_t i = 0; i < inc; ++i) { pCtx->pOutput += pCtx->outputBytes; memcpy(pCtx->pOutput, buf, (size_t)pCtx->outputBytes); } @@ -1449,6 +1449,11 @@ int32_t tscDoLocalMerge(SSqlObj *pSql) { SQueryInfo *pQueryInfo = tscGetQueryInfoDetail(pCmd, pCmd->clauseIndex); tFilePage *tmpBuffer = pLocalMerge->pTempBuffer; + int32_t remain = 1; + if (tscIsTopbotQuery(pQueryInfo)) { + remain = tscGetTopbotQueryParam(pQueryInfo); + } + if (doHandleLastRemainData(pSql)) { return TSDB_CODE_SUCCESS; } @@ -1537,7 +1542,7 @@ int32_t tscDoLocalMerge(SSqlObj *pSql) { * if the previous group does NOT generate any result (pResBuf->num == 0), * continue to process results instead of return results. */ - if ((!sameGroup && pResBuf->num > 0) || (pResBuf->num == pLocalMerge->resColModel->capacity)) { + if ((!sameGroup && pResBuf->num > 0) || (pResBuf->num + remain >= pLocalMerge->resColModel->capacity)) { // does not belong to the same group bool notSkipped = genFinalResults(pSql, pLocalMerge, !sameGroup); diff --git a/tests/script/general/parser/select_with_tags.sim b/tests/script/general/parser/select_with_tags.sim index 6d22332fb2..ab34713d90 100644 --- a/tests/script/general/parser/select_with_tags.sim +++ b/tests/script/general/parser/select_with_tags.sim @@ -162,6 +162,11 @@ endi #sql select top(c6, 3) from select_tags_mt0 interval(10a) #sql select top(c6, 3) from select_tags_mt0 interval(10a) group by tbname; +sql select top(c6, 10) from select_tags_mt0 interval(10a); +if $rows != 12800 then + return -1 +endi + sql select top(c1, 100), tbname, t1, t2 from select_tags_mt0; if $rows != 100 then return -1