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
This PR adds FILL clause support for EXTERNAL_WINDOW by extending the SQL grammar/parser, propagating fill metadata through planner/plan serialization, and implementing filled-window emission logic in the external window executor. It also adds CI coverage with a new test suite for external-window fill modes.
Changes:
- Extend parser grammar + translation to accept
EXTERNAL_WINDOW(... fill(...)), validate unsupported modes (e.g., LINEAR/NEAR/SURROUND), and enforce “aggregate-only” constraints. - Carry external-window fill mode/exprs/values through logical/physical plan nodes, including clone/JSON/TLV encode/decode and destruction.
- Implement runtime fill behavior in
externalwindowoperatorand add dedicated pytest coverage (test_external_fill.py) wired into CI.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
test/ci/cases.task |
Adds the new external-window fill pytest to CI. |
test/cases/13-TimeSeriesExt/08-ExternalWindow/test_external.py |
Updates existing regression expectations now that some FILL modes are supported for external windows. |
test/cases/13-TimeSeriesExt/08-ExternalWindow/test_external_fill.py |
New, extensive test coverage for external-window fill modes and comparisons vs interval fill behavior. |
source/libs/planner/src/planPhysiCreater.c |
Builds/attaches external-window fill info in the physical plan. |
source/libs/planner/src/planLogicCreater.c |
Captures fill mode/values in the external-window logic node and defers fill-expr construction. |
source/libs/parser/src/parTranslater.c |
Adds external-window fill translation, validation, and aggregate-only restriction. |
source/libs/parser/src/parAstCreater.c |
Ensures fill clause _wstart column is wired to the external window’s primary key column. |
source/libs/parser/inc/sql.y |
Grammar changes to allow external_window_fill_opt forms. |
source/libs/nodes/src/nodesUtilFuncs.c |
Ensures new fill fields are properly destroyed for logic/physical nodes. |
source/libs/nodes/src/nodesMsgFuncs.c |
TLV serialize/deserialize external-window fill mode/exprs/values. |
source/libs/nodes/src/nodesCodeFuncs.c |
JSON serialize/deserialize external-window fill mode/exprs/values. |
source/libs/nodes/src/nodesCloneFuncs.c |
Deep-clones new external-window fill fields for logic/physical nodes. |
source/libs/executor/src/externalwindowoperator.c |
Implements filled-window output behavior for external-window aggregation mode. |
include/libs/nodes/plannodes.h |
Introduces SExtWindowFillInfo and embeds it into window logic + external physical plan nodes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
test/cases/13-TimeSeriesExt/08-ExternalWindow/test_external_fill.py
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 15 out of 15 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (1)
source/libs/executor/src/filloperator.c:499
- The new FILL-HAVING debug dumping runs whenever pFilterInfo is present and logs entire data blocks (values + condition string) at qError level before/after filtering. This can severely impact performance and flood logs in normal queries, and may leak sensitive query results into error logs. Please gate this behind a compile-time/debug flag or log-level check (e.g., qDebug with runtime enable), or remove it entirely once debugging is complete.
size = taosArrayGetSize(pFill->pFillInfo->next.pRowVal);
for (int32_t i = 0; i < size; ++i) {
SGroupKeys* pKey = taosArrayGet(pFill->pFillInfo->next.pRowVal, i);
pKey->isNull = true;
}
taosMemoryFreeClear(pFill->pFillInfo->pTags);
taosArrayDestroy(pFill->pFillInfo->pColFillProgress);
pFill->pFillInfo->pColFillProgress = NULL;
tdListFreeP(pFill->pFillInfo->pFillSavedBlockList, destroyFillBlock);
pFill->pFillInfo->pFillSavedBlockList = NULL;
int32_t order = (pPhyNode->node.inputTsOrder == ORDER_ASC) ? TSDB_ORDER_ASC : TSDB_ORDER_DESC;
if (order == TSDB_ORDER_ASC) {
pFill->win.skey = pPhyNode->timeRange.skey;
pFill->win.ekey = pPhyNode->timeRange.ekey;
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
42fdb0c to
8466c7a
Compare
- Support FILL clause in external_window queries with options: NONE/NULL/PREV/NEXT/VALUE/NULL_F/VALUE_F - Support FILL clause combined with HAVING clause - Reject unsupported fill modes: LINEAR/NEAR/SURROUND - Add CI test cases (test_external_fill.py) for coverage
8466c7a to
8ec9bb1
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 16 out of 17 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
enh: external_window support FILL clause
Description
Issue(s)
Checklist
Please check the items in the checklist if applicable.