To be useful with aggs, we'd need to move PropagateEvalFoldables into the Substitutions batch and run this more than once - but we can worry about that later.

Sure - it would be good to see whether it should be ran more than once since Propagate runs across the whole plan.

Ideally, we need to see in a unit test that sum([1,2]) is in fact mv_sum * count.
This test here is ok, but it doesn't really check the transformation we are doing for sum([1,2]).

Updated - now the tests only check the result of the substitutions, and in particular we check whether we replace avg by mv_avg etc.

This is probably an error here with row.

Discussed with Andrei:

• We both thought that it would be nicer to use a LocalRelation with 0 attributes and a subsequent Eval on top
  that contains s = mv_avg(...), ... - esp. given that Row is mostly used in tests and not production.
• It turns out that LocalRelation with 0 attributes is not correct, though, as this would automatically have 0 rows as well; it seems that Row is the most fitting concept, here, after all.

To stick closer to the original code in SubstituteSurrogates, I handle this case now by

• creating a placeholder Row when we end up not having any actual aggs, and
• handling the evaluation of s = mv_avg(...) in a subsequent eval - this required less change, actually, as that's how we handle the avg surrogate substitution today.

Use a LocalRelation instead of Row which returns the necessary field - in fact, we should retrofit Row on top of LocalRelation sooner rather than later (and look into removing the row operator as well).

Sure - it would be good to see whether it should be ran more than once since Propagate runs across the whole plan.

Use a LocalRelation instead of Row which returns the necessary field - in fact, we should retrofit Row on top of LocalRelation sooner rather than later (and look into removing the row operator as well).

No need for this hack - create a LocalRelation with an output schema and no values:
plan = new LocalRelation(source, new Alias...,, LocalSupplier.EMPTY)

Btw, what is the point of the alias - who's the consumer? Why not use the initial plan output?

Attempted using LocalRelation this way already - the problem is that new LocalRelation(source, new Alias...,, LocalSupplier.EMPTY) creates 0 rows, but I need exactly 1 row instead.

What I can do is doing a similar hack, using a placeholder attribute (or the original output schema), with a LocalRelation with just 1 row; this will avoid further relying on RowExec.

The point of the $$placeholder attribute is that currently, plans with empty rows do not seem to be allowed, resp. not handled correctly; it'd be cleaner if that worked, though, but making that work is beyond the scope of this PR IMHO.

@costin and I discussed another, similar approach using LocalRelation, namely replacing

... | STATS a = AVG([1,2]), m = min([1,2,3])

by

LocalRelation(source, attributes, singleRowLocalSupplier)`

where attributes consists of a and m, and singleRowLocalSupplier has a single row containing the values MV_AVG([1,2]) and MV_MIN([1,2,3]) (folded).

I don't think this is the best approach for the following reasons:

• Creating the singleRowLocalSupplier requires manually creating blocks with the folded values - this means that in addition to our ConstantFolding optimizer rule, we have another place that does constant folding; and we need to manually create proper blocks this time rather than literals, meaning that more low-level block handling leaks into the planner code.
• This does not benefit from optimization rules that prune unused Eval expressions, while the present approach does.
• This opens up 1 special edge case in which we do not use an Eval to compute the values of the surrogate expressions but need to do this manually; this may require additional edge case handling later on.

Imo, this a too long explanation which is also a bit misleading. Could be shorter, but it's ok.
ROW placeholder = null is the misleading part. There is essentially no row (row in itself a command and can be confusing) node created. And also there is no placeholder element. If you really want an esql command in there as an explanation: from _nothing_ | eval .....

Simplified!

This isn't necessary as the block is always the same; I'm pretty sure we have something similar declared somewhere else inside the code.
Just create the relationship in place no need for an emptyRow method.
Speaking of, propagate the source instead of passing in Source.EMPTY - that's never a good solution inside the planner.

Inlined and shortened!

What Andrei said. Better to have this comment inside SurrogateExpression for future guidance - here it should be just one or two lines, explaining what's going on.