ESQL: Reorganize optimizer rules

[alex-spies]

Closes #111317

Organize the optimizer rules consistently for all 4 optimizers (logical, physical, local logical, local physical) and move helper methods meant for optimizer rules out of the optimizers into the relevant rules or into helper classes. Make the optimizers now only contain code relevant to the actual optimizer definition, i.e. which batches of rules they use.

Also, consolidate the 2 nearly identical logical ParameterizedRules into one.

New package structure:

package org.elasticsearch.xpack.esql.optimizer
└── rules
    ├── logical
    │   ├── local
    └── physical
        └── local

New helper classes:

- org.elasticsearch.xpack.esql.optimizer.rules.logical.pushdown.PushDownUtils
- org.elasticsearch.xpack.esql.optimizer.rules.logical.TemporaryNameUtils
- org.elasticsearch.xpack.esql.optimizer.rules.physical.local.LucenePushDownUtils

[costin]

Thanks for looking into this.
Quick feedback: at the logical level, the pushdown and local are on the same level which is for me confusing as local calls rules form the parent package as well; I suggest getting rid of the pushdown package and moving the place next to the rest of the other rules as there's nothing special about them.
lucenepushdown - quite the mouthful. At least at the moment it's the only time of pushdown possible in a local node and conceptually related to the other rules. I suggest to move these rules under local.

So the final structure looks something like:

package org.elasticsearch.xpack.esql.optimizer
└── rules
    ├── logical
    │   ├── local
    └── physical
        └── local

+1 on the separating the utils - even better make them protected.

[elasticsearchmachine]

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

[alex-spies]

@costin, the structure is now the one you suggested + I made the new utils package private.

[costin]

LGTM

[costin]

The name should indicate the role of this class - checking the consistency of dependencies. Right now it sounds like a DTO class which contains the dependencies consistency.

How about something more mundane but clearer such as DependencyChecker, PlanStateChecker or ValidatePlanState?

[astefan]

This class was just extracted from an already existent one, but it wouldn't hurt a rename tbh.

Thinking about the name, I realized we don't have a javadoc for it to explain its intention. How about adding a sentence there?

Leaving the javadoc aside and just looking at the code, I am throwing another name suggestion in the bucket, but up to @alex-spies to make the change or not altogether: PlanConsistencyChecker (the reasoning behind the name is that this is a checker class and does something, an action; it looks at the "inputs" and "outputs" of each node plan if there is an inconsistency there it spits out an error, thus the PlanConsistency part). My 2c.

[alex-spies]

I went with Andrei's suggestion :)

[astefan]

LGTM. Thanks, Alex.

I've left some comments mostly spawning from personal preferences and experience. Feel free to adopt or ignore them.

[astefan]

This class was just extracted from an already existent one, but it wouldn't hurt a rename tbh.

Thinking about the name, I realized we don't have a javadoc for it to explain its intention. How about adding a sentence there?

Leaving the javadoc aside and just looking at the code, I am throwing another name suggestion in the bucket, but up to @alex-spies to make the change or not altogether: PlanConsistencyChecker (the reasoning behind the name is that this is a checker class and does something, an action; it looks at the "inputs" and "outputs" of each node plan if there is an inconsistency there it spits out an error, thus the PlanConsistency part). My 2c.

[alex-spies]

Thanks for the speedy reviews @astefan and @costin !