Skip to content
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: rename vectorize setting 192auto to auto and make it the default #46845

Merged
merged 2 commits into from
Apr 2, 2020

Conversation

asubiotto
Copy link
Contributor

@asubiotto asubiotto commented Apr 1, 2020

This PR will be backported to 20.1. A followup PR will make "on" the default for master. Check the commits for more details.

This PR also disables processor wrapping for all processors when vectorize=auto apart from the JoinReader, since this is in line with 19.2 behavior.

Release note: None

@asubiotto asubiotto requested review from jordanlewis, yuzefovich and a team April 1, 2020 12:03
@asubiotto asubiotto requested a review from a team as a code owner April 1, 2020 12:03
@cockroach-teamcity
Copy link
Member

This change is Reviewable

…ault

This commit renames 192auto back to auto and makes it the default, so that
only streaming vectorized operators will be planned by default. The previous
auto setting would also plan operators that can spill to disk. This is now
removed and they can be run using the "on" setting.

Release note (sql change): the vectorized execution engine will only run
queries with streaming operators. SET vectorize=on to enable the vectorized
execution engine for buffering operators.
This commit will only allow JoinReaders to be part of a colexec flow when
vectorize=auto.

Release note: None (no observable user change)
Copy link
Member

@yuzefovich yuzefovich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

Reviewed 30 of 30 files at r1, 20 of 20 files at r2.
Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @asubiotto and @jordanlewis)


pkg/sql/sessiondata/session_data.go, line 270 at r1 (raw file):

	// streaming operators (i.e. those that do not require any buffering) will
	// be run using the columnar execution. If any part of a query is not
	// supported by the vectorized execution engine, the whole query will fall

nit: this comment is slightly misleading since we're wrapping index and lookup joins, but I'm not sure whether it's worth calling out. Probably not. Never mind.

@asubiotto
Copy link
Contributor Author

bors r=yuzefovich

@craig
Copy link
Contributor

craig bot commented Apr 2, 2020

Build failed

@asubiotto
Copy link
Contributor Author

The failure seems like a hard to hit float precision issue that we've run into before: https://github.com/cockroachdb/cockroach/pull/37600/files. We should probably take care of that in a separate PR.

@asubiotto
Copy link
Contributor Author

bors r-

@asubiotto
Copy link
Contributor Author

bors r=yuzefovich

@craig
Copy link
Contributor

craig bot commented Apr 2, 2020

Build succeeded

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants