From 1db0c05529a632856cc3c9a2ca55fc95ee8dfc94 Mon Sep 17 00:00:00 2001 From: Haojun Liao Date: Fri, 2 Dec 2022 21:48:32 +0800 Subject: [PATCH 1/8] fix(query): check null ptr before dereferencing it. --- source/dnode/mnode/impl/src/mndUser.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/source/dnode/mnode/impl/src/mndUser.c b/source/dnode/mnode/impl/src/mndUser.c index 9f4b9fc2de..685e1ece68 100644 --- a/source/dnode/mnode/impl/src/mndUser.c +++ b/source/dnode/mnode/impl/src/mndUser.c @@ -168,6 +168,8 @@ _OVER: static SSdbRow *mndUserActionDecode(SSdbRaw *pRaw) { terrno = TSDB_CODE_OUT_OF_MEMORY; + SUserObj *pUser = NULL; // fix the un-initialized local parameter error, caused by log print + SSdbRow *pRow = NULL; int8_t sver = 0; if (sdbGetRawSoftVer(pRaw, &sver) != 0) goto _OVER; @@ -177,10 +179,10 @@ static SSdbRow *mndUserActionDecode(SSdbRaw *pRaw) { goto _OVER; } - SSdbRow *pRow = sdbAllocRow(sizeof(SUserObj)); + pRow = sdbAllocRow(sizeof(SUserObj)); if (pRow == NULL) goto _OVER; - SUserObj *pUser = sdbGetRowObj(pRow); + pUser = sdbGetRowObj(pRow); if (pUser == NULL) goto _OVER; int32_t dataPos = 0; @@ -225,9 +227,12 @@ static SSdbRow *mndUserActionDecode(SSdbRaw *pRaw) { _OVER: if (terrno != 0) { - mError("user:%s, failed to decode from raw:%p since %s", pUser->user, pRaw, terrstr()); - taosHashCleanup(pUser->readDbs); - taosHashCleanup(pUser->writeDbs); + if (pUser != NULL) { + mError("user:%s, failed to decode from raw:%p since %s", pUser->user, pRaw, terrstr()); + taosHashCleanup(pUser->readDbs); + taosHashCleanup(pUser->writeDbs); + } + taosMemoryFreeClear(pRow); return NULL; } From 4a121d40d6490b5c78c0326635d6010c9a86c013 Mon Sep 17 00:00:00 2001 From: Haojun Liao Date: Sat, 3 Dec 2022 13:47:51 +0800 Subject: [PATCH 2/8] fix(query): fix syntax error. --- source/libs/executor/src/executorimpl.c | 1 + source/libs/executor/src/scanoperator.c | 8 +++++--- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/source/libs/executor/src/executorimpl.c b/source/libs/executor/src/executorimpl.c index b408e32061..ea4b37cf0b 100644 --- a/source/libs/executor/src/executorimpl.c +++ b/source/libs/executor/src/executorimpl.c @@ -1580,6 +1580,7 @@ void destroyOperatorInfo(SOperatorInfo* pOperator) { int32_t optrDefaultBufFn(SOperatorInfo* pOperator) { if (pOperator->blocking) { ASSERT(0); + return 0; } else { return 0; } diff --git a/source/libs/executor/src/scanoperator.c b/source/libs/executor/src/scanoperator.c index 0d245f0815..54d7182ac3 100644 --- a/source/libs/executor/src/scanoperator.c +++ b/source/libs/executor/src/scanoperator.c @@ -3032,8 +3032,10 @@ void fillTableCountScanDataBlock(STableCountScanSupp* pSupp, char* dbName, char* if (pSupp->dbNameSlotId != -1) { ASSERT(strlen(dbName)); SColumnInfoData* colInfoData = taosArrayGet(pRes->pDataBlock, pSupp->dbNameSlotId); - char varDbName[TSDB_DB_NAME_LEN + VARSTR_HEADER_SIZE] = {0}; - strncpy(varDataVal(varDbName), dbName, strlen(dbName)); + + char varDbName[TSDB_DB_NAME_LEN + VARSTR_HEADER_SIZE] = {0}; + tstrncpy(varDataVal(varDbName), dbName, TSDB_DB_NAME_LEN); + varDataSetLen(varDbName, strlen(dbName)); colDataAppend(colInfoData, 0, varDbName, false); } @@ -3042,7 +3044,7 @@ void fillTableCountScanDataBlock(STableCountScanSupp* pSupp, char* dbName, char* SColumnInfoData* colInfoData = taosArrayGet(pRes->pDataBlock, pSupp->stbNameSlotId); if (strlen(stbName) != 0) { char varStbName[TSDB_TABLE_NAME_LEN + VARSTR_HEADER_SIZE] = {0}; - strncpy(varDataVal(varStbName), stbName, strlen(stbName)); + strncpy(varDataVal(varStbName), stbName, TSDB_TABLE_NAME_LEN); varDataSetLen(varStbName, strlen(stbName)); colDataAppend(colInfoData, 0, varStbName, false); } else { From 136937d69a52983762b33f1c730da254bedd309e Mon Sep 17 00:00:00 2001 From: Haojun Liao Date: Mon, 5 Dec 2022 17:05:07 +0800 Subject: [PATCH 3/8] fix(query): insert null column sma firstly. --- source/dnode/vnode/src/tsdb/tsdbRead.c | 38 ++++++++++++++++++++------ 1 file changed, 29 insertions(+), 9 deletions(-) diff --git a/source/dnode/vnode/src/tsdb/tsdbRead.c b/source/dnode/vnode/src/tsdb/tsdbRead.c index 5dd9293118..77a2306fce 100644 --- a/source/dnode/vnode/src/tsdb/tsdbRead.c +++ b/source/dnode/vnode/src/tsdb/tsdbRead.c @@ -4055,6 +4055,29 @@ void tsdbRetrieveDataBlockInfo(const STsdbReader* pReader, int32_t* rows, uint64 } } + +static void doFillNullColSMA(SBlockLoadSuppInfo* pSup, int32_t numOfRows, int32_t numOfCols) { + // do fill all null column value SMA info + int32_t i = 0, j = 0; + int32_t size = (int32_t) taosArrayGetSize(pSup->pColAgg); + + while (j < numOfCols && i < size) { + SColumnDataAgg* pAgg = taosArrayGet(pSup->pColAgg, i); + if (pAgg->colId == pSup->colId[j]) { + i += 1; + j += 1; + } else if (pAgg->colId < pSup->colId[j]) { + i += 1; + } else if (pSup->colId[j] < pAgg->colId) { + if (pSup->colId[j] != PRIMARYKEY_TIMESTAMP_COL_ID) { + SColumnDataAgg nullColAgg = {.colId = pSup->colId[j], .numOfNull = numOfRows}; + taosArrayPush(pSup->pColAgg, &nullColAgg); + } + j += 1; + } + } +} + int32_t tsdbRetrieveDatablockSMA(STsdbReader* pReader, SColumnDataAgg ***pBlockSMA, bool* allHave) { int32_t code = 0; *allHave = false; @@ -4110,6 +4133,10 @@ int32_t tsdbRetrieveDatablockSMA(STsdbReader* pReader, SColumnDataAgg ***pBlockS pResBlock->pBlockAgg = taosMemoryCalloc(num, sizeof(SColumnDataAgg)); } + // do fill all null column value SMA info + doFillNullColSMA(pSup, pBlock->nRow, numOfCols); + + i = 0, j = 0; while (j < numOfCols && i < size) { SColumnDataAgg* pAgg = taosArrayGet(pSup->pColAgg, i); if (pAgg->colId == pSup->colId[j]) { @@ -4119,15 +4146,8 @@ int32_t tsdbRetrieveDatablockSMA(STsdbReader* pReader, SColumnDataAgg ***pBlockS } else if (pAgg->colId < pSup->colId[j]) { i += 1; } else if (pSup->colId[j] < pAgg->colId) { - if (pSup->colId[j] == PRIMARYKEY_TIMESTAMP_COL_ID) { - pResBlock->pBlockAgg[pSup->slotId[j]] = &pSup->tsColAgg; - } else { - // all date in this block are null - SColumnDataAgg nullColAgg = {.colId = pSup->colId[j], .numOfNull = pBlock->nRow}; - taosArrayPush(pSup->pColAgg, &nullColAgg); - - pResBlock->pBlockAgg[pSup->slotId[j]] = taosArrayGetLast(pSup->pColAgg); - } + ASSERT(pSup->colId[j] == PRIMARYKEY_TIMESTAMP_COL_ID); + pResBlock->pBlockAgg[pSup->slotId[j]] = &pSup->tsColAgg; j += 1; } } From 9e04f42635524535a2d8658c74ee55edacf0dc81 Mon Sep 17 00:00:00 2001 From: Haojun Liao Date: Mon, 5 Dec 2022 20:20:48 +0800 Subject: [PATCH 4/8] fix(query): check for neighbor block when merging data block. --- source/dnode/vnode/src/tsdb/tsdbRead.c | 34 +++++++++++++++++++++++++- 1 file changed, 33 insertions(+), 1 deletion(-) diff --git a/source/dnode/vnode/src/tsdb/tsdbRead.c b/source/dnode/vnode/src/tsdb/tsdbRead.c index 77a2306fce..fbf3327290 100644 --- a/source/dnode/vnode/src/tsdb/tsdbRead.c +++ b/source/dnode/vnode/src/tsdb/tsdbRead.c @@ -214,6 +214,7 @@ static bool hasDataInLastBlock(SLastBlockReader* pLastBlockReader); static int32_t doBuildDataBlock(STsdbReader* pReader); static TSDBKEY getCurrentKeyInBuf(STableBlockScanInfo* pScanInfo, STsdbReader* pReader); static bool hasDataInFileBlock(const SBlockData* pBlockData, const SFileBlockDumpInfo* pDumpInfo); +static void initBlockDumpInfo(STsdbReader* pReader, SDataBlockIter* pBlockIter); static bool outOfTimeWindow(int64_t ts, STimeWindow* pWindow) { return (ts > pWindow->ekey) || (ts < pWindow->skey); } @@ -2477,6 +2478,37 @@ static int32_t buildComposedDataBlock(STsdbReader* pReader) { SDataBlk* pBlock = getCurrentBlock(&pReader->status.blockIter); if (pDumpInfo->rowIndex >= pBlock->nRow || pDumpInfo->rowIndex < 0) { + + int32_t nextIndex = -1; + SBlockIndex bIndex = {0}; + bool hasNeighbor = getNeighborBlockOfSameTable(pBlockInfo, pBlockScanInfo, &nextIndex, pReader->order, &bIndex); + if (!hasNeighbor) { // do nothing + return 0; + } + + bool overlap = overlapWithNeighborBlock(pBlock, &bIndex, pReader->order); + + if (overlap) { // load next block + SReaderStatus* pStatus = &pReader->status; + SDataBlockIter* pBlockIter = &pStatus->blockIter; + + // 1. find the next neighbor block in the scan block list + SFileDataBlockInfo fb = {.uid = pBlockInfo->uid, .tbBlockIdx = nextIndex}; + int32_t neighborIndex = findFileBlockInfoIndex(pBlockIter, &fb); + + // 2. remove it from the scan block list + setFileBlockActiveInBlockIter(pBlockIter, neighborIndex, step); + + // 3. load the neighbor block, and set it to be the currently accessed file data block + code = doLoadFileBlockData(pReader, pBlockIter, &pStatus->fileBlockData, pBlockInfo->uid); + if (code != TSDB_CODE_SUCCESS) { + return code; + } + + // 4. check the data values + initBlockDumpInfo(pReader, pBlockIter); + } + setBlockAllDumped(pDumpInfo, pBlock->maxKey.ts, pReader->order); break; } @@ -2888,7 +2920,7 @@ static int32_t buildBlockFromBufferSequentially(STsdbReader* pReader) { } // set the correct start position in case of the first/last file block, according to the query time window -static void initBlockDumpInfo(STsdbReader* pReader, SDataBlockIter* pBlockIter) { +void initBlockDumpInfo(STsdbReader* pReader, SDataBlockIter* pBlockIter) { SDataBlk* pBlock = getCurrentBlock(pBlockIter); SReaderStatus* pStatus = &pReader->status; From d5b493677b7f9e74c5f08a1c403a8ce93d501eae Mon Sep 17 00:00:00 2001 From: Haojun Liao Date: Mon, 5 Dec 2022 20:39:37 +0800 Subject: [PATCH 5/8] fix(query): set column sma into the correct position. --- source/dnode/vnode/src/tsdb/tsdbRead.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/source/dnode/vnode/src/tsdb/tsdbRead.c b/source/dnode/vnode/src/tsdb/tsdbRead.c index fbf3327290..7717a2371b 100644 --- a/source/dnode/vnode/src/tsdb/tsdbRead.c +++ b/source/dnode/vnode/src/tsdb/tsdbRead.c @@ -4088,10 +4088,12 @@ void tsdbRetrieveDataBlockInfo(const STsdbReader* pReader, int32_t* rows, uint64 } -static void doFillNullColSMA(SBlockLoadSuppInfo* pSup, int32_t numOfRows, int32_t numOfCols) { +static void doFillNullColSMA(SBlockLoadSuppInfo* pSup, int32_t numOfRows, int32_t numOfCols, + SColumnDataAgg* pTsAgg) { // do fill all null column value SMA info int32_t i = 0, j = 0; int32_t size = (int32_t) taosArrayGetSize(pSup->pColAgg); + taosArrayInsert(pSup->pColAgg, 0, pTsAgg); while (j < numOfCols && i < size) { SColumnDataAgg* pAgg = taosArrayGet(pSup->pColAgg, i); @@ -4103,7 +4105,7 @@ static void doFillNullColSMA(SBlockLoadSuppInfo* pSup, int32_t numOfRows, int32_ } else if (pSup->colId[j] < pAgg->colId) { if (pSup->colId[j] != PRIMARYKEY_TIMESTAMP_COL_ID) { SColumnDataAgg nullColAgg = {.colId = pSup->colId[j], .numOfNull = numOfRows}; - taosArrayPush(pSup->pColAgg, &nullColAgg); + taosArrayInsert(pSup->pColAgg, i ,&nullColAgg); } j += 1; } @@ -4166,7 +4168,7 @@ int32_t tsdbRetrieveDatablockSMA(STsdbReader* pReader, SColumnDataAgg ***pBlockS } // do fill all null column value SMA info - doFillNullColSMA(pSup, pBlock->nRow, numOfCols); + doFillNullColSMA(pSup, pBlock->nRow, numOfCols, pTsAgg); i = 0, j = 0; while (j < numOfCols && i < size) { From 114943ddb00a7c4cb618c51559ed4e24b50681de Mon Sep 17 00:00:00 2001 From: Haojun Liao Date: Mon, 5 Dec 2022 22:28:26 +0800 Subject: [PATCH 6/8] fix(query): set the correct flag when no neighbors exist. --- source/dnode/vnode/src/tsdb/tsdbRead.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/source/dnode/vnode/src/tsdb/tsdbRead.c b/source/dnode/vnode/src/tsdb/tsdbRead.c index 7717a2371b..a4581f5472 100644 --- a/source/dnode/vnode/src/tsdb/tsdbRead.c +++ b/source/dnode/vnode/src/tsdb/tsdbRead.c @@ -2483,12 +2483,11 @@ static int32_t buildComposedDataBlock(STsdbReader* pReader) { SBlockIndex bIndex = {0}; bool hasNeighbor = getNeighborBlockOfSameTable(pBlockInfo, pBlockScanInfo, &nextIndex, pReader->order, &bIndex); if (!hasNeighbor) { // do nothing - return 0; + setBlockAllDumped(pDumpInfo, pBlock->maxKey.ts, pReader->order); + break; } - bool overlap = overlapWithNeighborBlock(pBlock, &bIndex, pReader->order); - - if (overlap) { // load next block + if (overlapWithNeighborBlock(pBlock, &bIndex, pReader->order)) { // load next block SReaderStatus* pStatus = &pReader->status; SDataBlockIter* pBlockIter = &pStatus->blockIter; @@ -2502,15 +2501,16 @@ static int32_t buildComposedDataBlock(STsdbReader* pReader) { // 3. load the neighbor block, and set it to be the currently accessed file data block code = doLoadFileBlockData(pReader, pBlockIter, &pStatus->fileBlockData, pBlockInfo->uid); if (code != TSDB_CODE_SUCCESS) { - return code; + setBlockAllDumped(pDumpInfo, pBlock->maxKey.ts, pReader->order); + break; } // 4. check the data values initBlockDumpInfo(pReader, pBlockIter); + } else { + setBlockAllDumped(pDumpInfo, pBlock->maxKey.ts, pReader->order); + break; } - - setBlockAllDumped(pDumpInfo, pBlock->maxKey.ts, pReader->order); - break; } } } From a693b9a2d6ad0e5831cf22d8a6862a9cf24758fd Mon Sep 17 00:00:00 2001 From: Haojun Liao Date: Tue, 6 Dec 2022 15:29:45 +0800 Subject: [PATCH 7/8] fix(query):remove invalid destroy appinst operation. --- source/client/src/clientEnv.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/source/client/src/clientEnv.c b/source/client/src/clientEnv.c index 93398d337d..d27757119a 100644 --- a/source/client/src/clientEnv.c +++ b/source/client/src/clientEnv.c @@ -231,10 +231,9 @@ void destroyTscObj(void *pObj) { tscDebug("connObj 0x%" PRIx64 " p:%p destroyed, remain inst totalConn:%" PRId64, pTscObj->id, pTscObj, pTscObj->pAppInfo->numOfConns); - int64_t connNum = atomic_sub_fetch_64(&pTscObj->pAppInfo->numOfConns, 1); - if (0 == connNum) { - destroyAppInst(pTscObj->pAppInfo); - } + // In any cases, we should not free app inst here. Or an race condition rise. + /*int64_t connNum = */atomic_sub_fetch_64(&pTscObj->pAppInfo->numOfConns, 1); + taosThreadMutexDestroy(&pTscObj->mutex); taosMemoryFree(pTscObj); From 2f1afb9bf0fce767b4b13e8cef55c01d019f8b16 Mon Sep 17 00:00:00 2001 From: Haojun Liao Date: Tue, 6 Dec 2022 15:30:08 +0800 Subject: [PATCH 8/8] fix(query):fix typo --- source/client/src/clientEnv.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/client/src/clientEnv.c b/source/client/src/clientEnv.c index d27757119a..7564d91330 100644 --- a/source/client/src/clientEnv.c +++ b/source/client/src/clientEnv.c @@ -231,7 +231,7 @@ void destroyTscObj(void *pObj) { tscDebug("connObj 0x%" PRIx64 " p:%p destroyed, remain inst totalConn:%" PRId64, pTscObj->id, pTscObj, pTscObj->pAppInfo->numOfConns); - // In any cases, we should not free app inst here. Or an race condition rise. + // In any cases, we should not free app inst here. Or an race condition rises. /*int64_t connNum = */atomic_sub_fetch_64(&pTscObj->pAppInfo->numOfConns, 1); taosThreadMutexDestroy(&pTscObj->mutex);