ReplaceMissingFieldWithNull rule lead to huge number of eval operators

[dnhatn]

There is an OOM report from a simple query FROM some-index | LIMIT 10. I found in the heap dump that some Drivers has more 16K eval operators. I believe the rule ReplaceMissingFieldWithNull could translate missing fields to an Eval. This lead to many eval operators if the target indices have many missing fields. Since 8.12, ValuesSourceReaderOperator can now read multiple fields at the same time. I think we need to combine these Literals in a single EvalOperator like we do with ValuesSourceReaderOperator.

[elasticsearchmachine]

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

[nik9000]

And the field loading infrastructure detects missing fields and loads them with null without a lot of ceremony either.

[dnhatn]

@nik9000 Do you mean that we should just remove that rule and have the ValuesSourceReaderOperator take care of this instead?

[nik9000]

Maybe so! I don't have the code in front of me and I think it'd be useful to make sure we can merge the reads into a single operator run. But presuming we can do that I think that's fine.

[alex-spies]

@nik9000 Do you mean that we should just remove that rule and have the ValuesSourceReaderOperator take care of this instead?

If that's easily possible, I think that'd be the best way to go about this.
#104586 fixes the problem at hand, so I think it's no problem to merge it - but removing ReplaceMissingFieldWithNull would lead to a less complex solution IMHO.

[nik9000]

Go ahead and merge, yeah. That get's us out of trouble. But it'd be nice to have a good story around why it's better to do this with eval. The readers are indeed pretty good at this.

[astefan]

The presence of the eval can simplify some queries and transform them in a local relation (not even reaching the Lucene level). I don't think it's so easy to determine if the presence of the rule is more beneficial than just having good readers. This might be one of those "features" that can prove their usefulness (or not) with time and edge cases. The logic behind it is pretty strong and I wouldn't discard it without a very good reason.

[luigidellaquila]

My 2 cents, optimizations tend to increase complexity: every optimization is one more piece of code to maintain, one more moving part that can interact/overlap/clash with other components, and a potential source of bugs.
Probably this specific case is particularly unlucky, because all the three above seem to happen at the same time.
I think any optimization we have in the code should be justified by a quantitative analysis of the benefits it brings (eg. in terms of saved query execution time in common scenarios). If the performance benefits are not measurable, then it's better to avoid it.