Skip to content

ESQL: TSTEP #3#144966

Draft
sidosera wants to merge 2 commits intoelastic:mainfrom
sidosera:feature/esql-window-functions-with-offset
Draft

ESQL: TSTEP #3#144966
sidosera wants to merge 2 commits intoelastic:mainfrom
sidosera:feature/esql-window-functions-with-offset

Conversation

@sidosera
Copy link
Copy Markdown
Contributor

@sidosera sidosera commented Mar 25, 2026 •
edited
Loading

Adds support for time-bucket offsets in windowed aggregations.

Stack:

Depends on #144711

This was referenced Mar 25, 2026
Draft
Merged
Merged
@sidosera sidosera added >non-issue Team:StorageEngine :StorageEngine/ES|QL Timeseries / metrics / PromQL / logsdb capabilities in ES|QL labels Mar 25, 2026
@sidosera sidosera requested a review from a team March 25, 2026 18:27
@sidosera
Copy link
Copy Markdown
Contributor Author

sidosera commented Mar 25, 2026

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 25, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 25, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro

Run ID: 761cfb36-9e29-425e-91ab-a4374b160bdc

📥 Commits

Reviewing files that changed from the base of the PR and between a33042d and aae9107.

📒 Files selected for processing (5)
  • server/src/main/java/org/elasticsearch/common/time/DateUtils.java
  • server/src/test/java/org/elasticsearch/common/time/DateUtilsTests.java
  • x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/aggregation/WindowGroupingAggregatorFunction.java
  • x-pack/plugin/esql/compute/src/test/java/org/elasticsearch/compute/aggregation/WindowGroupingDerivAggregatorFunctionTests.java
  • x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/date/TRange.java
📝 Walkthrough

Walkthrough

This pull request introduces offset-aware rounding utilities to DateUtils with three new public methods: roundFloor, roundCeil, and floorRemainder. Corresponding test coverage is added via two new test methods. In the esql compute module, WindowGroupingAggregatorFunction is enhanced to handle null intermediate states by filtering intermediate inputs to only positions with valid state. A new test class WindowGroupingDerivAggregatorFunctionTests validates window-grouped derivation aggregation. Finally, TRange is refactored to expose range boundary literal construction via rangeStartLiteral and rangeEndLiteral public methods, with range computation now accepting a DataType parameter for type-driven conversion logic.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • 🛠️ Update Documentation: Commit on current branch
  • 🛠️ Update Documentation: Create PR

Comment @coderabbitai help to get the list of available commands and usage tips.

@sidosera sidosera force-pushed the feature/esql-window-functions-with-offset branch from aae9107 to 316e350 Compare March 25, 2026 20:40
kkrik-es
kkrik-es reviewed Mar 26, 2026
View reviewed changes
...te/src/main/java/org/elasticsearch/compute/aggregation/WindowGroupingAggregatorFunction.java Outdated
fn.addIntermediateInput(position, oneGroup, page);
continue;
}
if (hasIntermediateState(page, position) == false) {
Copy link
Copy Markdown
Contributor

@kkrik-es kkrik-es Mar 26, 2026

Choose a reason for hiding this comment

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

Nit: check for true and move the try-block inside this condition?

@kkrik-es kkrik-es requested a review from dnhatn March 26, 2026 09:11
kkrik-es
kkrik-es reviewed Mar 26, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@kkrik-es kkrik-es left a comment

Choose a reason for hiding this comment

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

Looks reasonable, I assume you need #4 to add csv tests with window functions..

Nhat can approve.

@sidosera sidosera marked this pull request as ready for review March 26, 2026 11:10
@sidosera sidosera requested a review from a team as a code owner March 26, 2026 11:10
@elasticsearchmachine
Copy link
Copy Markdown
Collaborator

elasticsearchmachine commented Mar 26, 2026

Pinging @elastic/es-storage-engine (Team:StorageEngine)

@sidosera sidosera force-pushed the feature/esql-window-functions-with-offset branch from 316e350 to b365e2e Compare March 26, 2026 14:14
@sidosera sidosera marked this pull request as draft March 26, 2026 22:19
@sidosera
Refactored
bca9458 
Merge positionsWithIntermediateState and filterPage into single-pass addInitialIntermediateInput

Uses Block.asVector() to detect dense blocks upfront: all-vector fast path skips null
scanning entirely. For mixed pages, a single position scan replaces two separate passes
(null-finding then block.filter), and nullable blocks are identified per-block rather than
per-position.

Also removes stale IntArrayList import from TimeSeriesAggregationOperator.

Add CSV tests for window functions with offset TBUCKET

Add logic to handle time bucket offset in window functions
@sidosera sidosera force-pushed the feature/esql-window-functions-with-offset branch from 685f9cf to bca9458 Compare March 27, 2026 13:14
@sidosera sidosera force-pushed the feature/esql-window-functions-with-offset branch from a8f6db3 to 16105e6 Compare March 28, 2026 00:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kkrik-es kkrik-es kkrik-es left review comments

@dnhatn dnhatn Awaiting requested review from dnhatn

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

>non-issue :StorageEngine/ES|QL Timeseries / metrics / PromQL / logsdb capabilities in ES|QL Team:StorageEngine v9.4.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@sidosera @elasticsearchmachine @kkrik-es