-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
builtins: implement regr_avgy #57583
Conversation
Thank you for contributing to CockroachDB. Please ensure you have followed the guidelines for creating a PR. My owl senses detect your PR is good for review. Please keep an eye out for any test failures in CI. 🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is otan. |
Thank you for updating your pull request. My owl senses detect your PR is good for review. Please keep an eye out for any test failures in CI. 🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is otan. |
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.
nit: you could adjust the PR description (or the commit message) from See #41274
to something like Fixes: #41274
so that when the PR merges, the issue is closed automatically since regr_avgy
is the last aggregate in that issue.
Thanks a lot for your work!
Reviewed 14 of 14 files at r1.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @mneverov)
pkg/sql/opt/operator.go, line 324 at r1 (raw file):
VarPopOp, CovarPopOp, CovarSampOp, RegressionAvgXOp, RegressionInterceptOp, RegressionR2Op, RegressionSlopeOp, RegressionSXXOp, RegressionSXYOp, RegressionSYYOp, RegressionCountOp, RegressionAvgYOp:
super nit: it'd be nice to have AvgY
and AvgX
right next to each other.
Fixes: cockroachdb#41274 Release note (sql change): implement regr_avgy aggregation function.
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.
Hi @yuzefovich , thanks for the amazing support and patience :)!
Reviewable status: complete! 0 of 0 LGTMs obtained (and 1 stale) (waiting on @mneverov and @yuzefovich)
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.
bors r=yuzefovich
Reviewed 3 of 3 files at r2.
Reviewable status: complete! 0 of 0 LGTMs obtained (and 1 stale)
Build succeeded: |
builtins: implement regr_avgy
Fixes: #41274
Release note (sql change): implement regr_avgy aggregation function.