-
Notifications
You must be signed in to change notification settings - Fork 161
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: add more window functions #534
feat: add more window functions #534
Conversation
c9d1f27
to
f0721e7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mostly looks good but a couple of return types seem weird.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have a few requests. Also, I'm not sure we need to say things like "expression is evaluated against". We don't really use that terminology elsewhere. For example, we could phrase addition as:
Two different expressions are evaluated. The results of these expressions are added together.
But I think that is more confusing. In other words, I think:
Returns the first value in the window
is more clear than:
The
expression
is evaluated against the first row.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few more nitpicks
Looks like there's one failing check to address before we can proceed here. |
Thanks! Had a typo! |
@westonpace Anything else you'd like to see? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The return of first_value
and last_value
is only nullable if the input is nullable. I don't remember if we have syntax for that or not. But this is minor and we can always adjust later if needed.
72cf04b
Co-authored-by: Weston Pace <[email protected]>
Co-authored-by: Weston Pace <[email protected]>
PR to add more window functions: