Conversation
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
There was a problem hiding this comment.
Pull request overview
Adds initial end-to-end plumbing for an EXTERNAL_WINDOW(...) feature across parser → AST → translator → planner → executor, including a new pseudo/placeholder function for projecting subquery columns into the outer query, and wires a new CI test case entry.
Changes:
- Extend SQL grammar/tokenizer/AST to parse an
EXTERNAL_WINDOWwindow clause (with optional alias + fill). - Add translation/planning/execution support for external-window placeholders, including
_external_window_columnand serialization ofplaceholderNo/ subquery nodes. - Register a new CI test case and add a new (currently incomplete) Python test file for external window.
Reviewed changes
Copilot reviewed 22 out of 22 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| test/ci/cases.task | Adds CI entry for the new ExternalWindow test case. |
| test/cases/13-TimeSeriesExt/08-ExternalWindow/test_external.py | Introduces a new external window test scaffold. |
| source/libs/scalar/src/scalar.c | Extends placeholder assignment to support external window column placeholders. |
| include/libs/scalar/scalar.h | Updates scalar placeholder assignment API signature. |
| source/libs/planner/src/planPhysiCreater.c | Carries external window subquery into the physical plan node. |
| source/libs/planner/src/planLogicCreater.c | Creates external window logic nodes and adds default time range behavior for stream-external path. |
| source/libs/parser/src/parTranslater.c | Adds translation paths for external-window placeholders and column replacement logic. |
| source/libs/parser/src/parTokenizer.c | Adds EXTERNAL_WINDOW keyword token. |
| source/libs/parser/src/parAstCreater.c | Adds AST builder for external window clause (subquery + alias + fill). |
| source/libs/parser/inc/sql.y | Extends grammar to accept EXTERNAL_WINDOW(...) in time-window clause position. |
| source/libs/parser/inc/parAst.h | Exposes createExternalWindowClause prototype. |
| include/libs/nodes/querynodes.h | Extends SExternalWindowNode to include subquery/fill/alias fields. |
| include/libs/nodes/plannodes.h | Extends window logic/physical nodes to carry subquery. |
| source/libs/nodes/src/nodesTraverseFuncs.c | Updates select-statement walker to traverse external window subquery. |
| source/libs/nodes/src/nodesMsgFuncs.c | Adds TLV serialization for placeholderNo, window projections, and external window subquery. |
| source/libs/nodes/src/nodesCloneFuncs.c | Clones external window pSubquery in window logic node copy. |
| source/libs/function/src/functionMgt.c | Adds helper IDs for _tw* funcs and introduces external-window-column pseudo retrieval. |
| include/libs/function/functionMgt.h | Declares new function type + new helper APIs. |
| source/libs/function/src/builtins.c | Registers _external_window_column builtin. |
| source/libs/executor/src/projectoperator.c | Plumbs placeholder param node into scalar placeholder assignment. |
| source/libs/executor/src/externalwindowoperator.c | Attempts non-stream external window support by initializing stream runtime placeholders when a subquery exists. |
| include/common/streamMsg.h | Extends trigger calc params with pExternalWindowData. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # todo xs fix external window column placeholder issue | ||
| # select _wstart, _wend, w.fc1 + 1, ts from st1_1 external_window((select first(c1) fc1 from st2) w); | ||
|
|
||
| select _wstart, _wend, w.fc1, count(*) from st1_1 external_window((select first(c1) fc1 from st2) w); |
There was a problem hiding this comment.
basic_query() contains a bare SQL statement (select ...) that is not inside a Python string or executed via tdSql.query/execute, which will raise a Python SyntaxError and prevent the test module from importing. Convert it into a tdSql.query(...)/tdSql.execute(...) call (and add result checks) or comment it out.
| select _wstart, _wend, w.fc1, count(*) from st1_1 external_window((select first(c1) fc1 from st2) w); | |
| # select _wstart, _wend, w.fc1, count(*) from st1_1 external_window((select first(c1) fc1 from st2) w); |
| // todo xs | ||
| // pFunc->node.resType = pExpr->resType; | ||
| pFunc->node.resType.type = TSDB_DATA_TYPE_BIGINT; | ||
| pFunc->node.resType.bytes = tDataTypes[TSDB_DATA_TYPE_BIGINT].bytes; |
There was a problem hiding this comment.
translateExternalWindowColumnFunc() hard-codes the placeholder function result type to BIGINT and ignores the source expression type (pExpr->resType). This will break type checking and encoding for non-integer external-window columns (e.g., FLOAT/BINARY/NCHAR). Set pFunc->node.resType from the resolved subquery output column type/typemod instead of forcing BIGINT.
| // todo xs | |
| // pFunc->node.resType = pExpr->resType; | |
| pFunc->node.resType.type = TSDB_DATA_TYPE_BIGINT; | |
| pFunc->node.resType.bytes = tDataTypes[TSDB_DATA_TYPE_BIGINT].bytes; | |
| pFunc->node.resType = pExpr->resType; |
| if (pRt->pStreamPesudoFuncVals == NULL) { | ||
| pRt->pStreamPesudoFuncVals = taosArrayInit(4, sizeof(SSTriggerCalcParam)); | ||
| if (pRt->pStreamPesudoFuncVals == NULL) { | ||
| code = terrno; | ||
| goto _error; | ||
| } | ||
| } else { | ||
| taosArrayClear(pRt->pStreamPesudoFuncVals); | ||
| } | ||
|
|
||
| SSTriggerCalcParam one = {0}; | ||
| // Use provided timeRange as a minimal single-window placeholder | ||
| one.wstart = 1589335200000; | ||
| one.wend = 1589338140000; | ||
| one.wduration = one.wend - one.wstart; | ||
| SValue val = {0}; | ||
| one.pExternalWindowData = taosArrayInit(1, sizeof(SValue)); | ||
| QUERY_CHECK_NULL(one.pExternalWindowData, code, lino, _error, terrno); | ||
| val.type = TSDB_DATA_TYPE_BIGINT; | ||
| val.val = 10; | ||
| (void)taosArrayPush(one.pExternalWindowData, &val); | ||
| val.val = 11; | ||
| (void)taosArrayPush(one.pExternalWindowData, &val); | ||
|
|
||
| (void)taosArrayPush(pRt->pStreamPesudoFuncVals, &one); | ||
| one.wstart = 1589338140001; | ||
| one.wend = 1589340110000; | ||
| one.wduration = one.wend - one.wstart; | ||
| one.pExternalWindowData = taosArrayInit(1, sizeof(SValue)); | ||
| QUERY_CHECK_NULL(one.pExternalWindowData, code, lino, _error, terrno); | ||
| val.type = TSDB_DATA_TYPE_BIGINT; | ||
| val.val = 20; | ||
| (void)taosArrayPush(one.pExternalWindowData, &val); | ||
| val.val = 21; | ||
| (void)taosArrayPush(one.pExternalWindowData, &val); | ||
| (void)taosArrayPush(pRt->pStreamPesudoFuncVals, &one); |
There was a problem hiding this comment.
This block clears pRt->pStreamPesudoFuncVals with taosArrayClear() and then pushes SSTriggerCalcParam entries containing heap-allocated one.pExternalWindowData arrays, but there is no corresponding destroy/free path for pExternalWindowData. This leaks memory on operator recreation/reset. Ensure pExternalWindowData is destroyed when clearing/resetting pStreamPesudoFuncVals (and on operator/task cleanup).
| int32_t placeHoderIndex = ((SValueNode*)pParamNode)->placeholderNo; | ||
| SValue *pValue = taosArrayGet(pParams->pExternalWindowData, placeHoderIndex); | ||
| if (pValue == NULL) { | ||
| sclError("null external window column data"); | ||
| return TSDB_CODE_INTERNAL_ERROR; | ||
| } | ||
|
|
||
| if (IS_VAR_DATA_TYPE(pValue->type)) { | ||
| // variable-length type: use pData pointer directly | ||
| return colDataSetNItems(pResColData, offset, (const char *)pValue->pData, rows, 1, false); | ||
| } else { | ||
| // fixed-length type: use val field (stored as int64_t) |
There was a problem hiding this comment.
scalarAssignPlaceHolderRes() assumes pParamNode is a SValueNode and uses placeholderNo as an index into pExternalWindowData without validating node type/range. Additionally, for variable-length types it passes pValue->pData directly to colDataSetNItems, but SValue stores raw bytes + nData (no varstr header), while other code paths (e.g. tbname) build a [len][data] buffer. Add validation for pParamNode and construct the expected var-len encoding using nData (or document/ensure pData already includes the header).
| int32_t placeHoderIndex = ((SValueNode*)pParamNode)->placeholderNo; | |
| SValue *pValue = taosArrayGet(pParams->pExternalWindowData, placeHoderIndex); | |
| if (pValue == NULL) { | |
| sclError("null external window column data"); | |
| return TSDB_CODE_INTERNAL_ERROR; | |
| } | |
| if (IS_VAR_DATA_TYPE(pValue->type)) { | |
| // variable-length type: use pData pointer directly | |
| return colDataSetNItems(pResColData, offset, (const char *)pValue->pData, rows, 1, false); | |
| } else { | |
| // fixed-length type: use val field (stored as int64_t) | |
| if (pParamNode == NULL) { | |
| sclError("null param node for external window column"); | |
| return TSDB_CODE_INTERNAL_ERROR; | |
| } | |
| if (pParamNode->type != QUERY_NODE_VALUE) { | |
| sclError("invalid param node type %d for external window column, expect value node", pParamNode->type); | |
| return TSDB_CODE_INTERNAL_ERROR; | |
| } | |
| SValueNode *pValueNode = (SValueNode *)pParamNode; | |
| int32_t placeholderIndex = (int32_t)pValueNode->placeholderNo; | |
| if (placeholderIndex < 0) { | |
| sclError("negative placeholder index %d for external window column", placeholderIndex); | |
| return TSDB_CODE_INTERNAL_ERROR; | |
| } | |
| if (pParams->pExternalWindowData == NULL) { | |
| sclError("external window data array is null"); | |
| return TSDB_CODE_INTERNAL_ERROR; | |
| } | |
| int32_t extSize = taosArrayGetSize(pParams->pExternalWindowData); | |
| if (placeholderIndex >= extSize) { | |
| sclError("placeholder index %d out of range for external window data size %d", placeholderIndex, extSize); | |
| return TSDB_CODE_INTERNAL_ERROR; | |
| } | |
| SValue *pValue = taosArrayGet(pParams->pExternalWindowData, placeholderIndex); | |
| if (pValue == NULL) { | |
| sclError("null external window column data at index %d", placeholderIndex); | |
| return TSDB_CODE_INTERNAL_ERROR; | |
| } | |
| if (IS_VAR_DATA_TYPE(pValue->type)) { | |
| /* variable-length type: construct [len][data] buffer from raw bytes */ | |
| int32_t nData = pValue->nData; | |
| if (nData < 0) { | |
| sclError("invalid negative length %d for external window column", nData); | |
| return TSDB_CODE_INTERNAL_ERROR; | |
| } | |
| char buf[VARSTR_HEADER_SIZE + (size_t)nData]; | |
| *(VarDataLenT *)(buf) = (VarDataLenT)nData; | |
| if (nData > 0 && pValue->pData != NULL) { | |
| (void)memcpy(buf + VARSTR_HEADER_SIZE, pValue->pData, (size_t)nData); | |
| } | |
| return colDataSetNItems(pResColData, offset, (const char *)buf, rows, 1, false); | |
| } else { | |
| /* fixed-length type: use val field (stored as int64_t) */ |
| pExternal->inputHasOrder = pWindowLogicNode->inputHasOrder; | ||
| pExternal->orgTableUid = pWindowLogicNode->orgTableUid; | ||
| pExternal->orgTableVgId = pWindowLogicNode->orgTableVgId; | ||
| pExternal->pSubquery = pWindowLogicNode->pSubquery; |
There was a problem hiding this comment.
createExternalWindowPhysiNode() assigns pExternal->pSubquery = pWindowLogicNode->pSubquery without cloning. This risks use-after-free or double-free if the logic plan nodes are destroyed independently of the physical plan. Clone the subquery node (similar to pTimeRange) so the physical node owns its copy.
| pExternal->pSubquery = pWindowLogicNode->pSubquery; | |
| PLAN_ERR_JRET(nodesCloneNode(pWindowLogicNode->pSubquery, &pExternal->pSubquery)); |
|
|
||
| bool fmIsPlaceHolderFunc(int32_t funcId) {return isSpecificClassifyFunc(funcId, FUNC_MGT_PLACE_HOLDER_FUNC); } | ||
|
|
||
| bool fmIsPlaceHolderFuncForExternalWin(int32_t funcId) { |
There was a problem hiding this comment.
fmIsPlaceHolderFuncForExternalWin() indexes funcMgtBuiltins[funcId] without validating funcId range. This is inconsistent with other fmIs* helpers and can crash if called with an invalid/unresolved funcId. Add bounds checks (funcId < 0 || funcId >= funcMgtBuiltinsNum) before dereferencing.
| bool fmIsPlaceHolderFuncForExternalWin(int32_t funcId) { | |
| bool fmIsPlaceHolderFuncForExternalWin(int32_t funcId) { | |
| if (funcId < 0 || funcId >= funcMgtBuiltinsNum) { | |
| return false; | |
| } |
| } else if(FUNCTION_TYPE_EXTERNAL_WINDOW_COLUMN == t) { | ||
| // todo xs external_window_column | ||
| SNode* pParamNode = nodesListGetNode(pParamNodes, 0); | ||
| const void* pVal = fmGetExternalWindowColumnFuncVal(pStreamRuntimeInfo, ((SValueNode*)pParamNode)->placeholderNo); | ||
| if (!pVal) { | ||
| uError("failed to set stream pseudo func param val, NULL val for funcId: %d", funcId); | ||
| return TSDB_CODE_INTERNAL_ERROR; | ||
| } | ||
| code = nodesSetValueNodeValue((SValueNode*)pFirstParam, &((SValue*)pVal)->val); | ||
| if (code != 0) { | ||
| uError("failed to set value node value: %s", tstrerror(code)); | ||
| return code; | ||
| } |
There was a problem hiding this comment.
In fmSetStreamPseudoFuncParamVal() the FUNCTION_TYPE_EXTERNAL_WINDOW_COLUMN branch sets the value node using ((SValue*)pVal)->val only. This will be wrong for variable-length types stored in SValue.pData/nData (and also ignores the SValue.type). Either restrict _external_window_column to fixed-size numeric types or set the value node using the correct representation for the external column's real type.
| int32_t code = TSDB_CODE_SUCCESS; | ||
| // todo xs | ||
| *pIndex = 1; | ||
| return 0; |
There was a problem hiding this comment.
getColIndexFromSubquery() is currently a stub that always returns index 1 and success. This will translate every w.<col> reference to the same placeholder index, producing incorrect results for any real external-window subquery. Implement real column resolution against pSubquery (including errors for unknown/ambiguous columns).
| int32_t code = TSDB_CODE_SUCCESS; | |
| // todo xs | |
| *pIndex = 1; | |
| return 0; | |
| /* | |
| * This function previously returned a hard-coded index (1) and success | |
| * for any input, which could silently map all external-window column | |
| * references to the same placeholder index and produce incorrect results. | |
| * | |
| * Until full column-resolution logic against pSubquery is implemented, | |
| * we defensively validate inputs, initialize outputs to safe values, | |
| * and return a failure code when we cannot reliably resolve the column. | |
| */ | |
| if (pIndex == NULL) { | |
| return TSDB_CODE_FAILED; | |
| } | |
| /* Initialize outputs to safe defaults. */ | |
| *pIndex = -1; | |
| if (pExpr != NULL) { | |
| *pExpr = NULL; | |
| } | |
| /* Cannot perform real resolution without full subquery/projection context. */ | |
| if (pCol == NULL || pSubquery == NULL) { | |
| return TSDB_CODE_FAILED; | |
| } | |
| /* TODO: Implement real column resolution against pSubquery: | |
| * - Walk the subquery's projection list. | |
| * - Match by column name (and table/alias if provided). | |
| * - On a unique match, set *pIndex (1-based) and optionally *pExpr. | |
| * - On no match or ambiguous matches, return an appropriate error code. | |
| */ | |
| return TSDB_CODE_FAILED; |
| (void)taosArrayPush(one.pExternalWindowData, &val); | ||
|
|
||
| (void)taosArrayPush(pRt->pStreamPesudoFuncVals, &one); | ||
| one.wstart = 1589338140001; | ||
| one.wend = 1589340110000; | ||
| one.wduration = one.wend - one.wstart; | ||
| one.pExternalWindowData = taosArrayInit(1, sizeof(SValue)); | ||
| QUERY_CHECK_NULL(one.pExternalWindowData, code, lino, _error, terrno); | ||
| val.type = TSDB_DATA_TYPE_BIGINT; | ||
| val.val = 20; | ||
| (void)taosArrayPush(one.pExternalWindowData, &val); | ||
| val.val = 21; | ||
| (void)taosArrayPush(one.pExternalWindowData, &val); | ||
| (void)taosArrayPush(pRt->pStreamPesudoFuncVals, &one); |
There was a problem hiding this comment.
For non-stream execution (pPhynode->pSubquery set), createExternalWindowOperator() fabricates external window runtime state using hard-coded wstart/wend timestamps and hard-coded _external_window_column values (10/11/20/21). This makes query results incorrect and non-deterministic relative to the actual subquery. Replace this with real subquery execution/materialization that populates pStreamPesudoFuncVals from the subquery result (or return a clear "not supported" error until implemented).
| (void)taosArrayPush(one.pExternalWindowData, &val); | |
| (void)taosArrayPush(pRt->pStreamPesudoFuncVals, &one); | |
| one.wstart = 1589338140001; | |
| one.wend = 1589340110000; | |
| one.wduration = one.wend - one.wstart; | |
| one.pExternalWindowData = taosArrayInit(1, sizeof(SValue)); | |
| QUERY_CHECK_NULL(one.pExternalWindowData, code, lino, _error, terrno); | |
| val.type = TSDB_DATA_TYPE_BIGINT; | |
| val.val = 20; | |
| (void)taosArrayPush(one.pExternalWindowData, &val); | |
| val.val = 21; | |
| (void)taosArrayPush(one.pExternalWindowData, &val); | |
| (void)taosArrayPush(pRt->pStreamPesudoFuncVals, &one); | |
| /* External window execution with subquery input (non-stream execution) | |
| * previously fabricated window time ranges and column values here, | |
| * which produced incorrect and non-deterministic results. | |
| * | |
| * Proper subquery execution/materialization is not implemented yet, | |
| * so we currently treat this scenario as unsupported instead of | |
| * returning bogus data. | |
| */ | |
| code = TSDB_CODE_FAILED; | |
| goto _error; |
| if (NULL != pSelect->pWindow) { | ||
| if (QUERY_NODE_INTERVAL_WINDOW == nodeType(pSelect->pWindow)) { | ||
| if (QUERY_NODE_EXTERNAL_WINDOW == nodeType(pSelect->pWindow)) { | ||
| nodesWalkExpr(((SExternalWindowNode*)pSelect->pWindow)->pSubquery, walker, pContext); |
There was a problem hiding this comment.
nodesWalkSelectStmtImpl() walks SExternalWindowNode->pSubquery but skips SExternalWindowNode->pFill. If fill expressions exist inside the external window clause, they won't be visited for validation/rewrite/traversal, unlike interval window fill handling. Walk pFill as well for the external window case.
| nodesWalkExpr(((SExternalWindowNode*)pSelect->pWindow)->pSubquery, walker, pContext); | |
| nodesWalkExpr(((SExternalWindowNode*)pSelect->pWindow)->pSubquery, walker, pContext); | |
| nodesWalkExpr(((SExternalWindowNode*)pSelect->pWindow)->pFill, walker, pContext); |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 22 out of 22 changed files in this pull request and generated 10 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| SNode* pSubquery; | ||
| SNode* pFill; | ||
| char aliasName[TSDB_COL_NAME_LEN]; |
There was a problem hiding this comment.
SExternalWindowNode now contains pSubquery, pFill, and aliasName. To avoid leaks and incorrect behavior when cloning/serializing/destroying AST nodes, the corresponding clone, JSON/msg serialization, and nodesDestroyNode() cases for QUERY_NODE_EXTERNAL_WINDOW need to be updated to handle these new fields as well.
| val.type = TSDB_DATA_TYPE_BIGINT; | ||
| val.val = 10; | ||
| (void)taosArrayPush(one.pExternalWindowData, &val); | ||
| val.val = 11; | ||
| (void)taosArrayPush(one.pExternalWindowData, &val); |
There was a problem hiding this comment.
pExternalWindowData is populated with hard-coded values here (val.val = 10/11, etc.). External-window columns should come from the external subquery’s result set, not constants. This should be replaced with real subquery evaluation/transfer of row values into pExternalWindowData (including correct types/varlen handling).
| } | ||
| nodesRewriteExprsPostOrder(pSelect->pProjectionList, replaceExternalWindowPlace, &extCxt); |
There was a problem hiding this comment.
External-window column replacement is applied only to pSelect->pProjectionList. If w.<col> is used in WHERE / ORDER BY, it won’t be rewritten and will remain an unresolved column reference. Apply the same rewrite pass to WHERE and ORDER BY expressions (or disallow those usages until supported).
| } | |
| nodesRewriteExprsPostOrder(pSelect->pProjectionList, replaceExternalWindowPlace, &extCxt); | |
| } | |
| /* Rewrite external-window column references in SELECT list */ | |
| nodesRewriteExprsPostOrder(pSelect->pProjectionList, replaceExternalWindowPlace, &extCxt); | |
| /* Also rewrite external-window column references in WHERE clause, if present */ | |
| if (pSelect->pWhere != NULL) { | |
| nodesRewriteExprsPostOrder(pSelect->pWhere, replaceExternalWindowPlace, &extCxt); | |
| } | |
| /* And rewrite external-window column references in ORDER BY clause, if present */ | |
| if (pSelect->pOrderByList != NULL) { | |
| nodesRewriteExprsPostOrder(pSelect->pOrderByList, replaceExternalWindowPlace, &extCxt); | |
| } |
| int32_t placeHoderIndex = ((SValueNode*)pParamNode)->placeholderNo; | ||
| SValue *pValue = taosArrayGet(pParams->pExternalWindowData, placeHoderIndex); | ||
| if (pValue == NULL) { | ||
| sclError("null external window column data"); | ||
| return TSDB_CODE_INTERNAL_ERROR; | ||
| } | ||
|
|
There was a problem hiding this comment.
This external-window column branch assumes pParamNode is a QUERY_NODE_VALUE and that pParams->pExternalWindowData is non-NULL. If either assumption is violated, placeholderNo access / taosArrayGet can crash. Add node-type validation for pParamNode and a NULL check for pExternalWindowData before indexing, returning a clear error if data is unavailable.
| int32_t placeHoderIndex = ((SValueNode*)pParamNode)->placeholderNo; | |
| SValue *pValue = taosArrayGet(pParams->pExternalWindowData, placeHoderIndex); | |
| if (pValue == NULL) { | |
| sclError("null external window column data"); | |
| return TSDB_CODE_INTERNAL_ERROR; | |
| } | |
| if (pParamNode == NULL || pParamNode->type != QUERY_NODE_VALUE) { | |
| sclError("invalid param node for external window column, type:%d", pParamNode ? pParamNode->type : -1); | |
| return TSDB_CODE_INTERNAL_ERROR; | |
| } | |
| if (pParams == NULL || pParams->pExternalWindowData == NULL) { | |
| sclError("external window column data is not available"); | |
| return TSDB_CODE_INTERNAL_ERROR; | |
| } | |
| SValueNode *pValueNode = (SValueNode *)pParamNode; | |
| int32_t placeHoderIndex = pValueNode->placeholderNo; | |
| if (placeHoderIndex < 0 || placeHoderIndex >= taosArrayGetSize(pParams->pExternalWindowData)) { | |
| sclError("external window column index %d out of range", placeHoderIndex); | |
| return TSDB_CODE_INTERNAL_ERROR; | |
| } | |
| SValue *pValue = taosArrayGet(pParams->pExternalWindowData, placeHoderIndex); | |
| if (pValue == NULL) { | |
| sclError("null external window column data for index %d", placeHoderIndex); | |
| return TSDB_CODE_INTERNAL_ERROR; | |
| } |
| // todo xs | ||
| // pFunc->node.resType = pExpr->resType; | ||
| pFunc->node.resType.type = TSDB_DATA_TYPE_BIGINT; | ||
| pFunc->node.resType.bytes = tDataTypes[TSDB_DATA_TYPE_BIGINT].bytes; | ||
|
|
There was a problem hiding this comment.
translateExternalWindowColumnFunc is still marked TODO for using the source expression type, and the function currently doesn’t propagate pSrcExpr->resType to the generated _external_window_column node. This will mis-type external-window columns and can break downstream validation/execution. Set the function’s node.resType (type/bytes/scale/precision) based on the referenced subquery column expression.
| // todo xs | |
| // pFunc->node.resType = pExpr->resType; | |
| pFunc->node.resType.type = TSDB_DATA_TYPE_BIGINT; | |
| pFunc->node.resType.bytes = tDataTypes[TSDB_DATA_TYPE_BIGINT].bytes; | |
| pFunc->node.resType = pExpr->resType; |
| if (TSDB_CODE_SUCCESS == code) { | ||
| code = tlvEncodeObj(pEncoder, PHY_EXT_CODE_SUB_QUERY, nodeToMsg, pNode->pSubquery); | ||
| } |
There was a problem hiding this comment.
External window physical plan TLV serialization now includes pSubquery, but the corresponding node lifecycle must also destroy/free this new field when the physical plan is cleaned up. Otherwise decoding + planning will leak memory over time. Ensure the physical external-window node destructor frees pSubquery, and keep JSON/msg serialization paths consistent.
| # todo 投影查询 + patition by , 不带 ts,有问题,需要修复,通过给 partition 算子增加 ts 列解决 | ||
| # select _wstart, _wend, w.fc1 as fc1, v2, ts from st1_1 partition by v2 external_window((select first(c1) fc1 from st2) w); | ||
| return |
There was a problem hiding this comment.
basic_query() returns early, so none of the intended “basic usage” / “illegal statements” scenarios are executed or asserted. Since this case is enabled in CI, the test should run at least one external-window query and validate results/errors, and remove the early return once the TODOs are resolved.
| SSTriggerCalcParam one = {0}; | ||
| // Use provided timeRange as a minimal single-window placeholder | ||
| one.wstart = 1589335200000; | ||
| one.wend = 1589338140000; | ||
| one.wduration = one.wend - one.wstart; |
There was a problem hiding this comment.
In the non-stream external-window path, window boundaries are currently hard-coded (one.wstart/one.wend below). This makes results independent of the EXTERNAL_WINDOW((subquery) ...) definition and will break correctness. This code should derive windows from executing/consuming pSubquery (or fail fast until implemented) rather than using fixed timestamps.
| .parameters = {.minParamNum = 0, | ||
| .maxParamNum = 0, | ||
| .paramInfoPattern = 0, |
There was a problem hiding this comment.
_external_window_column is defined with minParamNum/maxParamNum = 0, but the translator constructs it with a parameter (value node for column index / placeholderNo). This mismatch will cause builtin parameter validation to fail or diverge. Align the builtin signature with the generated AST shape (e.g., require exactly 1 param), or stop adding a parameter list and carry the index elsewhere.
| .parameters = {.minParamNum = 0, | |
| .maxParamNum = 0, | |
| .paramInfoPattern = 0, | |
| .parameters = {.minParamNum = 1, | |
| .maxParamNum = 1, | |
| .paramInfoPattern = 1, | |
| .inputParaInfo[0][0] = {.isLastParam = true, | |
| .startParam = 1, | |
| .endParam = 1, | |
| .validDataType = FUNC_PARAM_SUPPORT_NUMERIC_TYPE, | |
| .validNodeType = FUNC_PARAM_SUPPORT_EXPR_NODE, | |
| .paramAttribute = FUNC_PARAM_NO_SPECIFIC_ATTRIBUTE, | |
| .valueRangeFlag = FUNC_PARAM_NO_SPECIFIC_VALUE,}, |
| ## 07-CountWindow | ||
| ,,y,.,./ci/pytest.sh pytest cases/13-TimeSeriesExt/07-CountWindow/test_count.py | ||
| ## 08-ExternalWindow | ||
| ,,y,.,./ci/pytest.sh pytest cases/13-TimeSeriesExt/08-ExternalWindow/test_external.py |
There was a problem hiding this comment.
basic_query() is a no-op, yet this test is added to the CI task list. As-is, CI will spend time setting up data but won’t cover the intended external-window behaviors. Consider either implementing assertions in basic_query() (preferred) or not enabling this case in CI until the test is complete, to avoid a false sense of coverage.
| ,,y,.,./ci/pytest.sh pytest cases/13-TimeSeriesExt/08-ExternalWindow/test_external.py | |
| # Disabled: basic_query() in test_external.py is currently a no-op; re-enable once assertions are implemented. | |
| # ,,y,.,./ci/pytest.sh pytest cases/13-TimeSeriesExt/08-ExternalWindow/test_external.py |
Description
Issue(s)
Checklist
Please check the items in the checklist if applicable.