Skip to content

Feature/count by trunc with filter#138765

Merged
GalLalouche merged 11 commits intoelastic:mainfrom
GalLalouche:feature/count_by_trunc_with_filter
Dec 11, 2025
Merged

Feature/count by trunc with filter#138765
GalLalouche merged 11 commits intoelastic:mainfrom
GalLalouche:feature/count_by_trunc_with_filter

Conversation

@GalLalouche
Copy link
Contributor

@GalLalouche GalLalouche commented Nov 28, 2025

This PR adds filter support for pushing down COUNT(*) BY DATE_TRUNC introduced in #138023, i.e.,

FROM idx 
| WHERE date > NOW() - 90 DAYS 
| STATS COUNT(*) BY DATE_TRUNC(1 DAY, date)

Note that we still won't push down queries where the filter is on the COUNT(*) since that case is a lot more complicated due to having to keep zeros for filtered buckets:

FROM idx
| STATS COUNT(*) WHERE date > NOW() - 90 DAYS BY DATE_TRUNC(1 DAY, date)
@GalLalouche GalLalouche added >feature Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) :Analytics/ES|QL AKA ESQL labels Nov 28, 2025
@elasticsearchmachine
Copy link
Collaborator

Hi @GalLalouche, I've created a changelog YAML for you.

@GalLalouche GalLalouche force-pushed the feature/count_by_trunc_with_filter branch from a801562 to 93e6a63 Compare November 28, 2025 15:09
@GalLalouche GalLalouche marked this pull request as ready for review November 28, 2025 22:20
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-analytical-engine (Team:Analytics)

Copy link
Member

@fang-xing-esql fang-xing-esql left a comment

Choose a reason for hiding this comment

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

Thank you @GalLalouche! I wonder if it would also be beneficial to merge filters on the RoundTo field for queries that don't have count?

@GalLalouche
Copy link
Contributor Author

Thank you @GalLalouche! I wonder if it would also be beneficial to merge filters on the RoundTo field for queries that don't have count?

Yeah, I agree, in fact, we can merge filters on any QueryBuilder regardless of the operation! Let me create an issue for this: #139191, since I think it's out of scope for this PR.

Copy link
Member

@fang-xing-esql fang-xing-esql left a comment

Choose a reason for hiding this comment

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

LGTM, thank you @GalLalouche !

@GalLalouche GalLalouche enabled auto-merge (squash) December 10, 2025 16:32
@GalLalouche GalLalouche merged commit 73e1f5e into elastic:main Dec 11, 2025
33 of 34 checks passed
szybia added a commit to szybia/elasticsearch that referenced this pull request Dec 11, 2025
* upstream/main: (79 commits)
  Mute org.elasticsearch.test.rest.yaml.CcsCommonYamlTestSuiteIT test {p0=search/140_pre_filter_search_shards/prefilter on non-indexed date fields} elastic#139381
  Adjust error bounds for bfloat16 value checks (elastic#139371)
  Unmute some vector CSS tests (elastic#139370)
  Do not allow `project_routing` as a query param (elastic#139206)
  Unmute HalfFloat...Tests#testSynthesizeArrayRandom (elastic#139341)
  Remove leniency in LinkedProjectConfig builder methods (elastic#139012)
  EQL: fix project_routing (elastic#139366)
  Add patch version for 9.2 index version constant (elastic#139362)
  Mute org.elasticsearch.test.rest.yaml.RcsCcsCommonYamlTestSuiteIT test {p0=search.vectors/200_dense_vector_docvalue_fields/dense_vector docvalues with bfloat16} elastic#139368
  ES|QL: Enable CCS tests for FORK (elastic#139302)
  Restructuring the semantic_text field type page  (elastic#138571)
  AggregateMetricDouble fields should not build BKD indexes (elastic#138724)
  Feature/count by trunc with filter (elastic#138765)
  ESQL: Convert TS 500 error to 400 (elastic#139360)
  [CI] Rerun failing tests for periodic build pipelines (elastic#139200)
  revert muting saml test (elastic#139327)
  Add TDigest histogram as metric (elastic#139247)
  Links solved bugs to class cast exception changelog and unmutes errors (elastic#139340)
  Ensure integer sorts are rewritten to long sorts for BWC indexes (elastic#139293)
  Integrate stored fields format bloom filter with synthetic _id (elastic#138515)
  ...
GalLalouche added a commit that referenced this pull request Jan 19, 2026
This PR adds golden tests to ES|QL!

● I added a GoldenTestsReadme.md with instructions on how to add new golden tests.
● I tried to keep this (relatively) minimal for the first PR. In the future we can add more features, configurations, stages (e.g., local node_reduce), etc.
● As a proof of concept, I've re-implemented (most) of the tests in SubstitudeRoundToTests using the golden testing framework.
○ When I recently worked on these tests (#138023,#138765), the majority of my time was spent mechanically fixing the expected output in the tests.
○ This was a good use case since I needed to add more support for test nesting due to the loops in the original tests.
○ While more text is being produced when using golden tests (70k lines!), fixing the expected output is a lot simpler, since you can just bulldoze the output! In this case, eyeballing the difference is enough to be convinced that the change is correct. So the maintenance cost goes down.
○ One of the tests (testRoundToWithTimeSeriesIndices) was skipped because its output isn't actually consistent across runs. The original test only verifies a very specific subset of the AST, so it's not a good candidate for golden tests.
spinscale pushed a commit to spinscale/elasticsearch that referenced this pull request Jan 21, 2026
This PR adds golden tests to ES|QL!

● I added a GoldenTestsReadme.md with instructions on how to add new golden tests.
● I tried to keep this (relatively) minimal for the first PR. In the future we can add more features, configurations, stages (e.g., local node_reduce), etc.
● As a proof of concept, I've re-implemented (most) of the tests in SubstitudeRoundToTests using the golden testing framework.
○ When I recently worked on these tests (elastic#138023,elastic#138765), the majority of my time was spent mechanically fixing the expected output in the tests.
○ This was a good use case since I needed to add more support for test nesting due to the loops in the original tests.
○ While more text is being produced when using golden tests (70k lines!), fixing the expected output is a lot simpler, since you can just bulldoze the output! In this case, eyeballing the difference is enough to be convinced that the change is correct. So the maintenance cost goes down.
○ One of the tests (testRoundToWithTimeSeriesIndices) was skipped because its output isn't actually consistent across runs. The original test only verifies a very specific subset of the AST, so it's not a good candidate for golden tests.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Analytics/ES|QL AKA ESQL >feature Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) v9.3.0

3 participants