Window exec uses framing iterator and support agg functions in windows #746
Conversation
There was a problem hiding this comment.
Overall very good work, definitely an improvement in organization and functionality.
My one overall comment is that this needs a some package-level docs explaining what the various abstractions are supposed to do and how they fit together. This doesn't need to be long, just a couple sentences for each type and a paragraph or two on the big picture. But I still have trouble keeping what each of these abstractions do straight in my head as I read this code.
| } | ||
|
|
||
| func (a *unaryAggBase) Resolved() bool { | ||
| if _, ok := a.Child.(*expression.Star); ok { |
There was a problem hiding this comment.
What's with this? Star is by definition not resolved (should get expanded by some other analyzer rule)
If you want to do it this way, create another Star expression, like AggregateStar to differentiate
There was a problem hiding this comment.
this is copied from the old count code, if we have a resolver it doesn't catch these. should i make an issue?
| @@ -83,6 +85,29 @@ func (w *Window) String() string { | |||
| return sb.String() | |||
| } | |||
|
|
|||
| func (w *Window) PartitionId() (uint64, error) { | |||
There was a problem hiding this comment.
Relatively expensive. If this gets called a lot, should probably cache the result.
There was a problem hiding this comment.
how? if i'm keeping a cache of strings to int64 shouldn't i just use the strings as the ID?
There was a problem hiding this comment.
i see what you mean now, fixed
max-hoffman
force-pushed
the
max/window-refactor
branch
from
ae799fd
to
703afa0
Compare
max-hoffman
changed the title
plan.Window is converted into new WindowIter backed by the framing setup. Codegen unary agg function nodes, including executable and unit testing harness. Rewrite window aggregation functions for framing. More unit tests that will make it easier to debug WindowBlock/Iter Doc comments for the new WindowIter exec stack.
max-hoffman
force-pushed
the
max/window-refactor
branch
from
703afa0
to
f90f638
Compare
plan.Window is converted into new WindowIter backed by the framing setup.
Codegen unary agg function nodes, including executable and unit testing
harness. Rewrite window aggregation functions for framing.
More unit tests that will make it easier to debug WindowBlock/Iter
Doc comments for the new WindowIter exec stack.