From 078c7bdc31276e7aa5f4a055a6e1b05889dc8d0c Mon Sep 17 00:00:00 2001 From: dmchen Date: Thu, 20 Feb 2025 17:33:48 +0800 Subject: [PATCH 1/6] fix/TS-6028-check-column-name --- source/dnode/mnode/impl/src/mndStb.c | 31 ++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/source/dnode/mnode/impl/src/mndStb.c b/source/dnode/mnode/impl/src/mndStb.c index d46968a22d..7ce970403f 100644 --- a/source/dnode/mnode/impl/src/mndStb.c +++ b/source/dnode/mnode/impl/src/mndStb.c @@ -1259,6 +1259,7 @@ static int32_t mndProcessCreateStbReq(SRpcMsg *pReq) { SDbObj *pDb = NULL; SMCreateStbReq createReq = {0}; bool isAlter = false; + SHashObj *pHash = NULL; if (tDeserializeSMCreateStbReq(pReq->pCont, pReq->contLen, &createReq) != 0) { code = TSDB_CODE_INVALID_MSG; @@ -1319,6 +1320,25 @@ static int32_t mndProcessCreateStbReq(SRpcMsg *pReq) { goto _OVER; } + pHash = taosHashInit(3, taosGetDefaultHashFunction(TSDB_DATA_TYPE_BINARY), false, HASH_NO_LOCK); + void *pIter = NULL; + + for (int32_t i = 0; i < createReq.numOfColumns; ++i) { + SFieldWithOptions *pField = taosArrayGet(createReq.pColumns, i); + if (taosHashPut(pHash, pField->name, sizeof(pField->name), pField->name, sizeof(pField->name)) != 0) { + code = TSDB_CODE_TSC_DUP_COL_NAMES; + goto _OVER; + } + } + + for (int32_t i = 0; i < createReq.numOfTags; ++i) { + SField *pField = taosArrayGet(createReq.pTags, i); + if (taosHashPut(pHash, pField->name, sizeof(pField->name), pField->name, sizeof(pField->name)) != 0) { + code = TSDB_CODE_TSC_DUP_COL_NAMES; + goto _OVER; + } + } + pDb = mndAcquireDbByStb(pMnode, createReq.name); if (pDb == NULL) { code = TSDB_CODE_MND_DB_NOT_SELECTED; @@ -1383,6 +1403,17 @@ _OVER: mndReleaseDb(pMnode, pDb); tFreeSMCreateStbReq(&createReq); + if (pHash != NULL) { + pIter = taosHashIterate(pHash, NULL); + while (pIter) { + STablesReq *pDb = (STablesReq *)pIter; + taosArrayDestroy(pDb->pTables); + pIter = taosHashIterate(pHash, pIter); + } + + taosHashCleanup(pHash); + } + TAOS_RETURN(code); } From cdb11dd49774284a6f2316822bf99da563fafa1d Mon Sep 17 00:00:00 2001 From: dmchen Date: Fri, 21 Feb 2025 10:10:44 +0800 Subject: [PATCH 2/6] fix/TS-6028-column-name-fix-cases --- source/dnode/mnode/impl/src/mndStb.c | 7 ------- 1 file changed, 7 deletions(-) diff --git a/source/dnode/mnode/impl/src/mndStb.c b/source/dnode/mnode/impl/src/mndStb.c index 7ce970403f..5ada3e42e3 100644 --- a/source/dnode/mnode/impl/src/mndStb.c +++ b/source/dnode/mnode/impl/src/mndStb.c @@ -1404,13 +1404,6 @@ _OVER: tFreeSMCreateStbReq(&createReq); if (pHash != NULL) { - pIter = taosHashIterate(pHash, NULL); - while (pIter) { - STablesReq *pDb = (STablesReq *)pIter; - taosArrayDestroy(pDb->pTables); - pIter = taosHashIterate(pHash, pIter); - } - taosHashCleanup(pHash); } From 6a81665ffab5ddf7e22a37b944eaf4279f667595 Mon Sep 17 00:00:00 2001 From: dmchen Date: Fri, 21 Feb 2025 14:51:40 +0800 Subject: [PATCH 3/6] fix/TS-6028-check-column-name-add-case --- source/dnode/mnode/impl/test/stb/stb.cpp | 89 ++++++++++++++++++++++++ 1 file changed, 89 insertions(+) diff --git a/source/dnode/mnode/impl/test/stb/stb.cpp b/source/dnode/mnode/impl/test/stb/stb.cpp index e92231907f..438b4fd2e4 100644 --- a/source/dnode/mnode/impl/test/stb/stb.cpp +++ b/source/dnode/mnode/impl/test/stb/stb.cpp @@ -25,6 +25,7 @@ class MndTestStb : public ::testing::Test { void* BuildCreateDbReq(const char* dbname, int32_t* pContLen); void* BuildDropDbReq(const char* dbname, int32_t* pContLen); void* BuildCreateStbReq(const char* stbname, int32_t* pContLen); + void* BuildCreateStbDuplicateReq(const char* stbname, int32_t* pContLen); void* BuildAlterStbAddTagReq(const char* stbname, const char* tagname, int32_t* pContLen); void* BuildAlterStbDropTagReq(const char* stbname, const char* tagname, int32_t* pContLen); void* BuildAlterStbUpdateTagNameReq(const char* stbname, const char* tagname, const char* newtagname, @@ -137,6 +138,71 @@ void* MndTestStb::BuildCreateStbReq(const char* stbname, int32_t* pContLen) { return pHead; } +void* MndTestStb::BuildCreateStbDuplicateReq(const char* stbname, int32_t* pContLen) { + SMCreateStbReq createReq = {0}; + createReq.numOfColumns = 2; + createReq.numOfTags = 4; + createReq.igExists = 0; + createReq.pColumns = taosArrayInit(createReq.numOfColumns, sizeof(SField)); + createReq.pTags = taosArrayInit(createReq.numOfTags, sizeof(SField)); + strcpy(createReq.name, stbname); + + { + SField field = {0}; + field.bytes = 8; + field.type = TSDB_DATA_TYPE_TIMESTAMP; + strcpy(field.name, "ts"); + taosArrayPush(createReq.pColumns, &field); + } + + { + SField field = {0}; + field.bytes = 12; + field.type = TSDB_DATA_TYPE_BINARY; + strcpy(field.name, "col1"); + taosArrayPush(createReq.pColumns, &field); + } + + { + SField field = {0}; + field.bytes = 2; + field.type = TSDB_DATA_TYPE_TINYINT; + strcpy(field.name, "tag1"); + taosArrayPush(createReq.pTags, &field); + } + + { + SField field = {0}; + field.bytes = 8; + field.type = TSDB_DATA_TYPE_BIGINT; + strcpy(field.name, "tag2"); + taosArrayPush(createReq.pTags, &field); + } + + { + SField field = {0}; + field.bytes = 16; + field.type = TSDB_DATA_TYPE_BINARY; + strcpy(field.name, "tag3"); + taosArrayPush(createReq.pTags, &field); + } + + { + SField field = {0}; + field.bytes = 16; + field.type = TSDB_DATA_TYPE_BINARY; + strcpy(field.name, "tag3"); + taosArrayPush(createReq.pTags, &field); + } + + int32_t tlen = tSerializeSMCreateStbReq(NULL, 0, &createReq); + void* pHead = rpcMallocCont(tlen); + tSerializeSMCreateStbReq(pHead, tlen, &createReq); + tFreeSMCreateStbReq(&createReq); + *pContLen = tlen; + return pHead; +} + void* MndTestStb::BuildAlterStbAddTagReq(const char* stbname, const char* tagname, int32_t* pContLen) { SMAlterStbReq req = {0}; strcpy(req.name, stbname); @@ -896,3 +962,26 @@ TEST_F(MndTestStb, 08_Alter_Stb_AlterTagBytes) { rpcFreeCont(pRsp->pCont); } } + +TEST_F(MndTestStb, 09_Create_Duplicate_Stb) { + const char* dbname = "1.d2"; + const char* stbname = "1.d2.stb"; + + { + int32_t contLen = 0; + void* pReq = BuildCreateDbReq(dbname, &contLen); + SRpcMsg* pRsp = test.SendReq(TDMT_MND_CREATE_DB, pReq, contLen); + ASSERT_NE(pRsp, nullptr); + ASSERT_EQ(pRsp->code, 0); + rpcFreeCont(pRsp->pCont); + } + + { + int32_t contLen = 0; + void* pReq = BuildCreateStbDuplicateReq(stbname, &contLen); + SRpcMsg* pRsp = test.SendReq(TDMT_MND_CREATE_STB, pReq, contLen); + ASSERT_NE(pRsp, nullptr); + ASSERT_EQ(pRsp->code, TSDB_CODE_TSC_DUP_COL_NAMES); + rpcFreeCont(pRsp->pCont); + } +} \ No newline at end of file From fdc60a63369656b9d03a8918737c1f2838235c7c Mon Sep 17 00:00:00 2001 From: yihaoDeng Date: Fri, 21 Feb 2025 18:20:26 +0800 Subject: [PATCH 4/6] fix:[TS-6028]check-column-name in sml --- source/client/src/clientSml.c | 47 +++++++++++++++++++---------------- 1 file changed, 25 insertions(+), 22 deletions(-) diff --git a/source/client/src/clientSml.c b/source/client/src/clientSml.c index 376dea895c..c5ba653ad2 100644 --- a/source/client/src/clientSml.c +++ b/source/client/src/clientSml.c @@ -751,7 +751,7 @@ static int32_t smlFindNearestPowerOf2(int32_t length, uint8_t type) { } static int32_t smlProcessSchemaAction(SSmlHandle *info, SSchema *schemaField, SHashObj *schemaHash, SArray *cols, - SArray *checkDumplicateCols, ESchemaAction *action, bool isTag) { + SHashObj *schemaHashCheck, ESchemaAction *action, bool isTag) { int32_t code = TSDB_CODE_SUCCESS; int32_t lino = 0; for (int j = 0; j < taosArrayGetSize(cols); ++j) { @@ -759,15 +759,11 @@ static int32_t smlProcessSchemaAction(SSmlHandle *info, SSchema *schemaField, SH SSmlKv *kv = (SSmlKv *)taosArrayGet(cols, j); SML_CHECK_NULL(kv); SML_CHECK_CODE(smlGenerateSchemaAction(schemaField, schemaHash, kv, isTag, action, info)); - } - - for (int j = 0; j < taosArrayGetSize(checkDumplicateCols); ++j) { - SSmlKv *kv = (SSmlKv *)taosArrayGet(checkDumplicateCols, j); - SML_CHECK_NULL(kv); - if (taosHashGet(schemaHash, kv->key, kv->keyLen) != NULL) { + if (taosHashGet(schemaHashCheck, kv->key, kv->keyLen) != NULL) { SML_CHECK_CODE(TSDB_CODE_PAR_DUPLICATED_COLUMN); } } + END: RETURN } @@ -998,14 +994,14 @@ static int32_t smlBuildFields(SArray **pColumns, SArray **pTags, STableMeta *pTa END: RETURN } -static int32_t smlModifyTag(SSmlHandle *info, SHashObj* hashTmp, SRequestConnInfo *conn, +static int32_t smlModifyTag(SSmlHandle *info, SHashObj* hashTmpCheck, SHashObj* hashTmp, SRequestConnInfo *conn, SSmlSTableMeta *sTableData, SName *pName, STableMeta **pTableMeta){ ESchemaAction action = SCHEMA_ACTION_NULL; SArray *pColumns = NULL; SArray *pTags = NULL; int32_t code = 0; int32_t lino = 0; - SML_CHECK_CODE(smlProcessSchemaAction(info, (*pTableMeta)->schema, hashTmp, sTableData->tags, sTableData->cols, &action, true)); + SML_CHECK_CODE(smlProcessSchemaAction(info, (*pTableMeta)->schema, hashTmp, sTableData->tags, hashTmpCheck, &action, true)); if (action != SCHEMA_ACTION_NULL) { SML_CHECK_CODE(smlCheckAuth(info, conn, pName->tname, AUTH_TYPE_WRITE)); @@ -1029,14 +1025,14 @@ END: RETURN } -static int32_t smlModifyCols(SSmlHandle *info, SHashObj* hashTmp, SRequestConnInfo *conn, +static int32_t smlModifyCols(SSmlHandle *info, SHashObj* hashTmpCheck, SHashObj* hashTmp, SRequestConnInfo *conn, SSmlSTableMeta *sTableData, SName *pName, STableMeta **pTableMeta){ ESchemaAction action = SCHEMA_ACTION_NULL; SArray *pColumns = NULL; SArray *pTags = NULL; int32_t code = 0; int32_t lino = 0; - SML_CHECK_CODE(smlProcessSchemaAction(info, (*pTableMeta)->schema, hashTmp, sTableData->cols, sTableData->tags, &action, false)); + SML_CHECK_CODE(smlProcessSchemaAction(info, (*pTableMeta)->schema, hashTmp, sTableData->cols, hashTmpCheck, &action, false)); if (action != SCHEMA_ACTION_NULL) { SML_CHECK_CODE(smlCheckAuth(info, conn, pName->tname, AUTH_TYPE_WRITE)); @@ -1079,7 +1075,8 @@ static int32_t smlModifyDBSchemas(SSmlHandle *info) { } int32_t code = 0; int32_t lino = 0; - SHashObj *hashTmp = NULL; + SHashObj *colHashTmp = NULL; + SHashObj *tagHashTmp = NULL; STableMeta *pTableMeta = NULL; SName pName = {TSDB_TABLE_NAME_T, info->taos->acctId, {0}, {0}}; @@ -1119,17 +1116,21 @@ static int32_t smlModifyDBSchemas(SSmlHandle *info) { SML_CHECK_CODE(TSDB_CODE_SML_NOT_SUPPORT_PK); } - hashTmp = taosHashInit(pTableMeta->tableInfo.numOfTags, taosGetDefaultHashFunction(TSDB_DATA_TYPE_BINARY), true, HASH_NO_LOCK); - SML_CHECK_NULL(hashTmp); - SML_CHECK_CODE(smlBuildTempHash(hashTmp, pTableMeta, pTableMeta->tableInfo.numOfColumns, pTableMeta->tableInfo.numOfColumns + pTableMeta->tableInfo.numOfTags)); - SML_CHECK_CODE(smlModifyTag(info, hashTmp, &conn, sTableData, &pName, &pTableMeta)); - taosHashClear(hashTmp); - SML_CHECK_CODE(smlBuildTempHash(hashTmp, pTableMeta, 0, pTableMeta->tableInfo.numOfColumns)); - SML_CHECK_CODE(smlModifyCols(info, hashTmp, &conn, sTableData, &pName, &pTableMeta)); + colHashTmp = taosHashInit(pTableMeta->tableInfo.numOfColumns, taosGetDefaultHashFunction(TSDB_DATA_TYPE_BINARY), true, HASH_NO_LOCK); + tagHashTmp = taosHashInit(pTableMeta->tableInfo.numOfTags, taosGetDefaultHashFunction(TSDB_DATA_TYPE_BINARY), true, HASH_NO_LOCK); + SML_CHECK_NULL(colHashTmp); + SML_CHECK_NULL(tagHashTmp); + SML_CHECK_CODE(smlBuildTempHash(tagHashTmp, pTableMeta, pTableMeta->tableInfo.numOfColumns, pTableMeta->tableInfo.numOfColumns + pTableMeta->tableInfo.numOfTags)); + SML_CHECK_CODE(smlBuildTempHash(colHashTmp, pTableMeta, 0, pTableMeta->tableInfo.numOfColumns)); + + SML_CHECK_CODE(smlModifyTag(info, colHashTmp, tagHashTmp, &conn, sTableData, &pName, &pTableMeta)); + SML_CHECK_CODE(smlModifyCols(info, tagHashTmp, colHashTmp, &conn, sTableData, &pName, &pTableMeta)); needCheckMeta = true; - taosHashCleanup(hashTmp); - hashTmp = NULL; + taosHashCleanup(colHashTmp); + taosHashCleanup(tagHashTmp); + colHashTmp = NULL; + tagHashTmp = NULL; } else { uError("SML:0x%" PRIx64 " %s load table meta error: %s", info->id, __FUNCTION__, tstrerror(code)); goto END; @@ -1153,7 +1154,8 @@ static int32_t smlModifyDBSchemas(SSmlHandle *info) { END: taosHashCancelIterate(info->superTables, tmp); - taosHashCleanup(hashTmp); + taosHashCleanup(colHashTmp); + taosHashCleanup(tagHashTmp); taosMemoryFreeClear(pTableMeta); (void)catalogRefreshTableMeta(info->pCatalog, &conn, &pName, 1); // ignore refresh meta code if there is an error uError("SML:0x%" PRIx64 " %s end failed:%d:%s, format:%d, needModifySchema:%d", info->id, __FUNCTION__, code, @@ -1924,3 +1926,4 @@ TAOS_RES *taos_schemaless_insert_raw(TAOS *taos, char *lines, int len, int32_t * return taos_schemaless_insert_raw_ttl_with_reqid(taos, lines, len, totalRows, protocol, precision, TSDB_DEFAULT_TABLE_TTL, 0); } + From 26f3adcf6ad9d4642d4163afd0c9f8dad6d407f4 Mon Sep 17 00:00:00 2001 From: dmchen Date: Tue, 25 Feb 2025 10:27:32 +0000 Subject: [PATCH 5/6] fix/TS-6028-check-column-name-fix-review --- source/dnode/mnode/impl/src/mndStb.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/source/dnode/mnode/impl/src/mndStb.c b/source/dnode/mnode/impl/src/mndStb.c index 5ada3e42e3..78f633acad 100644 --- a/source/dnode/mnode/impl/src/mndStb.c +++ b/source/dnode/mnode/impl/src/mndStb.c @@ -1320,12 +1320,17 @@ static int32_t mndProcessCreateStbReq(SRpcMsg *pReq) { goto _OVER; } - pHash = taosHashInit(3, taosGetDefaultHashFunction(TSDB_DATA_TYPE_BINARY), false, HASH_NO_LOCK); + pHash = taosHashInit(createReq.numOfColumns + createReq.numOfTags, taosGetDefaultHashFunction(TSDB_DATA_TYPE_BINARY), + false, HASH_NO_LOCK); + if (pHash == NULL) { + code = TSDB_CODE_OUT_OF_MEMORY; + goto _OVER; + } void *pIter = NULL; for (int32_t i = 0; i < createReq.numOfColumns; ++i) { SFieldWithOptions *pField = taosArrayGet(createReq.pColumns, i); - if (taosHashPut(pHash, pField->name, sizeof(pField->name), pField->name, sizeof(pField->name)) != 0) { + if (taosHashPut(pHash, pField->name, strlen(pField->name), NULL, 0) != 0) { code = TSDB_CODE_TSC_DUP_COL_NAMES; goto _OVER; } @@ -1333,8 +1338,10 @@ static int32_t mndProcessCreateStbReq(SRpcMsg *pReq) { for (int32_t i = 0; i < createReq.numOfTags; ++i) { SField *pField = taosArrayGet(createReq.pTags, i); - if (taosHashPut(pHash, pField->name, sizeof(pField->name), pField->name, sizeof(pField->name)) != 0) { - code = TSDB_CODE_TSC_DUP_COL_NAMES; + if ((code = taosHashPut(pHash, pField->name, strlen(pField->name), NULL, 0)) != 0) { + if (code == TSDB_CODE_DUP_KEY) { + code = TSDB_CODE_TSC_DUP_COL_NAMES; + } goto _OVER; } } From ebd2e657c6a743f169abcd41d65dc2d6785e1ed0 Mon Sep 17 00:00:00 2001 From: dmchen Date: Wed, 26 Feb 2025 01:30:33 +0000 Subject: [PATCH 6/6] fix/TS-6028-check-column-name-fix-review --- source/dnode/mnode/impl/src/mndStb.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/source/dnode/mnode/impl/src/mndStb.c b/source/dnode/mnode/impl/src/mndStb.c index 78f633acad..05b9826ae8 100644 --- a/source/dnode/mnode/impl/src/mndStb.c +++ b/source/dnode/mnode/impl/src/mndStb.c @@ -1326,12 +1326,13 @@ static int32_t mndProcessCreateStbReq(SRpcMsg *pReq) { code = TSDB_CODE_OUT_OF_MEMORY; goto _OVER; } - void *pIter = NULL; for (int32_t i = 0; i < createReq.numOfColumns; ++i) { SFieldWithOptions *pField = taosArrayGet(createReq.pColumns, i); - if (taosHashPut(pHash, pField->name, strlen(pField->name), NULL, 0) != 0) { - code = TSDB_CODE_TSC_DUP_COL_NAMES; + if ((code = taosHashPut(pHash, pField->name, strlen(pField->name), NULL, 0)) != 0) { + if (code == TSDB_CODE_DUP_KEY) { + code = TSDB_CODE_TSC_DUP_COL_NAMES; + } goto _OVER; } }