Enh/external window support fill2 by facetosea · Pull Request #35083 · taosdata/TDengine

facetosea · 2026-04-08T06:24:07Z

Description

Issue(s)

Close/close/Fix/fix/Resolve/resolve: Issue Link

Checklist

Please check the items in the checklist if applicable.

Is the user manual updated?
Are the test cases passed and automated?
Is there no significant decrease in test coverage?

gemini-code-assist

Code Review

This pull request implements the FILL clause for EXTERNAL_WINDOW queries, supporting modes such as NULL, VALUE, PREV, and NEXT. The implementation covers documentation, grammar updates, plan generation, and executor logic. Review feedback points out a potential crash in extWinAppendAggFilledRow due to uninitialized pointer usage during error jumps and identifies redundant fill mode checks when processing adjacent rows.

Copilot

Pull request overview

This PR adds FILL clause support for EXTERNAL_WINDOW queries, plumbing fill mode/values through the parser → planner → physical-plan serialization → executor, and adding regression tests + documentation updates around the supported/unsupported fill matrix.

Changes:

Extend SQL grammar + translation to accept EXTERNAL_WINDOW(... fill(...)), validate unsupported modes (LINEAR/NEAR/SURROUND), and enforce “aggregate-only” fill usage.
Carry external-window fill metadata through logical/physical planning, cloning, TLV/JSON encode/decode, and node destruction.
Implement executor-side external-window fill behaviors (NONE/NULL/NULL_F/VALUE/VALUE_F/PREV/NEXT), and add CI coverage + docs.

Reviewed changes

Copilot reviewed 16 out of 17 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
test/ci/cases.task	Adds CI execution entry for new external-window fill test module
test/cases/13-TimeSeriesExt/08-ExternalWindow/test_external.py	Updates existing regression to reflect fill now supported; adds fill support matrix assertions + negatives
test/cases/13-TimeSeriesExt/08-ExternalWindow/test_external_fill.py	New comprehensive regression suite for external-window fill (and related interval-fill comparisons)
source/libs/planner/src/planPhysiCreater.c	Propagates external fill info into physical external-window plan nodes; maps fill expressions to slotted targets
source/libs/planner/src/planLogicCreater.c	Builds external-window fill expr list from projection; rewrites it post-select rewrite; adjusts HAVING placement for interval+fill
source/libs/parser/src/parTranslater.c	Enables fill handling for external window, adds aggregate-only restriction, and routes fill-values validation to external-window fill nodes
source/libs/parser/src/parAstCreater.c	Ensures external-window fill node uses the correct primary-key timestamp column (`pWStartTs`)
source/libs/parser/inc/sql.y	Extends `external_window_fill_opt` grammar to support value/position fill modes
source/libs/nodes/src/nodesUtilFuncs.c	Adds destruction for embedded external fill info in logic/physical nodes
source/libs/nodes/src/nodesMsgFuncs.c	Adds TLV serialization fields for external-window fill mode/exprs/values
source/libs/nodes/src/nodesCodeFuncs.c	Adds JSON serialization fields for external-window fill mode/exprs/values
source/libs/nodes/src/nodesCloneFuncs.c	Adds clone support for external-window fill fields (logic + physical nodes)
include/libs/nodes/plannodes.h	Introduces `SExtWindowFillInfo` embedded into window logic/physical plan nodes
source/libs/executor/src/externalwindowoperator.c	Implements executor-side external-window fill behavior and filter integration
source/libs/executor/src/filloperator.c	Minor formatting-only change
docs/zh/05-basic/03-query.md	Documents external-window fill modes and behavior
docs/en/05-basic/03-query.md	Documents external-window fill modes and behavior

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

- 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

… not 0

Copilot

Pull request overview

Copilot reviewed 20 out of 21 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (4)

docs/zh/14-reference/03-taos-sql/24-distinguished.md:56

The window_clause grammar snippet lists EXTERNAL_WINDOW ((subquery) window_alias) twice (once with [fill_clause] and again without). This duplication is confusing and should be reduced to a single entry reflecting the supported syntax (with optional [fill_clause]).

```sql
window_clause: {
    SESSION(ts_col, tol_val)
  | STATE_WINDOW(expr[, extend[, zeroth_state]]) [TRUE_FOR(true_for_expr)]
  | INTERVAL(interval_val [, interval_offset]) [SLIDING (sliding_val)] [fill_clause]
  | EXTERNAL_WINDOW ((subquery) window_alias) [fill_clause]
  | EVENT_WINDOW START WITH start_trigger_condition END WITH end_trigger_condition [TRUE_FOR(true_for_expr)]
  | COUNT_WINDOW(count_val[, sliding_val][, col_name ...])
  | EXTERNAL_WINDOW ((subquery) window_alias)
}

docs/en/14-reference/03-taos-sql/24-distinguished.md:54

The window_clause grammar snippet lists EXTERNAL_WINDOW ((subquery) window_alias) twice (once with [fill_clause] and again without). Please remove the redundant entry so the documentation shows a single canonical syntax.

```sql
window_clause: {
    SESSION(ts_col, tol_val)
  | STATE_WINDOW(expr [, extend[, zeroth_state]]) [TRUE_FOR(true_for_expr)]
  | INTERVAL(interval_val [, interval_offset]) [SLIDING (sliding_val)] [fill_clause]
  | EXTERNAL_WINDOW ((subquery) window_alias) [fill_clause]
  | EVENT_WINDOW START WITH start_trigger_condition END WITH end_trigger_condition [TRUE_FOR(true_for_expr)]
  | COUNT_WINDOW(count_val[, sliding_val][, col_name ...])
  | EXTERNAL_WINDOW ((subquery) window_alias)
}

docs/zh/14-reference/03-taos-sql/20-select.md:65

The window_clause syntax block includes EXTERNAL_WINDOW ((subquery) window_alias) twice (with and without [fill_clause]). Keeping both makes the grammar ambiguous; it should be a single production with optional [fill_clause].

window_clause: {
    SESSION(ts_col, tol_val)
  | STATE_WINDOW(expr [, extend[, zeroth_state]]) [TRUE_FOR(true_for_expr)]
  | INTERVAL(interval_val [, interval_offset]) [SLIDING (sliding_val)] [fill_clause]
  | EXTERNAL_WINDOW ((subquery) window_alias) [fill_clause]
  | EVENT_WINDOW START WITH start_trigger_condition END WITH end_trigger_condition [TRUE_FOR(true_for_expr)]
  | COUNT_WINDOW(count_val[, sliding_val][, col_name ...])
  | EXTERNAL_WINDOW ((subquery) window_alias)
}

docs/en/14-reference/03-taos-sql/20-select.md:63

The window_clause syntax block contains duplicate EXTERNAL_WINDOW ((subquery) window_alias) entries (with and without [fill_clause]). Please remove the redundant line and keep one canonical syntax with optional [fill_clause].

window_clause: {
    SESSION(ts_col, tol_val)
  | STATE_WINDOW(expr [, extend[, zeroth_state]]) [TRUE_FOR(true_for_expr)]
  | INTERVAL(interval_val [, interval_offset]) [SLIDING (sliding_val)] [WATERMARK(watermark_val)] [fill_clause]
  | EXTERNAL_WINDOW ((subquery) window_alias) [fill_clause]
  | EVENT_WINDOW START WITH start_trigger_condition END WITH end_trigger_condition [TRUE_FOR(true_for_expr)]
  | COUNT_WINDOW(count_val[, sliding_val][, col_name ...])
  | EXTERNAL_WINDOW ((subquery) window_alias)
}

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

dapan1121

提交到3.0分支

dapan1121 · 2026-04-13T00:28:42Z

 #include "stream.h"
 #include "filter.h"
 #include "cmdnodes.h"
+#include "../../function/inc/functionResInfoInt.h"


不要使用这种头文件方式

dapan1121 · 2026-04-13T00:49:14Z

+        if (pFunc->funcType == FUNCTION_TYPE_GROUP_KEY ||
+            pFunc->funcType == FUNCTION_TYPE_GROUP_CONST_VALUE) continue;
+        SNode* pClone = NULL;
+        PLAN_ERR_RET(nodesCloneNode(pExpr, &pClone));


应该用PLAN_ERR_JRET

dapan1121 · 2026-04-13T00:50:32Z

+        }
+        ++idx;
+      }
+      if (pSlotted != NULL) {


缺少错误处理

dapan1121 · 2026-04-13T00:53:50Z

  }

-  if (TSDB_CODE_SUCCESS == code && NULL != pSelect->pHaving) {
+  if (TSDB_CODE_SUCCESS == code && NULL != pSelect->pHaving && !havingHandledByFill) {


这个改动看上去是对的，但是没有新增测试用例，原来的测试用例有吗

dapan1121 · 2026-04-13T00:54:36Z

@@ -58,6 +58,7 @@ window_clause: {
    SESSION(ts_col, tol_val)
  | STATE_WINDOW(expr [, extend[, zeroth_state]]) [TRUE_FOR(true_for_expr)]
  | INTERVAL(interval_val [, interval_offset]) [SLIDING (sliding_val)] [fill_clause]


EXTERNAL_WINDOW 出现了两遍

dapan1121 · 2026-04-13T00:57:47Z

+            "external_window((select ts, endtime, mark from ext_cx_win) w) "
+            "fill(none)",
        )
+        tdSql.checkRows(4)


这种只校验行数太宽泛了

dapan1121 · 2026-04-13T00:59:12Z

+            "external_window((select ts, endtime, mark from ext_fill_win) w) fill(null) "
+            "order by ws"
+        )
+        tdSql.checkRows(4)


这种比较方式太落后了，直接采用输出文件对比即可

facetosea · 2026-04-17T01:00:24Z

转 3.0 分支

facetosea requested review from a team, dapan1121, guanshengliang and zitsen as code owners April 8, 2026 06:24

Copilot AI review requested due to automatic review settings April 8, 2026 06:24

taosdata-bot bot added the team engine label Apr 8, 2026

Copilot started reviewing on behalf of facetosea April 8, 2026 06:25 View session

gemini-code-assist bot reviewed Apr 8, 2026

View reviewed changes

Comment thread source/libs/executor/src/externalwindowoperator.c

Comment thread source/libs/executor/src/externalwindowoperator.c Outdated

Comment thread source/libs/executor/src/externalwindowoperator.c Outdated

Copilot AI reviewed Apr 8, 2026

View reviewed changes

Comment thread docs/en/05-basic/03-query.md

Comment thread docs/zh/05-basic/03-query.md Outdated

Comment thread source/libs/planner/src/planLogicCreater.c

facetosea added 5 commits April 10, 2026 10:34

docs: add FILL clause description for external_window in query guide

674b21b

fix: external_window fill(null) empty window count(*) should be NULL,…

5b9a13c

… not 0

fix: memleak

56b515f

fix: align external_window fill syntax

836797f

Copilot AI review requested due to automatic review settings April 10, 2026 02:48

facetosea force-pushed the enh/externalWindowSupportFill2 branch from 2b2feb3 to 836797f Compare April 10, 2026 02:48

Copilot started reviewing on behalf of facetosea April 10, 2026 02:48 View session

Copilot AI reviewed Apr 10, 2026

View reviewed changes

docs: format

ec869c7

dapan1121 reviewed Apr 13, 2026

View reviewed changes

facetosea closed this Apr 17, 2026

tomchon deleted the enh/externalWindowSupportFill2 branch April 17, 2026 05:53

Conversation

facetosea commented Apr 8, 2026

Description

Issue(s)

Checklist

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

dapan1121 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dapan1121 Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

dapan1121 Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

dapan1121 Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

dapan1121 Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

dapan1121 Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

dapan1121 Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

dapan1121 Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

facetosea commented Apr 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

dapan1121 left a comment •

edited

Loading