Fix to #19295 - Query: optimize CASE blocks

[maumar]

• if one of the conditions in CaseWhen test evaluates to false we can remove it,
• if one of the conditions in CaseWhen test evaluates to true, we can remove all blocks (including else block) that appears after,
• if we only have one CaseWhen block in the end, and test evaluates to true, we can simply return the result,
• if only Else block is left, return it's result,
• if nothing is left, return NULL.

Resolves #19295

[roji]

Not very important but I wonder if this belongs in NullabilityBasedSqlProcessingExpressionVisitor - these optimizations don't sound like they're directly related to nullability. Is this where we currently do stuff like eliminate constant true/false that have been left over from previous optimizations?

Just in terms of factoring and to keep NullabilityBasedSqlProcessingExpressionVisitor smaller, it may be better to have a later cleanup phase that would do this kind of thing. The possible downside would be another visitation in RelationalParameterBasedQueryTranslationPostprocessor (after the command cache), but that seems OK to me...

[maumar]

@roji this is somewhat unfortunate/confusing naming - we now perform ALL* optimizations in this visitor. This stems from design meeting discussion we had on the topic. I initially named the class simply SqlExpressionOptimizingVisitor (or some such), but @smitpatel wanted class name to be connected to the nullability optimizations, so we came up with NullabilityBasedSqlProcessingExpressionVisitor. I'm open to discussing & changing the name of this visitor once Smit is back.

• there are some optimizations repeated in the SearchConditionConvertingExpressionVisitor that simplify the product of search condition rewrite, so that we don't need to have a separate optimizing visitor pass after search condition runs.

[roji]

@maumar OK, no problem - I don't really mind, this was just about having non-nullability-related optimizations in a visitor named NullabilityBasedSqlProcessingExpressionVisitor. I'd definitely leans towards renaming back to SqlExpressionOptimizingVisitor, but we can always merge and revisit after @smitpatel is back.

[smitpatel]

If the optimization is not related to nullability then it should not be put into that visitor. There should be visitor for performing optimization which are independent of nullability of nodes.

[roji]

Somebody seems to have hacked into @smitpatel's account while they are in vacation, but they do seem to know stuff about EF Core ;)

[maumar]

@smitpatel we explicitly decided to combine null semantics and general optimizer during the design meeting. Perhaps we can re-visit this once we have more optimizations, but will leave it as is for now.

[smitpatel]

@maumar - Design meeting decision was to keep them separate explicitly. Optimizations which does not require/depend will remain in separate visitor. I don't have any issue we leave this optimization inside here (which is bit of an abuse) and change it once we have more than one optimization but name of this visitor must reflect the dependency on nullability.