Add Dependency Checker for LogicalLocalPlanOptimizer

[julian-elastic]

Add verification for LocalLogical plan
The verification is skipped if there is remote enrich, similar to how it is skipped for LocalPhysical plan optimization

[elasticsearchmachine]

Hi @julian-elastic, I've created a changelog YAML for you.

[elasticsearchmachine]

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

[alex-spies]

Thanks @julian-elastic ! This is close.

I'd add a test that demonstrates that the verifier actually works so accidentally disabling it becomes noticeable in CI. Check out LogicalPlanOptimizerTests#testPlanSanityCheck - I think we need a similar test in the LocalLogicalPlanOptimizerTests. Also, I noticed that there is PhysicalPlanOptimizerTests#testVerifierOnMissingReferences but no corresponding test in LocalPhysicalPlanOptimizerTests - it'd be nice if we could fix that, although it's technically out of scope for this PR.

[bpintea]

It'd be great to also add the justification for the change; i.e. what changes are now possible that the original check wouldn't catch (I guess triggered by folding a field missing locally). Noting it as we've been trying to shave planning costs.

Also, one check which would be great (since it caused at least two bugs), but not done atm. is verifying the the physical layout doesn't change (so somewhat related, but not directly). Just mentioning it as possible alternative.

[alex-spies]

Also, one check which would be great (since it caused at least two bugs), but not done atm. is verifying the the physical layout doesn't change (so somewhat related, but not directly). Just mentioning it as possible alternative.

That's actually a very good idea for a follow-up. Pre- and post optimization, the local plan must have the same output; simplest would be to try and use the QueryPlan#output method for this check.

[bpintea]

simplest would be to try and use the QueryPlan#output method for this check.

FWIW, what #129370 fixed wouldn't have been caught by verifying the outputs pre- and post-optimizations, since agg's outputs contain both aggs and groups (aliased or not). Only the layout changed because of a group being dropped. So we need a check of the layouts on both ends (coordinator and data node), which is tricker.

[julian-elastic]

I created the follow up PR for the output checker here #130757