Feat/addVirtualHire#35318
Conversation
Phase 1: Error codes, constants, struct extensions, serialization - Add 9 VST inheritance error codes (0x04A2-0x04AA) - Add TSDB_MAX_VST_PARENTS=10 constant in tdef.h - Extend SStbObj with numParents, parentSuids[], ownColStart, ownTagStart - Extend SMCreateStbReq with parentStbFNames[], ownColStart, ownTagStart - Extend SVCreateStbReq with parentSuids[], ownColStart, ownTagStart - Add serialization/deserialization with backward compatibility - Bump SDB version to STB_VER_SUPPORT_INHERIT=5 Phase 2: Grammar and AST node changes - Add TK_BASE and TK_INHERITS tokens - Add CREATE STABLE ... BASE ON grammar rules - Add ALTER TABLE ADD/DROP BASE ON grammar rules - Add SHOW VTABLE INHERITS grammar rule - Add base_on_list non-terminal - Extend SCreateTableStmt with pBaseOnList field - Add TSDB_ALTER_TABLE_ADD_BASE_ON/DROP_BASE_ON constants - Add QUERY_NODE_SHOW_VSTABLE_INHERITS_STMT - Add createCreateInheritedStableStmt() and createAlterTableBaseOn() - Update nodesUtilFuncs, nodesCodeFuncs for new node types Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Add BASE ON inheritance handling in translateCreateSuperTable(): validates parent count <= 10, fills parentStbFNames, ownColStart/ownTagStart - Add ADD/DROP BASE ON support in buildAlterSuperTableReq(): serializes parent table names for mnode processing - Extend SMAlterStbReq with numParents + parentStbFNames fields - Add SMAlterStbReq serialization for BASE ON alter types - Add TSDB_INS_TABLE_VSTABLE_INHERITS system table constant - Register SHOW VTABLE INHERITS in show-rewrite table - Wire QUERY_NODE_SHOW_VSTABLE_INHERITS_STMT to rewriteShow() Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Add utility functions: mndStbHasChildren, mndStbHasVCT, mndCheckCyclicInherit - Add inheritance validation in mndProcessCreateStbReq: - Parent existence, virtualStb check, same-DB, VCT gate, max parents, cycle detection - Copy inheritance fields (parentSuids, ownColStart, ownTagStart) in mndBuildStbFromReq - Add DROP VST protection: refuse if VST has child VSTs - Implement ALTER TABLE ADD/DROP BASE ON in mndAlterStb switch - mndAlterStbAddBaseOn: validate + add parents - mndAlterStbDropBaseOn: find + remove parents - Add ins_vstable_inherits system table: - Schema: db_name, parent_stable_name, parent_uid, child_stable_name, child_uid, create_time - Register in systable.c, mndShow.c, and mndStb.c - Implement mndRetrieveVstableInherits retrieval function - Add TSDB_MGMT_TABLE_VSTABLE_INHERITS to EShowType enum Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Add tests/system-test/0-others/vst_inheritance_ddl.py - Tests: create parent VST, single/multi inheritance, error cases (non-virtual parent, max parents, cross-DB, drop-with-children), SHOW VTABLE INHERITS, ALTER ADD/DROP BASE ON Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Add numParents/parentStbNames fields to STableCfg - Serialize/deserialize inheritance in STableCfgRsp with backward compat - Resolve parent names in mndBuildStbCfgImp for display - Append BASE ON clause in setCreateTBResultIntoDataBlock Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…cases - Add column name conflict detection test - Add tag name conflict detection test - Add circular inheritance detection test - Add SHOW CREATE STABLE with BASE ON verification - Add non-leaf VCT creation rejection test - Add leaf VCT creation positive test Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Add hasInheritors field to STableMetaRsp and STableMeta - Set hasInheritors in mndBuildStbSchemaImp via mndStbHasChildren - Propagate through catalog metadata (querymsg.c) - Check in translator: refuse VCT creation on non-leaf VST - Backward-compatible serialization with tDecodeIsEnd guard Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
When querying a non-leaf (parent) VST that has inheritors: 1. Translator intercepts in translateRealTable() 2. Calls catalogGetVstLeaves() to discover leaf descendants via mnode BFS 3. Builds UNION ALL of SELECT <parent_schema_cols> FROM each leaf 4. Wraps in STempTableNode so standard translate/plan/execute handles it Leaf VST queries go directly through translateVirtualTable (no expansion). Parent VST queries project only the parent's own schema columns from leaves. Components: - mnode: mndProcessGetVstLeavesReq (BFS leaf finder) - catalog: catalogGetVstLeaves (sync RPC wrapper) - parser: rewriteNonLeafVstQuery + buildLeafSelectStmt - tmsg: SVstLeavesReq/Rsp structs and serialization Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This reverts commit 8a40bb0.
DROP BASE ON: identify parent's columns/tags by name matching in the inherited region [0, ownColStart), remove them, rebuild schema arrays, validate ≥2 cols + ≥1 tag remain. Adjust ownColStart/ownTagStart. ADD BASE ON: validate no column name conflicts, merge new parent's columns (skip ts) and tags into the inherited region, assign new colIds, adjust ownColStart/ownTagStart. Includes cycle detection. Both paths allocate new schema arrays and bump colVer/tagVer so the change propagates to vnodes via mndAlterStbImp. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
20 test cases covering: - CREATE with single/multi parent inheritance - ALTER ADD/DROP BASE ON with schema verification - Repeated ADD/DROP cycles (3 rounds) - Column and tag name conflict detection on ADD - Circular inheritance detection (direct + indirect) - Max 10 parents limit - Non-leaf VCT prohibition - VCT creation on leaf, data insert, query verification - Parent VST query via UNION ALL expansion (multi-leaf) - ADD parent then query (new columns visible as NULL) - DROP parent then query (columns removed) - DROP BASE ON with existing VCT (colRef cascade) - Re-add previously dropped parent - Non-virtual parent rejection, parent-with-VCT rejection - Cross-DB inheritance rejection - Multi-level inheritance query (grandparent→mid→leaf) - Diamond inheritance (2 parents, 2 leaves, query parent) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Major fixes for VST multi-inheritance DDL: mndStb.c: - Fix SDB update callback to copy inheritance fields (numParents, parentSuids, ownColStart, ownTagStart) from pNew to pOld - Fix CREATE with BASE ON: merge parent columns/tags into child schema (layout: [ts][parent_cols][own_cols], [parent_tags][own_tags]) - Fix ALTER ADD BASE ON: preserve ts at position 0 using effectiveOwnColStart for standalone VSTs (ownColStart=0) - Fix ALTER DROP BASE ON: always keep ts column (keepCol[0]=true), iterate inherited range from [1, ownColStart) - Fix column conflict check: skip ts column (start at index 1) - Remove debug logging from encode/decode/retrieve functions vst_inheritance_cascade.py: - Fix all VCT creation to use correct FROM syntax (not backtick refs) - Remove ts FROM mappings (ts ref not allowed for virtual tables) - Rename 'value' to 'val' (reserved word conflict) - Comment out non-leaf VST queries (Phase 6 not yet implemented) - Skip test_parent_with_vct (mndStbHasVCT needs vnode query) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…EATE/ALTER BASE ON Add TDMT_VND_CHECK_HAS_CTB message to verify parent VSTs have no child tables before allowing inheritance. Uses SERIAL transaction mode to execute check actions before DDL actions sequentially. - Register CHECK_HAS_CTB message in vnode/mnode dispatch and routing - Implement vnodeProcessCheckHasCtbReq using metaCtbCursor - Build check redo actions for each parent suid across all vgroups - Use TRN_EXEC_SERIAL (not GROUP_PARALLEL) for correct ordering - Fix error code propagation: preserve pTrans->code through rollback when syncContext cannot execute (avoid CTX_SWITCH overwriting real error) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Rewrite test_vst_inheritance_cascade.py from the old test framework to the new pytest-based framework (new_test_framework). All 20 test methods pass with ASAN enabled and no memory leaks. Key changes: - Import from new_test_framework.utils instead of util.log/sql/cases - Class TestVstInheritanceCascade (not TDTestCase) - setup_class for DB/table creation, no run()/stop() methods - Static helper methods with underscore prefix - Proper docstrings with Catalog/Since/Labels/History metadata - Error validation via expectErrInfo for VCT detection tests (17a-17g) - Removed tdCases.addLinux/addWindows registration Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Code Review
This pull request introduces Virtual Super Table (VST) inheritance, allowing child VSTs to inherit schemas and tags from multiple parents via the BASE ON syntax. The implementation covers MNode metadata extensions, parser grammar updates, and system table integration. However, several critical issues must be addressed: the nextColId is not updated during inheritance-based schema changes, which could lead to metadata corruption; the mndStbHasVCT check is currently a stub, violating the functional specification; and the leaf discovery BFS uses a fixed-size queue that may ignore descendants in large trees. Furthermore, the inheritance retrieval logic needs optimization to avoid O(N^2) complexity, and a large stack allocation in MNode should be moved to the heap to mitigate stack overflow risks.
| pNew->ownTagStart = oldOwnTagStart + (int16_t)addTags; | ||
| pNew->colVer++; | ||
| pNew->tagVer++; | ||
|
|
There was a problem hiding this comment.
In mndAlterStbAddBaseOn, the nextColId of the super table object is not updated after assigning new IDs to the inherited columns and tags. This will cause subsequent ALTER TABLE ADD COLUMN operations to reuse existing column IDs, leading to metadata corruption.
pNew->ownColStart = effectiveOwnColStart + (int16_t)addCols;
pNew->ownTagStart = oldOwnTagStart + (int16_t)addTags;
pNew->nextColId = maxColId + 1;
pNew->colVer++;
pNew->tagVer++;| bool mndStbHasVCT(SMnode *pMnode, int64_t suid) { | ||
| // VCTs are stored as child tables of the VST; check via vnode meta. | ||
| // For simplicity in mnode, we check if any vgroup has ctables for this suid. | ||
| // TODO: implement proper VCT existence check via vgroup query | ||
| // For now, return false (conservative: allow inheritance) | ||
| return false; | ||
| } |
There was a problem hiding this comment.
| int64_t allDescs[256]; | ||
| int32_t numDescs = 0; | ||
|
|
||
| while (qHead < qTail && qTail < 256) { |
| SStbObj *pParent = NULL; | ||
| while (1) { | ||
| pIter2 = sdbFetch(pSdb, SDB_STB, pIter2, (void **)&pParent); | ||
| if (pIter2 == NULL) break; | ||
| if (pParent->uid == pStb->parentSuids[i]) { | ||
| mndExtractTbNameFromStbFullName(pParent->name, parentName, TSDB_TABLE_NAME_LEN); | ||
| sdbRelease(pSdb, pParent); | ||
| sdbCancelFetch(pSdb, pIter2); | ||
| break; | ||
| } | ||
| sdbRelease(pSdb, pParent); | ||
| } |
There was a problem hiding this comment.
This nested sdbFetch loop to find a parent's name by UID results in O(N^2) complexity for the SHOW VSTABLE INHERITS command. Since parent UIDs are stored in the SStbObj, a direct lookup by UID should be used if the SDB API supports it, or the mapping should be optimized to avoid full table scans for every inheritance relationship.
| } | ||
|
|
||
| // Among descendants, find leaves (those with no children of their own) | ||
| SVstLeafInfo leaves[256]; |
There was a problem hiding this comment.
There was a problem hiding this comment.
Pull request overview
This PR introduces Virtual Super Table (VST) inheritance support across the SQL parser/translator, metadata messages, system tables, and test suites, including BASE ON DDL, SHOW ... INHERITS, and non-leaf VST query handling.
Changes:
- Add
BASE ON(create/alter) syntax and AST plumbing, plus new error codes and message fields for VST inheritance metadata. - Add
ins_vstable_inheritssystem table +SHOW ... INHERITSadapter andSHOW CREATE STABLEoutput for inherited parents. - Implement non-leaf VST query rewrite to leaf descendants and add CI/system tests covering inheritance behaviors.
Reviewed changes
Copilot reviewed 38 out of 38 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/system-test/0-others/vst_inheritance_ddl.py | Adds legacy system-test coverage for inheritance DDL + errors. |
| test/ci/cases.task | Registers new inheritance cascade test in CI task list. |
| test/cases/05-VirtualTables/test_vst_inheritance_cascade.py | Adds new_test_framework cascade + query behavior tests for inheritance. |
| source/util/src/terror.c | Adds VST-inheritance related error strings. |
| source/libs/qcom/src/querymsg.c | Propagates hasInheritors from meta rsp into STableMeta. |
| source/libs/parser/src/parTranslater.c | Adds SHOW adapter and non-leaf VST query rewrite + BASE ON request fill. |
| source/libs/parser/src/parTokenizer.c | Adds BASE / INHERITS keywords. |
| source/libs/parser/src/parAstCreater.c | Adds AST builders for inherited STABLE + alter base-on clauses. |
| source/libs/parser/inc/sql.y | Adds grammar for CREATE/ALTER BASE ON and SHOW VTABLE INHERITS. |
| source/libs/parser/inc/parAst.h | Exposes new AST creator APIs. |
| source/libs/nodes/src/nodesUtilFuncs.c | Updates node creation/destruction to handle new SHOW type + base-on list cleanup. |
| source/libs/nodes/src/nodesCodeFuncs.c | Adds JSON (de)serialization for pBaseOnList and node name mapping. |
| source/libs/command/src/command.c | Extends SHOW CREATE STABLE rendering with BASE ON .... |
| source/libs/catalog/src/catalog.c | Adds Catalog RPC helper to fetch leaf descendants for non-leaf queries. |
| source/dnode/vnode/src/vnd/vnodeSvr.c | Routes new vnode fetch message CHECK_HAS_CTB. |
| source/dnode/vnode/src/vnd/vnodeQuery.c | Implements vnode handler to detect whether a VST has any child tables. |
| source/dnode/vnode/src/inc/vnd.h | Declares vnode handler for CHECK_HAS_CTB. |
| source/dnode/mnode/impl/src/mndTrans.c | Adjusts transaction error code retention logic. |
| source/dnode/mnode/impl/src/mndShow.c | Adds retrieve-type mapping for ins_vstable_inherits. |
| source/dnode/mnode/impl/inc/mndDef.h | Extends SStbObj with inheritance fields. |
| source/dnode/mgmt/mgmt_vnode/src/vmHandle.c | Registers vnode mgmt handler for CHECK_HAS_CTB. |
| source/dnode/mgmt/mgmt_mnode/src/mmHandle.c | Registers mnode mgmt handler for CHECK_HAS_CTB_RSP. |
| source/common/src/systable.c | Defines ins_vstable_inherits schema and registers it. |
| source/common/src/msg/tmsg.c | Adds serialization for inheritance fields + VST leaves + check-has-ctb request. |
| include/util/tdef.h | Adds TSDB_MAX_VST_PARENTS. |
| include/util/taoserror.h | Adds VST-inheritance error codes. |
| include/libs/qcom/query.h | Adds hasInheritors bit to STableMeta. |
| include/libs/nodes/cmdnodes.h | Adds pBaseOnList to create table AST node. |
| include/libs/catalog/catalog.h | Exposes catalogGetVstLeaves() API. |
| include/common/tmsgdef.h | Allocates new msg type for TDMT_MND_GET_VST_LEAVES and TDMT_VND_CHECK_HAS_CTB. |
| include/common/tmsg.h | Adds new msg payload structs/fields and new SHOW node type + alter types. |
| include/common/systable.h | Defines TSDB_INS_TABLE_VSTABLE_INHERITS. |
| compare.md | Design comparison doc (EXPAND vs UNION rewrite). |
| 17-vst-inheritance-plan.md | Implementation plan doc for VST inheritance. |
| 17-vst-inheritance-fs.md | Feature spec doc for VST inheritance. |
| 17-vst-inheritance-design.md | Technical design doc for VST inheritance. |
| .github/copilot-instructions.md | Adds repo-specific build/test and architecture notes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Build projection list from parent's schema columns | ||
| int32_t numCols = pParentMeta->tableInfo.numOfColumns; | ||
| SSchema* pSchema = pParentMeta->schema; | ||
| for (int32_t i = 0; i < numCols; ++i) { |
| // Single leaf: build SELECT <parent_cols> FROM leaf as subquery | ||
| SNode* pSubquery = NULL; | ||
| code = buildLeafSelectStmt(pCxt, rsp.pLeaves[0].dbFName, rsp.pLeaves[0].stbName, pParentMeta, &pSubquery); | ||
| if (TSDB_CODE_SUCCESS != code) { |
| STempTableNode* pTempTable = NULL; | ||
| code = nodesMakeNode(QUERY_NODE_TEMP_TABLE, (SNode**)&pTempTable); | ||
| if (TSDB_CODE_SUCCESS != code) { | ||
| nodesDestroyNode(pSubquery); | ||
| tFreeSVstLeavesRsp(&rsp); | ||
| return code; | ||
| } | ||
| pTempTable->pSubquery = pSubquery; | ||
| tstrncpy(pTempTable->table.tableAlias, pRealTable->table.tableAlias, sizeof(pTempTable->table.tableAlias)); | ||
| tstrncpy(pTempTable->table.dbName, pRealTable->table.dbName, sizeof(pTempTable->table.dbName)); | ||
|
|
| // Wrap in STempTableNode | ||
| STempTableNode* pTempTable = NULL; | ||
| PAR_ERR_JRET(nodesMakeNode(QUERY_NODE_TEMP_TABLE, (SNode**)&pTempTable)); | ||
| pTempTable->pSubquery = pSetOp; | ||
| pSetOp = NULL; | ||
| tstrncpy(pTempTable->table.tableAlias, pRealTable->table.tableAlias, sizeof(pTempTable->table.tableAlias)); | ||
| tstrncpy(pTempTable->table.dbName, pRealTable->table.dbName, sizeof(pTempTable->table.dbName)); |
| typedef struct SCreateTableStmt { | ||
| ENodeType type; | ||
| char dbName[TSDB_DB_NAME_LEN]; | ||
| char tableName[TSDB_TABLE_NAME_LEN]; | ||
| bool ignoreExists; | ||
| SNodeList* pCols; | ||
| SNodeList* pTags; | ||
| STableOptions* pOptions; | ||
| SNodeList* pBaseOnList; // list of SRealTableNode for BASE ON parents (NULL = no inheritance) | ||
| } SCreateTableStmt; |
| if (pCur != NULL) { | ||
| tb_uid_t id = metaCtbCursorNext(pCur); | ||
| metaCloseCtbCursor(pCur); | ||
| if (id != 0) { | ||
| code = TSDB_CODE_MND_VST_PARENT_HAS_VCT; | ||
| } | ||
| } | ||
|
|
| int32_t tDeserializeSVstLeavesRsp(void *buf, int32_t bufLen, SVstLeavesRsp *pRsp) { | ||
| SDecoder decoder = {0}; | ||
| int32_t code = 0; | ||
| int32_t lino; | ||
| tDecoderInit(&decoder, buf, bufLen); | ||
| TAOS_CHECK_EXIT(tStartDecode(&decoder)); | ||
| TAOS_CHECK_EXIT(tDecodeI32(&decoder, &pRsp->numLeaves)); | ||
| if (pRsp->numLeaves > 0) { | ||
| pRsp->pLeaves = taosMemoryCalloc(pRsp->numLeaves, sizeof(SVstLeafInfo)); | ||
| if (NULL == pRsp->pLeaves) { | ||
| code = terrno; | ||
| goto _exit; | ||
| } | ||
| for (int32_t i = 0; i < pRsp->numLeaves; ++i) { | ||
| TAOS_CHECK_EXIT(tDecodeCStrTo(&decoder, pRsp->pLeaves[i].dbFName)); | ||
| TAOS_CHECK_EXIT(tDecodeCStrTo(&decoder, pRsp->pLeaves[i].stbName)); | ||
| TAOS_CHECK_EXIT(tDecodeI64(&decoder, &pRsp->pLeaves[i].suid)); | ||
| } | ||
| } | ||
| tEndDecode(&decoder); |
| ```sql | ||
| SHOW VSTABLE INHERITS; | ||
| ``` |
Description
Issue(s)
Checklist
Please check the items in the checklist if applicable.