Query: Refactor GroupBy aggregate to avoid dependency on Requires Materialization

[smitpatel]

No description provided.

[divega]

@smitpatel is there a more general issue for "delayed requires materialization" that we talked about in planning or is this the one?

[smitpatel]

@divega - We don't have tracking issue for that separately. Could be part of #7520

[divega]

@smitpatel ok, let's chat when we have a chance about how to best track it. If you add it to the list in #7520 that would be a good start.

[smitpatel]

@divega - #11215 I added note to that issue.

[ajcvickers]

Make sure to test case in #11813

[ajcvickers]

Moved this into 2.2 since we believe it will help stabilize query. If, when working on it. it looks like it is going to have the opposite effect. then we should reconsider.

[smitpatel]

We do not have requires materialization visitor anymore. So this is no longer needed. We can do needful when we re-implement group by aggregate.

[a-a-k]

@smitpatel what does it exactly mean? Could you explain in simple terms?

[smitpatel]

In old pipeline, we had to determine in advance if we need to materialize entity or we can server eval. For GroupBy when you are applying aggregate, entity materialization is not required. But with mixed mode evaluation, it was not possible to determine in all cases, if we can translate to SQL group by since some client function or not mapped property can cause client eval. Hence we were having issues when advance determination was incorrect. This issue was tracking it. Now in new pipeline we don't do advance determination of materialization so this issue is no longer relevant.