Skip to content

feat: add STMT/STMT2 support for external_window queries#35217

Open
facetosea wants to merge 1 commit into3.0from
feat/external-window-stmt-support2
Open

feat: add STMT/STMT2 support for external_window queries#35217
facetosea wants to merge 1 commit into3.0from
feat/external-window-stmt-support2

Conversation

@facetosea
Copy link
Copy Markdown
Contributor

  • 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

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?

Copilot AI review requested due to automatic review settings April 23, 2026 03:25
@facetosea facetosea requested review from a team, dapan1121 and guanshengliang as code owners April 23, 2026 03:25
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread source/client/src/clientStmt2.c
Comment thread source/client/src/clientStmt2.c
Comment thread source/client/src/clientStmt2.c
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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_window translation 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 EXECUTE directly from PREPARE and 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.

Comment thread source/libs/parser/src/parser.c Outdated
Comment thread source/client/src/clientStmt2.c
Comment thread source/client/src/clientStmt.c
- 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()
@facetosea facetosea force-pushed the feat/external-window-stmt-support2 branch from c9a86be to 2e2de5e Compare April 23, 2026 06:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants