feat: add STMT/STMT2 support for external_window queries#35217
feat: add STMT/STMT2 support for external_window queries#35217
Conversation
There was a problem hiding this comment.
Code Review
This pull request enables support for external windows in both STMT and STMT2 APIs, specifically handling the shortcut path where execute is called immediately after prepare without parameter binding. Key changes include updating statement status transitions, deferring query translation in the parser to correctly identify query types, and ensuring deep copies of subquery nodes during planning. Feedback focuses on source/client/src/clientStmt2.c, highlighting a missing setQueryFp callback in the SParseContext initialization and potential memory leaks when handling metadata if functions fail or if request objects are reused.
There was a problem hiding this comment.
Pull request overview
Adds support for running external_window(...) queries through both STMT and STMT2 prepared-statement APIs, including the “prepare → execute” shortcut path for 0-parameter SELECTs.
Changes:
- Enable
external_windowtranslation in STMT mode and add/adjust regression tests for STMT + STMT2. - Fix cloning/ownership for external window subquery nodes across logic/physical planning and node cloning/destruction.
- Update STMT/STMT2 execution state machine to allow
EXECUTEdirectly fromPREPAREand perform deferred translation when no params are bound.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| test/cases/13-TimeSeriesExt/08-ExternalWindow/test_external.py | Converts STMT external_window from negative to positive assertions and adds STMT2 coverage, including 0-param prepare→execute path. |
| source/libs/planner/src/planPhysiCreater.c | Deep-clones external window subquery when building physical plan nodes. |
| source/libs/planner/src/planLogicCreater.c | Deep-clones external window subquery when building logic plan nodes (incl. stream path). |
| source/libs/parser/src/parser.c | Defers translation for STMT non-INSERT queries to keep stmt query-type detection correct for 0-param SELECT. |
| source/libs/parser/src/parTranslater.c | Removes prior STMT guard that rejected external_window in prepared queries. |
| source/libs/nodes/src/nodesUtilFuncs.c | Ensures external window subquery is destroyed with window logic nodes. |
| source/libs/nodes/src/nodesCloneFuncs.c | Adds clone support for QUERY_NODE_EXTERNAL_WINDOW nodes (incl. subquery). |
| source/client/src/clientStmt2.c | Allows STMT2 execute from PREPARE and adds shortcut parsing/binding/translation flow for 0-param SELECT. |
| source/client/src/clientStmt.c | Same as above for STMT v1 API: allow execute from PREPARE and complete translation for 0-param SELECT. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Remove stmtBindVersion guard in translateWindow() to allow STMT mode - Add externalWindowNodeCopy() in nodesCloneFuncs.c for correct clone - Defer SELECT translation in analyseSemantic() when pStmtCb != NULL - Allow STMT_EXECUTE from STMT_PREPARE status for query type - Fix use-after-free: set pResSchema/pResExtSchema to NULL after ownership transfer - Fix direct return while holding error path: use goto _return pattern - Fix stale prevStatus: capture after async-bind wait completes - Add stmt_external_window_regression() and stmt2_external_window_regression()
c9a86be to
2e2de5e
Compare
Description
Issue(s)
Checklist
Please check the items in the checklist if applicable.