ES|QL: manage INLINE STATS count(*) on result sets with no columns #137017

luigidellaquila · 2025-10-23T10:20:46Z

Fixing queries where INLINE STATS count(*) is calculated on results with no columns, eg.

FROM employees
| KEEP emp_no
| DROP emp_no
| INLINE STATS count(*)

Fixes: #136851

elasticsearchmachine · 2025-10-23T10:21:10Z

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

elasticsearchmachine · 2025-10-23T10:21:35Z

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

luigidellaquila · 2025-10-23T10:22:08Z

...sql/compute/src/main/java/org/elasticsearch/compute/aggregation/CountAggregatorFunction.java


    private int blockIndex() {
-        return countAll ? 0 : channels.get(0);
+        return countAll ? -1 : channels.get(0);


Technically this was wrong: countAll does not rely on blocks, it only relies on page.positionCount

Is the intent here to return an illegal value when countAll is true given that addRawInput does not call blockIndex in that case? E.g. in case a future modification leads to calling page.getBlock(-1) at line 100 ?

Also curious if the intent is to trigger the assert in addIntermediateInput in that case.

Yes, the assert should better protect us now, in case of empty pages.

page.getBlock(-1) will never happen with current implementation, and in case of modifications it will throw a clear error (IMHO better than silently returning a wrong value, because we are calculating results based on the wrong block)

cimequinox · 2025-10-23T12:27:36Z

...sql/compute/src/main/java/org/elasticsearch/compute/aggregation/CountAggregatorFunction.java

+                if (mask.getBoolean(0) == false) {
+                    return;
+                }
+                count = countAll ? block.getPositionCount() : block.getTotalValueCount();


Won't countAll always be false in this path?

Good point, it can be simplified, thanks @cimequinox!

cimequinox

The change relies on properties of the mask when encountering blocks with no values. The calculations appear correct.

Two suggestions:

It would be good to add a comment describing the intent of the -1 constant returned by blockIndex.
The count calculation on line 107 could be simplified to count = block.getTotalValueCount();

luigidellaquila · 2025-10-23T13:30:49Z

Thanks @cimequinox, I pushed a fix for the suggested changes

astefan · 2025-10-23T14:26:21Z

x-pack/plugin/esql/qa/testFixtures/src/main/resources/inlinestats.csv-spec

+from employees
+| keep emp_no
+| drop emp_no
+| inline stats c = count(*)


The fix doesn't seem to be inline stats specific. If so (please, correct me if I'm wrong), there should be use cases where a regular stats should reveal this error, no?

The original query had a STATS as last command.
Unfortunately it doesn't run anymore, because of the limitations introduced by #136981, and without the LIMIT the query works just fine.
(Disabling that limitation, and with this fix, the original query doesn't fail anymore btw)

I suspect that the problem could still happen with STATS, but I couldn't find a way to reproduce it.

I suspect that the problem could still happen with STATS

I mean, it could have happened, but this fix should also cover normal STATS.

I tried to debug the two queries

ROW a = 12 | DROP a | STATS count(*) ROW a = 12 | DROP a | INLINE STATS b = count(*)

The interesting bit is that with INLINE STATS the page in the aggregator has no blocks, while with STATS it still has the original block for a.

I am looking at this

Ok, I've looked at this and your point @luigidellaquila about aggregator having a different behavior for stats vs. inline stats showed that what we do with inline stats has a "manual" step that I did question in code several times so far: marking a plan as optimized without pushing it through another round of optimizations, only because the parts it's made of were already pushed through the logical optimizer.

What happens is that with inline stats we get to execute a plan like this one below where the EsqlProject with empty projection list as a ProjectOperator on the execution layer gets to create a Page with no blocks (because there is nothing to project)

Aggregate[[],[COUNT(*[KEYWORD],true[BOOLEAN]) AS count(*)#11]] \_EsqlProject[[]] \_LocalRelation[[a{r}#9],org.elasticsearch.xpack.esql.plan.logical.local.CopyingLocalSupplier@7cc]

While with regular stats (the same aggregation as above), the optimizer "merges" the two plans:

Rule logical.CombineProjections applied with change Limit[1000[INTEGER],false,false] = Limit[1000[INTEGER],false,false] \_Aggregate[[],[COUNT(*[KEYWORD],true[BOOLEAN]) AS count(*)#117]] = \_Aggregate[[],[COUNT(*[KEYWORD],true[BOOLEAN]) AS count(*)#117]] \_EsqlProject[[]] ! \_Row[[12[INTEGER] AS a#115]] \_Row[[12[INTEGER] AS a#115]] !

And there is no ProjectOperator creating an empty Page for which AggregateOperator not to know what to do with it.

I am not saying the fix in this PR is not necessary @luigidellaquila (you know better than me the Page with no rows story), but the fact that the code reached a point with inline stats that it wouldn't reach with regular stats made me wonder. I have opened a Draft PR to explore an idea that I didn't explore so far because there were no concrete reasons to do it.

Thanks for the details and for the in-depth analysis @astefan!
As long as we have zero-blocks pages, I think it makes sense to have this PR merged, but I agree with you that it would be better to be consistent between STATS and INLINE STATS.

…o_columns' into esql/fix_inline_stats_no_columns

astefan

LGTM

ES|QL: manage INLINE STATS count(*) on result sets with no columns

b024558

luigidellaquila added >bug :Analytics/ES|QL AKA ESQL labels Oct 23, 2025

elasticsearchmachine added the Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) label Oct 23, 2025

elasticsearchmachine added the v9.3.0 label Oct 23, 2025

Update docs/changelog/137017.yaml

6526e5a

luigidellaquila commented Oct 23, 2025

View reviewed changes

Test with ROW

7e4a92d

cimequinox reviewed Oct 23, 2025

View reviewed changes

Simplify + comments

5345455

cimequinox approved these changes Oct 23, 2025

View reviewed changes

Merge branch 'main' into esql/fix_inline_stats_no_columns

b66682d

luigidellaquila enabled auto-merge (squash) October 23, 2025 13:51

astefan reviewed Oct 23, 2025

View reviewed changes

luigidellaquila added 2 commits October 23, 2025 16:53

Merge branch 'main' into esql/fix_inline_stats_no_columns

b2d945e

Merge remote-tracking branch 'luigidellaquila/esql/fix_inline_stats_n…

720bc36

…o_columns' into esql/fix_inline_stats_no_columns

luigidellaquila disabled auto-merge October 23, 2025 15:15

astefan approved these changes Oct 24, 2025

View reviewed changes

luigidellaquila merged commit b05bf64 into elastic:main Oct 24, 2025
34 checks passed

Uh oh!

ES|QL: manage INLINE STATS count(*) on result sets with no columns #137017

ES|QL: manage INLINE STATS count(*) on result sets with no columns #137017

Uh oh!

Conversation

luigidellaquila commented Oct 23, 2025

Uh oh!

elasticsearchmachine commented Oct 23, 2025

Uh oh!

elasticsearchmachine commented Oct 23, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cimequinox Oct 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cimequinox left a comment

Choose a reason for hiding this comment

Uh oh!

luigidellaquila commented Oct 23, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

astefan left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

cimequinox Oct 23, 2025 •

edited

Loading