-
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
physicalplan: add support for multi-stage execution of regr_avgx, regr_avgy, regr_intercept, regr_r2, and regr_slope aggregate functions. #76007
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.
@yuzefovich can you help me out with the changes in zone
?
When I run testlogic
with rewrite
flag it modifies the zone
test file, and the ccl
tests are failing with the changes. I can adjust zone
test expectations to make tests pass but the next rewrite will override the file.
There are no changes on master branch when I run testlogic
with rewrite
.
Reviewable status: complete! 0 of 0 LGTMs obtained
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.
Hm, when I rewrite with your commit reverted (on 7c4e234), I do see changes in zone
file too. Clearly, your change is not to blame. My guess is that we have an issue with -rewrite
flag when retry
option is specified for the queries, so I'd just revert the changes in zone
file in your PR.
Reviewable status: complete! 0 of 0 LGTMs obtained
…r_avgy, regr_intercept, regr_r2, and regr_slope aggregate functions. See cockroachdb#58347. Release note (performance improvement): regr_avgx, regr_avgy, regr_intercept, regr_r2, and regr_slope aggregate functions are now evaluated more efficiently in a distributed setting
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.
thanks, reverted the changes in zone
file.
Reviewable status: complete! 0 of 0 LGTMs obtained
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 8 of 9 files at r2, 1 of 1 files at r3, all commit messages.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @mneverov)
Build succeeded: |
physicalplan: add support for multi-stage execution of regr_avgx, regr_avgy,
regr_intercept, regr_r2, and regr_slope aggregate functions.
See #58347.
Release note (performance improvement): regr_avgx, regr_avgy, regr_intercept,
regr_r2, and regr_slope aggregate functions are now evaluated more efficiently
in a distributed setting