Enh/virtual table ref1 - #34752
Enh/virtual table ref1#34752yihaoDeng wants to merge 21 commits into
Conversation
…queries - Fix vgArray build order in getTableMetaViaRpc: call dynMakeVgArraySortBy before checking vgArray size, since queryBuildUseDbOutput only populates vgHash and vgArray needs to be built from it on demand. - Register TDMT_VND_TABLE_META_RSP in dmTransport.c response routing table so executor's asyncSendMsgToServer callback is properly invoked. - Add comprehensive test script t.sql covering all vtable-ref-vtable scenarios including 2-layer chains, super table scans, aggregation, window queries, and data consistency checks. Made-with: Cursor
Add 30+ new test sections covering: all data types (bool/binary/nchar) through vtable chain, NULL propagation, empty source tables, mixed physical+vtable refs under same vstb, multiple vstbs sharing same source, TAG/TBNAME/PARTITION BY filters, 3-layer vtable chain, many children under one vstb, subqueries, UNION, JOIN, advanced functions (SPREAD/DIFF/TOP/BOTTOM/SAMPLE/UNIQUE), interval+FILL, state/session windows, DISTINCT, SLIMIT/SOFFSET, arithmetic expressions, CAST, string/math/time functions, DROP intermediate vtable, LAST_ROW, cross-table column refs, information_schema queries. Uses VGROUPS 4 to exercise multi-vgroup routing. Made-with: Cursor
Move TSDB_MAX_VTABLE_REF_DEPTH from local definitions in parTranslater.c and catalog.c to include/util/tdef.h as a single global definition. Replace hardcoded depth limit of 2 in dynqueryctrloperator.c and scanoperator.c with the macro. Increase the maximum virtual table reference depth from 2 to 5. Add comprehensive depth chain tests in t.sql covering layers 3/4/5, data consistency verification across all layers, vstb with children at different depths, and aggregation/window queries on the deepest chain. Made-with: Cursor
Replace the async-gated checkRefDepth with an iterative depth check using catalogGetTableMeta directly. This ensures the depth limit is enforced during CREATE VTABLE even in async parsing mode, preventing creation of vtable chains exceeding TSDB_MAX_VTABLE_REF_DEPTH. Fix colRef iteration to skip ts column (colRef[0].hasRef=0) and find the first column with an actual reference. Update t.sql: remove vstb child referencing L5 (correctly exceeds depth limit), add explicit test for depth exceeded on CREATE. Made-with: Cursor
- Add int8_t depth field to SColRef structure - Calculate and store reference depth during CREATE VTABLE - Use SColRefWrapper version (static=1) for backward compatibility with old data (version=0, depth defaults to 0) - Update all serialization paths: tmsg.h inline, tmsg.c wrapper, metaEntry.c storage, nodesCodeFuncs.c JSON - Executor optimization: skip RPC iteration in dynqueryctrloperator when depth<=1 (direct physical table reference) Made-with: Cursor
Cover vtable chain creation/query up to depth 5, vstb with vtable children, mixed physical+vtable refs, deep chain consistency, NULL propagation, empty source, window/join/union/subquery, depth limit enforcement, and DROP behavior. Made-with: Cursor
Pass the colRef depth from meta storage through the full data path (vnode -> serialization -> systable scan -> coordinator) so that resolveVtableColRefToPhysical can skip RPC resolution when depth=1 (direct physical table reference), preserving pre-feature performance. Changes across 5 layers: - SRefColInfo/SColRefInfo: add depth field - vnodeReadVSubtables: fill depth from SColRef - tmsg serialize/deserialize: encode depth with backward compat - systable schema (userVctbColsSchema): add col_ref_depth column - sysscanoperator: fill depth column in data block - planLogicCreater: extend ins_vctb_cols scan to include depth col - dynqueryctrloperator: read depth, pass to resolve, short-circuit when depth>0 && depth<=1 - parTranslater: disallow virtual super table as colRef target Made-with: Cursor
The ts column's depth is hardcoded to 0, but its reference target may be a virtual table (not physical). Using colDepth <= 1 incorrectly short- circuited for depth=0, returning the virtual table name instead of recursively resolving to the physical table. Change to colDepth == 1 so only explicitly direct-physical references skip RPC resolution. Made-with: Cursor
…ble scan The hardcoded loop bound (i=1 to 9) in addInsColumnScanCol skipped the table_name column and was fragile when new columns were added to the ins_virtual_child_columns schema. Use pMeta->numOfColumns to scan all columns dynamically, ensuring the depth column and any future additions are included in the VSTB systable scan data block. Made-with: Cursor
…engine into enh/virtualTableRef2
There was a problem hiding this comment.
Pull request overview
This PR adds end-to-end support for virtual tables referencing other virtual tables (chained vtable refs), including depth limiting/cycle detection, metadata propagation, and CI coverage to validate the behavior across query patterns.
Changes:
- Add vtable→vtable chain handling (depth tracking, (de)serialization, and runtime resolution to physical tables).
- Extend parser/planner/executor/catalog plumbing for reference depth and original-table resolution.
- Add a comprehensive pytest suite and wire it into CI.
Reviewed changes
Copilot reviewed 25 out of 25 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| test/ci/cases.task | Adds the new virtual-table chain pytest to CI task list. |
| test/cases/05-VirtualTables/test_vtable_ref_vtable.py | New regression tests covering vtable→vtable chains (depth, windows, joins, drops, etc.). |
| t.sql | Adds a manual SQL script for exercising vtable chain behavior. |
| source/util/src/terror.c | Adds new error messages for circular refs / depth exceeded / missing origin. |
| include/util/taoserror.h | Defines new error codes for vtable chain errors. |
| include/util/tdef.h | Introduces TSDB_MAX_VTABLE_REF_DEPTH=5. |
| source/libs/qcom/src/queryUtil.c + include/libs/qcom/query.h | Extends setColRef to carry ref depth. |
| include/common/tmsg.h + source/common/src/msg/tmsg.c | Extends col-ref wire format to include depth (with compatibility guards in some paths). |
| source/dnode/vnode/src/meta/metaEntry.c | Persists colRef depth in vnode meta encoding. |
| source/dnode/vnode/src/vnd/vnodeQuery.c | Propagates depth in vsubtables refCols. |
| source/common/src/systable.c + source/libs/executor/src/sysscanoperator.c | Adds and fills a new systable column (col_ref_depth). |
| source/libs/nodes/src/nodesCodeFuncs.c | Adds JSON field for col-ref depth. |
| include/libs/nodes/querynodes.h + include/libs/nodes/cmdnodes.h | Adds refDepth fields to AST nodes/options. |
| source/libs/parser/src/parTranslater.c | Adds depth/cycle logic and resolves refs (but see comments). |
| source/libs/planner/src/planLogicCreater.c | Fixes insert-column scan loop bound to use real column count. |
| source/libs/executor/src/scanoperator.c | Adds runtime vtable-chain resolution to physical tables (but see comments). |
| source/libs/executor/src/dynqueryctrloperator.c + source/libs/executor/inc/dynqueryctrl.h | Adds RPC path to fetch table meta for resolving vtable col refs (but see comments). |
| source/libs/catalog/src/catalog.c + include/libs/catalog/catalog.h | Adds catalog helper to resolve original table vgroup across vtable chains. |
| source/dnode/mgmt/node_mgmt/src/dmTransport.c | Routes TDMT_VND_TABLE_META_RSP into qWorker response handler. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (ownedMeta) taosMemoryFree(pCurMeta); | ||
| if (code != TSDB_CODE_SUCCESS) { | ||
| break; | ||
| } |
There was a problem hiding this comment.
In checkColRef's virtual-table depth traversal, pCurMeta may be freed before checking whether checkRefDepthGetMeta succeeded; if the call fails, the loop breaks with pCurMeta still pointing to freed memory and the final cleanup can double-free it. Reorder the free to happen only after a successful fetch, or null out pCurMeta after freeing on the error path.
| if (ownedMeta) taosMemoryFree(pCurMeta); | |
| if (code != TSDB_CODE_SUCCESS) { | |
| break; | |
| } | |
| if (code != TSDB_CODE_SUCCESS) { | |
| if (ownedMeta) { | |
| taosMemoryFree(pCurMeta); | |
| } | |
| pCurMeta = NULL; | |
| break; | |
| } | |
| if (ownedMeta) { | |
| taosMemoryFree(pCurMeta); | |
| } |
| pOrgTable->me.type = TSDB_NORMAL_TABLE; | ||
| break; |
There was a problem hiding this comment.
resolveVirtualTableToPhysical sets pOrgTable->me.type = TSDB_NORMAL_TABLE when getTableEntryByName fails (next table not on this vnode), but orgTable doesn't contain the referenced table's metadata/schema in that case. The caller immediately uses schema->nCols/schema->pSchema for column mapping, so this can produce invalid reads/crashes. Instead, return an explicit error/route indicator and let the caller handle remote routing, or fetch the referenced table meta before returning a schema pointer.
| pOrgTable->me.type = TSDB_NORMAL_TABLE; | |
| break; | |
| return code; |
| qDebug("getTableMetaViaRpc: sending to vgId:%d ep:%s:%u for table %s.%s", vgInfo->vgId, pEp->fqdn, pEp->port, dbName, tbName); | ||
|
|
||
| code = asyncSendMsgToServer(pVtbScan->pMsgCb->clientRpc, &vgInfo->epSet, NULL, pMsgSendInfo); | ||
| QUERY_CHECK_CODE(code, lino, _return); |
There was a problem hiding this comment.
getTableMetaViaRpc allocates the request buffer and SMsgSendInfo, but if asyncSendMsgToServer returns an error those allocations aren't freed (ownership is only transferred on success). Add cleanup on the asyncSend failure path to avoid leaking buf/pMsgSendInfo.
| QUERY_CHECK_CODE(code, lino, _return); | |
| if (code != TSDB_CODE_SUCCESS) { | |
| taosMemoryFree(buf); | |
| taosMemoryFree(pMsgSendInfo); | |
| QUERY_CHECK_CODE(code, lino, _return); | |
| } |
| int32_t code = checkRefDepth(pCxt, pDbName, pTableName, 1, pVisitedTables, pPrecision); | ||
|
|
||
| // Cleanup visited tables array | ||
| taosArrayDestroy(pVisitedTables); | ||
| return code; |
There was a problem hiding this comment.
getOriginalTablePrecision/checkRefDepth duplicates fullName strings into pVisitedTables, but then only calls taosArrayDestroy(pVisitedTables). taosArrayDestroy does not free pointed-to elements, so this leaks each taosStrdup() entry. Free the stored strings (e.g., iterate and taosMemoryFree) or use taosArrayDestroyP with a free callback.
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the virtual table functionality by allowing virtual tables to reference other virtual tables, forming a chain of references. This feature provides greater flexibility in data modeling and abstraction. The changes include modifications to core data structures, serialization protocols, and query execution logic to correctly resolve these chained references. Robust checks for circular references and a configurable maximum reference depth have been integrated to ensure system stability and prevent infinite loops. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](/s/com/githubusercontent/avatars/G.https/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a significant enhancement to support virtual tables referencing other virtual tables, enabling chained or nested virtual tables. However, several memory leaks were identified in the new RPC handling and parser logic, specifically with RPC response buffers and internal tracking arrays for circular reference detection not being properly cleaned up, which could lead to memory exhaustion. Additionally, a critical bug in the parser could lead to a crash, and there's a high-severity issue regarding missing validation for circular references during virtual table creation. There are also several new but unused functions that need to be addressed.
| int32_t code = checkRefDepth(pCxt, pDbName, pTableName, 1, pVisitedTables, pPrecision); | ||
|
|
||
| // Cleanup visited tables array | ||
| taosArrayDestroy(pVisitedTables); |
There was a problem hiding this comment.
A memory leak was identified in getOriginalTablePrecision. The pVisitedTables array, used for circular reference detection, stores duplicated strings via taosStrdup. However, taosArrayDestroy only frees the array structure, not the string elements. You must iterate through the array and free each string before destroying the array to prevent memory exhaustion. Furthermore, the validation logic in this section has issues: the checkColRef function, while calculating reference depth, does not check for circular references, potentially allowing invalid virtual table chains. The functions detectCircularReference and checkRefDepth contain the necessary logic but are not called. Please integrate the circular reference detection logic into checkColRef.
| QUERY_CHECK_NULL(pVtbScan->pTbMetaRsp, code, lino, _return, terrno) | ||
| } | ||
|
|
||
| code = tDeserializeSTableMetaRsp(pMsg->pData, (int32_t)pMsg->len, pVtbScan->pTbMetaRsp); | ||
| QUERY_CHECK_CODE(code, lino, _return); |
There was a problem hiding this comment.
Memory leak in dynProcessTbMetaRsp. The response data buffer pMsg->pData is not freed if tDeserializeSTableMetaRsp fails or if memory allocation for pVtbScan->pTbMetaRsp fails. In TDengine's asynchronous RPC framework, the callback is responsible for freeing the message data buffer when the RPC is successful (code == TSDB_CODE_SUCCESS).
| QUERY_CHECK_NULL(pMsgSendInfo, code, lino, _return, terrno) | ||
|
|
||
| pVtbScan->pTbMetaRsp = pRsp; | ||
|
|
||
| pMsgSendInfo->param = pOperator; | ||
| pMsgSendInfo->msgInfo.pData = buf; | ||
| pMsgSendInfo->msgInfo.len = contLen; | ||
| pMsgSendInfo->msgType = TDMT_VND_TABLE_META; | ||
| pMsgSendInfo->fp = dynProcessTbMetaRsp; | ||
| pMsgSendInfo->requestId = pTaskInfo->id.queryId; | ||
|
|
||
| SEp* pEp = &vgInfo->epSet.eps[vgInfo->epSet.inUse]; | ||
| qDebug("getTableMetaViaRpc: sending to vgId:%d ep:%s:%u for table %s.%s", vgInfo->vgId, pEp->fqdn, pEp->port, dbName, tbName); | ||
|
|
||
| code = asyncSendMsgToServer(pVtbScan->pMsgCb->clientRpc, &vgInfo->epSet, NULL, pMsgSendInfo); | ||
| QUERY_CHECK_CODE(code, lino, _return); |
There was a problem hiding this comment.
Memory leak in getTableMetaViaRpc. The request buffer buf and the message send info pMsgSendInfo are leaked if asyncSendMsgToServer fails or if intermediate allocations fail. These buffers should be cleaned up in the _return block if they were not successfully handed over to the RPC layer.
| /** | ||
| * @brief Recursively get the original table's vgroup info from a virtual table chain | ||
| * @param pCxt Translate context | ||
| * @param pDbName Database name | ||
| * @param pTableName Table name (can be virtual table) | ||
| * @param pRefVgInfo Output vgroup info | ||
| * @param pVisitedTables Array of visited table names for circular reference detection | ||
| * @return TSDB_CODE_SUCCESS on success | ||
| */ | ||
| static int32_t getOriginalTableVgroupInfo(STranslateContext* pCxt, const char* pDbName, const char* pTableName, | ||
| SVgroupInfo* pRefVgInfo, SArray* pVisitedTables) { | ||
| STableMeta* pRefTableMeta = NULL; | ||
| int32_t code = TSDB_CODE_SUCCESS; | ||
| SName refName = {0}; | ||
| char fullName[TSDB_TABLE_FNAME_LEN] = {0}; | ||
|
|
||
| snprintf(fullName, sizeof(fullName), "%s.%s", pDbName, pTableName); | ||
|
|
||
| // Check circular reference | ||
| for (int32_t i = 0; i < taosArrayGetSize(pVisitedTables); i++) { | ||
| char* pVisited = *(char**)taosArrayGet(pVisitedTables, i); | ||
| if (pVisited != NULL && strcmp(pVisited, fullName) == 0) { | ||
| return generateSyntaxErrMsgExt(&pCxt->msgBuf, TSDB_CODE_VTABLE_CIRCULAR_REFERENCE, | ||
| "virtual table '%s' creates circular reference", fullName); | ||
| } | ||
| } | ||
|
|
||
| // Add current table to visited list | ||
| char* pFullNameCopy = taosStrdup(fullName); | ||
| if (pFullNameCopy == NULL || taosArrayPush(pVisitedTables, &pFullNameCopy) == NULL) { | ||
| taosMemoryFree(pFullNameCopy); | ||
| return terrno; | ||
| } | ||
|
|
||
| // Get table meta | ||
| toName(pCxt->pParseCxt->acctId, (char*)pDbName, (char*)pTableName, &refName); | ||
| code = getTableMeta(pCxt, (char*)pDbName, (char*)pTableName, &pRefTableMeta); | ||
| if (code != TSDB_CODE_SUCCESS) { | ||
| return code; | ||
| } | ||
|
|
||
| // Check if it's a virtual table | ||
| bool isVirtualTable = (pRefTableMeta->tableType == TSDB_VIRTUAL_NORMAL_TABLE || | ||
| pRefTableMeta->tableType == TSDB_VIRTUAL_CHILD_TABLE || | ||
| (pRefTableMeta->virtualStb && pRefTableMeta->tableType == TSDB_SUPER_TABLE)); | ||
|
|
||
| if (isVirtualTable && pRefTableMeta->numOfColRefs > 0 && pRefTableMeta->colRef[0].hasRef) { | ||
| // For virtual table, recursively get the original table's vgroup info | ||
| // Use the first column reference's table as the source | ||
| code = getOriginalTableVgroupInfo(pCxt, pRefTableMeta->colRef[0].refDbName, | ||
| pRefTableMeta->colRef[0].refTableName, pRefVgInfo, pVisitedTables); | ||
| taosMemoryFree(pRefTableMeta); | ||
| return code; | ||
| } else { | ||
| // For normal/child table, get vgroup info directly | ||
| code = getTableHashVgroupImpl(pCxt, &refName, pRefVgInfo); | ||
| taosMemoryFree(pRefTableMeta); | ||
| return code; | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * @brief Recursively get the original table's metadata from a virtual table chain | ||
| * @param pCxt Translate context | ||
| * @param pDbName Database name | ||
| * @param pTableName Table name (can be virtual table) | ||
| * @param ppOriginalMeta Output pointer to original table meta | ||
| * @param pOriginalDbName Output original database name | ||
| * @param pOriginalTableName Output original table name | ||
| * @param pVisitedTables Array of visited table names for circular reference detection | ||
| * @return TSDB_CODE_SUCCESS on success | ||
| */ | ||
| static int32_t getOriginalTableInfo(STranslateContext* pCxt, const char* pDbName, const char* pTableName, | ||
| STableMeta** ppOriginalMeta, char* pOriginalDbName, char* pOriginalTableName, | ||
| SArray* pVisitedTables) { | ||
| STableMeta* pRefTableMeta = NULL; | ||
| int32_t code = TSDB_CODE_SUCCESS; | ||
| char fullName[TSDB_TABLE_FNAME_LEN] = {0}; | ||
|
|
||
| snprintf(fullName, sizeof(fullName), "%s.%s", pDbName, pTableName); | ||
|
|
||
| // Check circular reference | ||
| for (int32_t i = 0; i < taosArrayGetSize(pVisitedTables); i++) { | ||
| char* pVisited = *(char**)taosArrayGet(pVisitedTables, i); | ||
| if (pVisited != NULL && strcmp(pVisited, fullName) == 0) { | ||
| return generateSyntaxErrMsgExt(&pCxt->msgBuf, TSDB_CODE_VTABLE_CIRCULAR_REFERENCE, | ||
| "virtual table '%s' creates circular reference", fullName); | ||
| } | ||
| } | ||
|
|
||
| // Check depth limit | ||
| if (taosArrayGetSize(pVisitedTables) >= TSDB_MAX_VTABLE_REF_DEPTH) { | ||
| return generateSyntaxErrMsgExt(&pCxt->msgBuf, TSDB_CODE_VTABLE_REF_DEPTH_EXCEEDED, | ||
| "virtual table reference depth exceeded maximum limit %d", TSDB_MAX_VTABLE_REF_DEPTH); | ||
| } | ||
|
|
||
| // Add current table to visited list | ||
| char* pFullNameCopy = taosStrdup(fullName); | ||
| if (pFullNameCopy == NULL || taosArrayPush(pVisitedTables, &pFullNameCopy) == NULL) { | ||
| taosMemoryFree(pFullNameCopy); | ||
| return terrno; | ||
| } | ||
|
|
||
| // Get table meta | ||
| code = getTableMeta(pCxt, (char*)pDbName, (char*)pTableName, &pRefTableMeta); | ||
| if (code != TSDB_CODE_SUCCESS) { | ||
| return code; | ||
| } | ||
|
|
||
| // Check if it's a virtual table | ||
| bool isVirtualTable = (pRefTableMeta->tableType == TSDB_VIRTUAL_NORMAL_TABLE || | ||
| pRefTableMeta->tableType == TSDB_VIRTUAL_CHILD_TABLE || | ||
| (pRefTableMeta->virtualStb && pRefTableMeta->tableType == TSDB_SUPER_TABLE)); | ||
|
|
||
| if (isVirtualTable && pRefTableMeta->numOfColRefs > 0 && pRefTableMeta->colRef[0].hasRef) { | ||
| // For virtual table, recursively get the original table info | ||
| char* nextDbName = pRefTableMeta->colRef[0].refDbName; | ||
| char* nextTableName = pRefTableMeta->colRef[0].refTableName; | ||
| taosMemoryFree(pRefTableMeta); | ||
|
|
||
| return getOriginalTableInfo(pCxt, nextDbName, nextTableName, ppOriginalMeta, | ||
| pOriginalDbName, pOriginalTableName, pVisitedTables); | ||
| } else { | ||
| // For normal/child table, return the metadata | ||
| *ppOriginalMeta = pRefTableMeta; | ||
| if (pOriginalDbName != NULL) { | ||
| tstrncpy(pOriginalDbName, pDbName, TSDB_DB_FNAME_LEN); | ||
| } | ||
| if (pOriginalTableName != NULL) { | ||
| tstrncpy(pOriginalTableName, pTableName, TSDB_TABLE_FNAME_LEN); | ||
| } | ||
| return TSDB_CODE_SUCCESS; | ||
| } | ||
| } |
There was a problem hiding this comment.
These new static functions, getOriginalTableVgroupInfo and getOriginalTableInfo, appear to be unused within this file. If they are not needed for the current implementation, they should be removed to improve code maintainability and avoid confusion.
| } | ||
| int32_t refIdx = -1; | ||
| for (int32_t ri = 0; ri < pCurMeta->numOfColRefs; ri++) { | ||
| if (pCurMeta->colRef[ri].hasRef) { refIdx = ri; break; } |
There was a problem hiding this comment.
这里相当于只看每个虚拟表引用的第一个表？后面的列的引用直接跳过了吗，这样算出来的 depth 会不会不准确？
比如某个虚拟表第一列引用的是原始表，第二列引用的是虚拟表这种情况
| } | ||
| int32_t refIdx = -1; | ||
| for (int32_t ri = 0; ri < pCurMeta->numOfColRefs; ri++) { | ||
| if (pCurMeta->colRef[ri].hasRef) { refIdx = ri; break; } |
There was a problem hiding this comment.
而且这里不应该是找到引用的那一列吗，这怎么只要 hasRef 就直接 break 了
| PAR_ERR_RET(checkColRef(pCxt, pStmt->colName, pStmt->refDbName, pStmt->refTableName, pStmt->refColName, | ||
| (SDataType){.type = pStmt->dataType.type, .bytes = calcTypeBytes(pStmt->dataType)}, | ||
| pTableMeta->tableInfo.precision)); | ||
| pTableMeta->tableInfo.precision, NULL)); |
| tstrncpy(curTable, pColRef->refTableName, sizeof(curTable)); | ||
| tstrncpy(curCol, pColRef->refColName, sizeof(curCol)); | ||
|
|
||
| while (depth < TSDB_MAX_VTABLE_REF_DEPTH) { |
There was a problem hiding this comment.
假如 alter 的时候给某一列 alter 了虚拟表作为 ref, 导致 depth 可能超过 TSDB_MAX_VTABLE_REF_DEPTH，那这里检查到第五层就直接停了，但第五层可能还是虚拟表，后续直接用不会有问题吗
| * @param pVisitedTables Array of visited table names for circular reference detection | ||
| * @return TSDB_CODE_SUCCESS on success | ||
| */ | ||
| static int32_t getOriginalTableVgroupInfo(STranslateContext* pCxt, const char* pDbName, const char* pTableName, |
|
假如虚拟超级表的虚拟子表，虚拟子表引用的虚拟表，虚拟表引用的原始表分别在三个库里，他们所在的 vnode 也就不一样，客户端生成虚拟超级表查询计划的时候有处理这种场景吗，感觉好像没看到这块逻辑 |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 24 out of 24 changed files in this pull request and generated 8 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| static int32_t getOriginalTablePrecision(STranslateContext* pCxt, const char* pDbName, const char* pTableName, | ||
| int8_t* pPrecision) { | ||
| SArray* pVisitedTables = taosArrayInit(16, sizeof(char*)); | ||
| if (pVisitedTables == NULL) { | ||
| return terrno; | ||
| } | ||
|
|
||
| int32_t code = checkRefDepth(pCxt, pDbName, pTableName, 1, pVisitedTables, pPrecision); | ||
|
|
||
| // Cleanup visited tables array | ||
| taosArrayDestroy(pVisitedTables); | ||
| return code; |
| code = colDataSetVal(pColInfoData, numOfRows, (char*)&colRef->version, false); | ||
| QUERY_CHECK_CODE(code, lino, _end); | ||
|
|
||
| // col ref depth | ||
| pColInfoData = taosArrayGet(dataBlock->pDataBlock, 9); |
| pOrgTable->me.type == TSDB_VIRTUAL_CHILD_TABLE) { | ||
| if (++depth > TSDB_MAX_VTABLE_REF_DEPTH) { | ||
| qError("virtual table reference depth exceeded max %d", TSDB_MAX_VTABLE_REF_DEPTH); | ||
| return TSDB_CODE_INVALID_PARA; |
| SColRefWrapper* pColRefW = &pOrgTable->me.colRef; | ||
| if (pColRefW->nCols <= 0 || pColRefW->pColRef == NULL) { | ||
| qError("virtual table has no col references"); | ||
| return TSDB_CODE_INVALID_PARA; |
| SELECT '=== 29. Layer 6 creation should fail (depth exceeds limit of 5) ===' AS test; | ||
| CREATE VTABLE vntb_L6 (ts timestamp, int_col int from vntb_L5.int_col); | ||
|
|
||
| SELECT '=== 29a. vstb child referencing L5 should also fail (6th layer) ===' AS test; |
| PAR_ERR_JRET(setColRef(&req.colRef.pColRef[index], index + 1, pSchema->name, pColRef->refColName, pColRef->refTableName, | ||
| pColRef->refDbName)); | ||
| index++; | ||
| PAR_ERR_JRET(setColRef(&req.colRef.pColRef[index], index + 1, NULL, pColRef->refColName, pColRef->refTableName, pColRef->refDbName, pColRef->refDepth)); |
| int32_t refIdx = -1; | ||
| for (int32_t ri = 0; ri < pCurMeta->numOfColRefs; ri++) { | ||
| if (pCurMeta->colRef[ri].hasRef) { refIdx = ri; break; } | ||
| } | ||
| if (refIdx < 0) break; | ||
| STableMeta* pNextMeta = NULL; | ||
| code = checkRefDepthGetMeta(pCxt, pCurMeta->colRef[refIdx].refDbName, | ||
| pCurMeta->colRef[refIdx].refTableName, &pNextMeta); | ||
| if (ownedMeta) taosMemoryFree(pCurMeta); |
| bool isVirtualTable = (pRefTableMeta->tableType == TSDB_VIRTUAL_NORMAL_TABLE || | ||
| pRefTableMeta->tableType == TSDB_VIRTUAL_CHILD_TABLE); | ||
|
|
||
| if (isVirtualTable) { | ||
| int32_t depth = 0; | ||
| STableMeta* pCurMeta = pRefTableMeta; | ||
| bool ownedMeta = false; | ||
| while (pCurMeta != NULL && | ||
| (pCurMeta->tableType == TSDB_VIRTUAL_NORMAL_TABLE || | ||
| pCurMeta->tableType == TSDB_VIRTUAL_CHILD_TABLE)) { | ||
| depth++; | ||
| if (depth >= TSDB_MAX_VTABLE_REF_DEPTH) { | ||
| if (ownedMeta) taosMemoryFree(pCurMeta); | ||
| PAR_ERR_JRET(generateSyntaxErrMsgExt(&pCxt->msgBuf, TSDB_CODE_VTABLE_REF_DEPTH_EXCEEDED, | ||
| "virtual table reference depth exceeded maximum limit %d", TSDB_MAX_VTABLE_REF_DEPTH)); | ||
| } | ||
| if (pCurMeta->numOfColRefs <= 0 || pCurMeta->colRef == NULL) { | ||
| break; | ||
| } | ||
| int32_t refIdx = -1; | ||
| for (int32_t ri = 0; ri < pCurMeta->numOfColRefs; ri++) { | ||
| if (pCurMeta->colRef[ri].hasRef) { refIdx = ri; break; } | ||
| } | ||
| if (refIdx < 0) break; | ||
| STableMeta* pNextMeta = NULL; | ||
| code = checkRefDepthGetMeta(pCxt, pCurMeta->colRef[refIdx].refDbName, | ||
| pCurMeta->colRef[refIdx].refTableName, &pNextMeta); | ||
| if (ownedMeta) taosMemoryFree(pCurMeta); | ||
| if (code != TSDB_CODE_SUCCESS) { | ||
| break; | ||
| } | ||
| pCurMeta = pNextMeta; | ||
| ownedMeta = true; | ||
| } | ||
| if (ownedMeta && pCurMeta) taosMemoryFree(pCurMeta); | ||
| if (pDepth) *pDepth = (int8_t)(depth + 1); | ||
| } else { | ||
| if (pRefTableMeta->tableInfo.precision != precision) { | ||
| PAR_ERR_JRET(generateSyntaxErrMsgExt(&pCxt->msgBuf, TSDB_CODE_PAR_INVALID_REF_COLUMN_TYPE, "timestamp precision of virtual table and its reference table do not match")); | ||
| } | ||
| if (pDepth) *pDepth = 1; | ||
| } |
Description
Issue(s)
Checklist
Please check the items in the checklist if applicable.