From feb1c8518e28aa04336f45087300049bef08c420 Mon Sep 17 00:00:00 2001 From: dapan1121 Date: Thu, 29 Aug 2024 15:08:38 +0800 Subject: [PATCH 1/3] fix: remove asserts --- include/common/tdatablock.h | 10 ---- source/common/src/tdatablock.c | 70 ++++++++++++++++++------ source/libs/executor/inc/executil.h | 1 - source/libs/executor/inc/mergejoin.h | 2 - source/libs/executor/src/groupoperator.c | 5 ++ source/libs/executor/src/tlinearhash.c | 2 +- source/libs/executor/src/tsort.c | 7 +++ source/libs/nodes/src/nodesUtilFuncs.c | 2 - source/libs/planner/src/planOptimizer.c | 2 - 9 files changed, 66 insertions(+), 35 deletions(-) diff --git a/include/common/tdatablock.h b/include/common/tdatablock.h index 080416ca2e..b4dd6d61e4 100644 --- a/include/common/tdatablock.h +++ b/include/common/tdatablock.h @@ -113,10 +113,8 @@ static FORCE_INLINE bool colDataIsNull(const SColumnInfoData* pColumnInfoData, u if (pColAgg != NULL && pColAgg->colId != -1) { if (pColAgg->numOfNull == totalRows) { - ASSERT(pColumnInfoData->nullbitmap == NULL); return true; } else if (pColAgg->numOfNull == 0) { - ASSERT(pColumnInfoData->nullbitmap == NULL); return false; } } @@ -159,40 +157,32 @@ static FORCE_INLINE void colDataSetNNULL(SColumnInfoData* pColumnInfoData, uint3 } static FORCE_INLINE void colDataSetInt8(SColumnInfoData* pColumnInfoData, uint32_t rowIndex, int8_t* v) { - ASSERT(pColumnInfoData->info.type == TSDB_DATA_TYPE_TINYINT || - pColumnInfoData->info.type == TSDB_DATA_TYPE_UTINYINT || pColumnInfoData->info.type == TSDB_DATA_TYPE_BOOL); char* p = pColumnInfoData->pData + pColumnInfoData->info.bytes * rowIndex; *(int8_t*)p = *(int8_t*)v; } static FORCE_INLINE void colDataSetInt16(SColumnInfoData* pColumnInfoData, uint32_t rowIndex, int16_t* v) { - ASSERT(pColumnInfoData->info.type == TSDB_DATA_TYPE_SMALLINT || - pColumnInfoData->info.type == TSDB_DATA_TYPE_USMALLINT); char* p = pColumnInfoData->pData + pColumnInfoData->info.bytes * rowIndex; *(int16_t*)p = *(int16_t*)v; } static FORCE_INLINE void colDataSetInt32(SColumnInfoData* pColumnInfoData, uint32_t rowIndex, int32_t* v) { - ASSERT(pColumnInfoData->info.type == TSDB_DATA_TYPE_INT || pColumnInfoData->info.type == TSDB_DATA_TYPE_UINT); char* p = pColumnInfoData->pData + pColumnInfoData->info.bytes * rowIndex; *(int32_t*)p = *(int32_t*)v; } static FORCE_INLINE void colDataSetInt64(SColumnInfoData* pColumnInfoData, uint32_t rowIndex, int64_t* v) { int32_t type = pColumnInfoData->info.type; - ASSERT(type == TSDB_DATA_TYPE_BIGINT || type == TSDB_DATA_TYPE_UBIGINT || type == TSDB_DATA_TYPE_TIMESTAMP); char* p = pColumnInfoData->pData + pColumnInfoData->info.bytes * rowIndex; *(int64_t*)p = *(int64_t*)v; } static FORCE_INLINE void colDataSetFloat(SColumnInfoData* pColumnInfoData, uint32_t rowIndex, float* v) { - ASSERT(pColumnInfoData->info.type == TSDB_DATA_TYPE_FLOAT); char* p = pColumnInfoData->pData + pColumnInfoData->info.bytes * rowIndex; *(float*)p = *(float*)v; } static FORCE_INLINE void colDataSetDouble(SColumnInfoData* pColumnInfoData, uint32_t rowIndex, double* v) { - ASSERT(pColumnInfoData->info.type == TSDB_DATA_TYPE_DOUBLE); char* p = pColumnInfoData->pData + pColumnInfoData->info.bytes * rowIndex; *(double*)p = *(double*)v; } diff --git a/source/common/src/tdatablock.c b/source/common/src/tdatablock.c index 16710d9555..264cde00ee 100644 --- a/source/common/src/tdatablock.c +++ b/source/common/src/tdatablock.c @@ -81,7 +81,7 @@ int32_t getJsonValueLen(const char* data) { } else if (tTagIsJson(data)) { // json string dataLen = ((STag*)(data))->len; } else { - ASSERT(0); + uError("Invalid data type:%d in Json", *data); } return dataLen; } @@ -801,7 +801,7 @@ int32_t blockDataSplitRows(SSDataBlock* pBlock, bool hasVarCol, int32_t startInd size_t rowSize = blockDataGetRowSize(pBlock); int32_t capacity = blockDataGetCapacityInRow(pBlock, pageSize, headerSize + colHeaderSize); if (capacity <= 0) { - return TSDB_CODE_FAILED; + return terrno; } *stopIndex = startIndex + capacity - 1; @@ -835,7 +835,7 @@ int32_t blockDataSplitRows(SSDataBlock* pBlock, bool hasVarCol, int32_t startInd if (size > pageSize) { // pageSize must be able to hold one row *stopIndex = j - 1; if (*stopIndex < startIndex) { - return TSDB_CODE_FAILED; + return TSDB_CODE_QRY_EXECUTOR_INTERNAL_ERROR; } return TSDB_CODE_SUCCESS; @@ -2060,7 +2060,7 @@ int32_t blockDataAppendColInfo(SSDataBlock* pBlock, SColumnInfoData* pColInfoDat } // todo disable it temporarily - // ASSERT(pColInfoData->info.type != 0); + // A S S E R T(pColInfoData->info.type != 0); if (IS_VAR_DATA_TYPE(pColInfoData->info.type)) { pBlock->info.hasVarCol = true; } @@ -2100,14 +2100,18 @@ size_t blockDataGetCapacityInRow(const SSDataBlock* pBlock, size_t pageSize, int int32_t payloadSize = pageSize - extraSize; int32_t rowSize = pBlock->info.rowSize; int32_t nRows = payloadSize / rowSize; - ASSERT(nRows >= 1); + if (nRows < 1) { + qError("rows %d in page is too small, payloadSize:%d, rowSize:%d", nRows, payloadSize, rowSize); + terrno = TSDB_CODE_QRY_EXECUTOR_INTERNAL_ERROR; + return -1; + } int32_t numVarCols = 0; int32_t numFixCols = 0; for (int32_t i = 0; i < numOfCols; ++i) { SColumnInfoData* pCol = taosArrayGet(pBlock->pDataBlock, i); if (pCol == NULL) { - return terrno; + return -1; } if (IS_VAR_DATA_TYPE(pCol->info.type)) { @@ -2135,7 +2139,11 @@ size_t blockDataGetCapacityInRow(const SSDataBlock* pBlock, size_t pageSize, int int32_t newRows = (result != -1) ? result - 1 : nRows; // the true value must be less than the value of nRows - ASSERT(newRows <= nRows && newRows >= 1); + if (newRows > nRows || newRows < 1) { + uError("invalid newRows:%d, nRows:%d", newRows, nRows); + terrno = TSDB_CODE_QRY_EXECUTOR_INTERNAL_ERROR; + return -1; + } return newRows; } @@ -2616,7 +2624,11 @@ int32_t buildSubmitReqFromDataBlock(SSubmitReq2** ppReq, const SSDataBlock* pDat } // the rsma result should has the same column number with schema. - ASSERT(colNum == pTSchema->numOfCols); + if (colNum != pTSchema->numOfCols) { + uError("colNum %d is not equal to numOfCols %d", colNum, pTSchema->numOfCols); + code = TSDB_CODE_QRY_EXECUTOR_INTERNAL_ERROR; + goto _end; + } SSubmitTbData tbData = {0}; @@ -2652,10 +2664,18 @@ int32_t buildSubmitReqFromDataBlock(SSubmitReq2** ppReq, const SSDataBlock* pDat switch (pColInfoData->info.type) { case TSDB_DATA_TYPE_TIMESTAMP: - ASSERT(pColInfoData->info.type == pCol->type); + if (pColInfoData->info.type != pCol->type) { + uError("colType:%d mismatch with sechma colType:%d", pColInfoData->info.type, pCol->type); + terrno = TSDB_CODE_QRY_EXECUTOR_INTERNAL_ERROR; + return terrno; + } if (!isStartKey) { isStartKey = true; - ASSERT(PRIMARYKEY_TIMESTAMP_COL_ID == pCol->colId); + if (PRIMARYKEY_TIMESTAMP_COL_ID != pCol->colId) { + uError("the first timestamp colId %d is not primary colId", pCol->colId); + terrno = TSDB_CODE_QRY_EXECUTOR_INTERNAL_ERROR; + return terrno; + } SColVal cv = COL_VAL_VALUE(pCol->colId, ((SValue){.type = pCol->type, .val = *(TSKEY*)var})); void* px = taosArrayPush(pVals, &cv); if (px == NULL) { @@ -2679,7 +2699,11 @@ int32_t buildSubmitReqFromDataBlock(SSubmitReq2** ppReq, const SSDataBlock* pDat case TSDB_DATA_TYPE_NCHAR: case TSDB_DATA_TYPE_VARBINARY: case TSDB_DATA_TYPE_VARCHAR: { // TSDB_DATA_TYPE_BINARY - ASSERT(pColInfoData->info.type == pCol->type); + if (pColInfoData->info.type != pCol->type) { + uError("colType:%d mismatch with sechma colType:%d", pColInfoData->info.type, pCol->type); + terrno = TSDB_CODE_QRY_EXECUTOR_INTERNAL_ERROR; + return terrno; + } if (colDataIsNull_s(pColInfoData, j)) { SColVal cv = COL_VAL_NULL(pCol->colId, pCol->type); void* px = taosArrayPush(pVals, &cv); @@ -2704,7 +2728,8 @@ int32_t buildSubmitReqFromDataBlock(SSubmitReq2** ppReq, const SSDataBlock* pDat case TSDB_DATA_TYPE_JSON: case TSDB_DATA_TYPE_MEDIUMBLOB: uError("the column type %" PRIi16 " is defined but not implemented yet", pColInfoData->info.type); - ASSERT(0); + terrno = TSDB_CODE_QRY_EXECUTOR_INTERNAL_ERROR; + return terrno; break; default: if (pColInfoData->info.type < TSDB_DATA_TYPE_MAX && pColInfoData->info.type > TSDB_DATA_TYPE_NULL) { @@ -2752,7 +2777,8 @@ int32_t buildSubmitReqFromDataBlock(SSubmitReq2** ppReq, const SSDataBlock* pDat } } else { uError("the column type %" PRIi16 " is undefined\n", pColInfoData->info.type); - ASSERT(0); + terrno = TSDB_CODE_QRY_EXECUTOR_INTERNAL_ERROR; + return terrno; } break; } @@ -2763,7 +2789,6 @@ int32_t buildSubmitReqFromDataBlock(SSubmitReq2** ppReq, const SSDataBlock* pDat goto _end; } - ASSERT(pRow); void* px = taosArrayPush(tbData.aRowP, &pRow); if (px == NULL) { code = terrno; @@ -2902,7 +2927,10 @@ int32_t blockEncode(const SSDataBlock* pBlock, char* data, int32_t numOfCols) { int32_t* rows = (int32_t*)data; *rows = pBlock->info.rows; data += sizeof(int32_t); - ASSERT(*rows > 0); + if (*rows <= 0) { + uError("Invalid rows %d in block", *rows); + terrno = TSDB_CODE_QRY_EXECUTOR_INTERNAL_ERROR; + } int32_t* cols = (int32_t*)data; *cols = numOfCols; @@ -3055,7 +3083,11 @@ int32_t blockDecode(SSDataBlock* pBlock, const char* pData, const char** pEndPos for (int32_t i = 0; i < numOfCols; ++i) { colLen[i] = htonl(colLen[i]); - ASSERT(colLen[i] >= 0); + if (colLen[i] < 0) { + uError("block decode colLen:%d error, colIdx:%d", colLen[i], i); + terrno = TSDB_CODE_QRY_EXECUTOR_INTERNAL_ERROR; + return terrno; + } SColumnInfoData* pColInfoData = taosArrayGet(pBlock->pDataBlock, i); if (pColInfoData == NULL) { @@ -3099,7 +3131,11 @@ int32_t blockDecode(SSDataBlock* pBlock, const char* pData, const char** pEndPos pBlock->info.dataLoad = 1; pBlock->info.rows = numOfRows; pBlock->info.blankFill = blankFill; - ASSERT(pStart - pData == dataLen); + if (pStart - pData != dataLen) { + uError("block decode msg len error, pStart:%p, pData:%p, dataLen:%d", pStart, pData, dataLen); + terrno = TSDB_CODE_QRY_EXECUTOR_INTERNAL_ERROR; + return terrno; + } *pEndPos = pStart; return code; diff --git a/source/libs/executor/inc/executil.h b/source/libs/executor/inc/executil.h index e35b26627b..95035dd96f 100644 --- a/source/libs/executor/inc/executil.h +++ b/source/libs/executor/inc/executil.h @@ -32,7 +32,6 @@ #define SET_RES_WINDOW_KEY(_k, _ori, _len, _uid) \ do { \ - assert(sizeof(_uid) == sizeof(uint64_t)); \ *(uint64_t*)(_k) = (_uid); \ (void)memcpy((_k) + sizeof(uint64_t), (_ori), (_len)); \ } while (0) diff --git a/source/libs/executor/inc/mergejoin.h b/source/libs/executor/inc/mergejoin.h index 64db1a57a0..ceb0037b8d 100755 --- a/source/libs/executor/inc/mergejoin.h +++ b/source/libs/executor/inc/mergejoin.h @@ -357,7 +357,6 @@ typedef struct SMJoinOperatorInfo { #define MJOIN_PUSH_BLK_TO_CACHE(_cache, _blk) \ do { \ - ASSERT(taosArrayGetSize((_cache)->grps) <= 1); \ SMJoinGrpRows* pGrp = (SMJoinGrpRows*)taosArrayReserve((_cache)->grps, 1); \ (_cache)->rowNum += (_blk)->info.rows; \ pGrp->blk = (_blk); \ @@ -381,7 +380,6 @@ typedef struct SMJoinOperatorInfo { do { \ SMJoinGrpRows* pGrp = taosArrayGet((_cache)->grps, 0); \ if (NULL != pGrp) { \ - ASSERT(pGrp->blk == (_tb)->blk); \ pGrp->beginIdx = (_tb)->blkRowIdx; \ pGrp->readIdx = pGrp->beginIdx; \ } \ diff --git a/source/libs/executor/src/groupoperator.c b/source/libs/executor/src/groupoperator.c index 3f203e7a95..6a7bb7c6be 100644 --- a/source/libs/executor/src/groupoperator.c +++ b/source/libs/executor/src/groupoperator.c @@ -1221,6 +1221,11 @@ int32_t createPartitionOperatorInfo(SOperatorInfo* downstream, SPartitionPhysiNo pInfo->rowCapacity = blockDataGetCapacityInRow(pInfo->binfo.pRes, getBufPageSize(pInfo->pBuf), blockDataGetSerialMetaSize(taosArrayGetSize(pInfo->binfo.pRes->pDataBlock))); + if (pInfo->rowCapacity < 0) { + code = terrno; + goto _error; + } + pInfo->columnOffset = setupColumnOffset(pInfo->binfo.pRes, pInfo->rowCapacity); QUERY_CHECK_NULL(pInfo->columnOffset, code, lino, _error, terrno); diff --git a/source/libs/executor/src/tlinearhash.c b/source/libs/executor/src/tlinearhash.c index 55628c18f9..83716d72ad 100644 --- a/source/libs/executor/src/tlinearhash.c +++ b/source/libs/executor/src/tlinearhash.c @@ -183,7 +183,7 @@ static void doTrimBucketPages(SLHashObj* pHashObj, SLHashBucket* pBucket) { setBufPageDirty(pFirst, true); setBufPageDirty(pLast, true); - // ASSERT(pLast->num >= nodeSize + sizeof(SFilePage)); + // A S S E R T(pLast->num >= nodeSize + sizeof(SFilePage)); pFirst->num += nodeSize; pLast->num -= nodeSize; diff --git a/source/libs/executor/src/tsort.c b/source/libs/executor/src/tsort.c index 621d643361..fa1ccc3100 100644 --- a/source/libs/executor/src/tsort.c +++ b/source/libs/executor/src/tsort.c @@ -970,6 +970,10 @@ static int32_t doInternalMergeSort(SSortHandle* pHandle) { int32_t numOfRows = blockDataGetCapacityInRow(pHandle->pDataBlock, pHandle->pageSize, blockDataGetSerialMetaSize(taosArrayGetSize(pHandle->pDataBlock->pDataBlock))); + if (numOfRows < 0) { + return terrno; + } + int32_t code = blockDataEnsureCapacity(pHandle->pDataBlock, numOfRows); if (code) { return code; @@ -1999,6 +2003,9 @@ static int32_t sortBlocksToExtSource(SSortHandle* pHandle, SArray* aBlk, SArray* int32_t code = TSDB_CODE_SUCCESS; int32_t pageHeaderSize = sizeof(int32_t) + sizeof(int32_t) * blockDataGetNumOfCols(pHandle->pDataBlock); int32_t rowCap = blockDataGetCapacityInRow(pHandle->pDataBlock, pHandle->pageSize, pageHeaderSize); + if (rowCap < 0) { + return terrno; + } code = blockDataEnsureCapacity(pHandle->pDataBlock, rowCap); if (code) { diff --git a/source/libs/nodes/src/nodesUtilFuncs.c b/source/libs/nodes/src/nodesUtilFuncs.c index c1afc4afb3..7bddc068e3 100644 --- a/source/libs/nodes/src/nodesUtilFuncs.c +++ b/source/libs/nodes/src/nodesUtilFuncs.c @@ -1089,8 +1089,6 @@ void nodesDestroyNode(SNode* pNode) { if (pStmt->destroyParseFileCxt) { pStmt->destroyParseFileCxt(&pStmt->pParFileCxt); } - - assert(TSDB_CODE_SUCCESS == taosCloseFile(&pStmt->fp)); break; } case QUERY_NODE_CREATE_DATABASE_STMT: diff --git a/source/libs/planner/src/planOptimizer.c b/source/libs/planner/src/planOptimizer.c index d48871fd70..3aab190eda 100644 --- a/source/libs/planner/src/planOptimizer.c +++ b/source/libs/planner/src/planOptimizer.c @@ -6578,7 +6578,6 @@ static bool tsmaOptMayBeOptimized(SLogicNode* pNode, void* pCtx) { return false; } - assert(pFuncs); FOREACH(pTmpNode, pFuncs) { SFunctionNode* pFunc = (SFunctionNode*)pTmpNode; if (!fmIsTSMASupportedFunc(pFunc->funcId) && !fmIsPseudoColumnFunc(pFunc->funcId) && @@ -7271,7 +7270,6 @@ static int32_t tsmaOptRewriteParent(STSMAOptCtx* pTsmaOptCtx, SLogicNode* pParen if (code == TSDB_CODE_SUCCESS && pWindow) { SColumnNode* pCol = (SColumnNode*)pScan->pScanCols->pTail->pNode; - assert(pCol->colId == PRIMARYKEY_TIMESTAMP_COL_ID); nodesDestroyNode(pWindow->pTspk); pWindow->pTspk = NULL; code = nodesCloneNode((SNode*)pCol, &pWindow->pTspk); From 99102d47ac65dfa957da6adaba9489de1e6bdaed Mon Sep 17 00:00:00 2001 From: dapan1121 Date: Thu, 29 Aug 2024 15:54:40 +0800 Subject: [PATCH 2/3] fix: compile issue --- source/common/src/tdatablock.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/common/src/tdatablock.c b/source/common/src/tdatablock.c index 264cde00ee..b21dd8c803 100644 --- a/source/common/src/tdatablock.c +++ b/source/common/src/tdatablock.c @@ -2101,7 +2101,7 @@ size_t blockDataGetCapacityInRow(const SSDataBlock* pBlock, size_t pageSize, int int32_t rowSize = pBlock->info.rowSize; int32_t nRows = payloadSize / rowSize; if (nRows < 1) { - qError("rows %d in page is too small, payloadSize:%d, rowSize:%d", nRows, payloadSize, rowSize); + uError("rows %d in page is too small, payloadSize:%d, rowSize:%d", nRows, payloadSize, rowSize); terrno = TSDB_CODE_QRY_EXECUTOR_INTERNAL_ERROR; return -1; } From c633468ec7e7a343eadde1b99900c1445e634c08 Mon Sep 17 00:00:00 2001 From: dapan1121 Date: Thu, 29 Aug 2024 18:38:51 +0800 Subject: [PATCH 3/3] fix: encode error return code issue --- source/common/src/tdatablock.c | 1 + 1 file changed, 1 insertion(+) diff --git a/source/common/src/tdatablock.c b/source/common/src/tdatablock.c index b21dd8c803..8e50c943b9 100644 --- a/source/common/src/tdatablock.c +++ b/source/common/src/tdatablock.c @@ -2930,6 +2930,7 @@ int32_t blockEncode(const SSDataBlock* pBlock, char* data, int32_t numOfCols) { if (*rows <= 0) { uError("Invalid rows %d in block", *rows); terrno = TSDB_CODE_QRY_EXECUTOR_INTERNAL_ERROR; + return -1; } int32_t* cols = (int32_t*)data;