-
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
colexec: plan disk-spilling enabled operators when vectorize=auto #45582
Conversation
Note that no tests are yet run with |
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.
Reviewed 3 of 3 files at r1, 16 of 16 files at r2, 12 of 12 files at r3.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @asubiotto)
pkg/sql/colflow/vectorized_flow.go, line 948 at r1 (raw file):
// buffer an unlimited number of tuples, even though it falls back to // disk. vectorize=auto does support this. return nil, errors.Errorf("hash router encountered when vectorize=auto")
nit: s/auto/192auto/g
.
The HashRouter can spill to disk, so it may now be planned when vectorize=auto. Release note: None (this behavior change will be called out in a following commit)
The sort operator can spill to disk, so it may now be planned when vectorize=auto. Release note (sql change): sorts are run using the vectorized engine when vectorize=auto (default configuration)
The hash joiner operator can spill to disk, so it may now be planned when vectorize=auto. Release note (sql change): hash joins are run using the vectorized engine when vectorize = auto (default configuration)
bors r=yuzefovich |
Build succeeded |
Can this be responsible for this failure?
https://teamcity.cockroachdb.com/viewLog.html?buildId=1779463&buildTypeId=Cockroach_UnitTests |
Each commit turns on one of the HashRouter, Sorter, and HashJoiner and includes the relevant test changes (all plan changes).
The only thing of note is that explain analyze plans include both row and column stats for wrapped operators. It seems like this is expected though (according to
vectorize_local
) and changing this naively results in other failures (unexpectedly not collecting stats
), so will leave this up to discussion of whether we want to change this. Regardless, I think it is out of scope for this PR.Closes #45172