Conversation
1c5f1d6 to
13aac4c
Compare
e150e13 to
156e275
Compare
🔍 Preview links for changed docs |
ℹ️ Important: Docs version tagging👋 Thanks for updating the docs! Just a friendly reminder that our docs are now cumulative. This means all 9.x versions are documented on the same page and published off of the main branch, instead of creating separate pages for each minor version. We use applies_to tags to mark version-specific features and changes. Expand for a quick overviewWhen to use applies_to tags:✅ At the page level to indicate which products/deployments the content applies to (mandatory) What NOT to do:❌ Don't remove or replace information that applies to an older version 🤔 Need help?
|
...te/src/main/java/org/elasticsearch/compute/aggregation/WindowGroupingAggregatorFunction.java
Outdated
Show resolved
Hide resolved
|
@leontyevdv for thoughts on how to integrate this with your window function change. |
40e0041 to
deb633e
Compare
|
Pinging @elastic/es-storage-engine (Team:StorageEngine) |
|
This is a very large PR. Could you split it up into smaller chunks? When this is in, we should be able to support PromQL instant queries where start=end, right? TSTEP should also support specifying the number of target buckets similar to TBUCKET, see #144057 and #142747. These can be separate changes. |
|
@elasticmachine test this please |
|
@elasticmachine test this please |
Yeah, fair point. GitHub still makes stacked changes pretty awkward to manage, imo, so I usually avoid splitting things too much unless the diff starts getting hard to review. But this one is probably at that point, so I split it up a bit to make it easier to digest.
Most likely, yes. But I didn't check that explicitly. We already use inclusive bounds, so
Agreed. We'll follow up on that separately. |
379805c to
ba2e060
Compare
...c/main/java/org/elasticsearch/xpack/esql/core/querydsl/QueryDslTimestampBoundsExtractor.java
Show resolved
Hide resolved
...c/main/java/org/elasticsearch/xpack/esql/core/querydsl/QueryDslTimestampBoundsExtractor.java
Outdated
Show resolved
Hide resolved
x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlSession.java
Show resolved
Hide resolved
...c/main/java/org/elasticsearch/xpack/esql/core/querydsl/QueryDslTimestampBoundsExtractor.java
Show resolved
Hide resolved
I agree it's less than ideal. We should still avoid creating PRs with > 500 lines changed. Some recent learnings I had to make it a bit more bearable:
|
ba2e060 to
6590a18
Compare
6590a18 to
2e1b850
Compare
|
@elasticmachine test this please |
dnhatn
left a comment
There was a problem hiding this comment.
One comment, but looks good. And sorry for the noise, Claude posted its review without asking.
| * to resolve date math expressions consistently with the current request. | ||
| */ | ||
| @Nullable | ||
| public static TimestampBounds extractTimestampBounds(@Nullable QueryBuilder filter, LongSupplier nowSupplier) { |
There was a problem hiding this comment.
Can we remove the extractTimestampBounds without nowSupplier
There was a problem hiding this comment.
Sure. Will follow-up in the next PRs.
|
Test fails with multi-value errors in LIKE/RLIKE operators. This is unrelated to TSTEP. |
Small refactoring to handle time patterns needed for TBUCKET/TSTEP bucketing.
Issue: #139187
Stack: