[query, bugfix] TableAggregate interpret supports globals in init op

[patrick-schultz]

There was a typo in the Interpret rule for TableAggregate which had it refer to the row instead of the globals inside the init op.

I tried to add a test for this, but it's frustratingly difficult to force the compiler to go through this code path. Even when using an InterpretOnly compilation, the lowering pipeline often lifts TableAggregate to a RelationalLet, and then evaluates it, using the compiler not the interpreter. This is a deeper issue we should address, but a user is currently blocked on this bug so I don't want to hold it up.

[patrick-schultz]

• [query, bugfix] TableAggregate interpret supports globals in init op #14673 👈
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @patrick-schultz and the rest of your teammates on Graphite

[chrisvittal]

Glad the fix was simple

approved by mistake

[ehigham]

Can you lock down this behaviour with a test?

[patrick-schultz]

Can you lock down this behaviour with a test?

As I said in the description, I tried hard to write a test, but I couldn't manage to find a way to exercise this with a targeted test. And this bug is currently blocking a user, so I want to get it released asap. I can try again, but I suspect it would end up being a day or two of extra work, and would likely still end up brittle and unsatisfactory. I'd rather spend my effort getting back into the line of work that would eventually make it much easier to target specific compiler code paths.

[ehigham]

sorry for not reading description!