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

[DNM] opt: prototype bypass planNode for Scan #30969

Closed
wants to merge 1 commit into from

Conversation

rytaft
Copy link
Collaborator

@rytaft rytaft commented Oct 4, 2018

This is a proof-of-concept for how we can bypass creation of
planNodes and create the distSQL plan directly from the execbuilder.
Bypassing creation of the planNode saves 3.6% in total execution time
for the query SELECT * FROM kv.

name                  old time/op  new time/op  delta
Exec/kv-scan/ExecOpt  247µs ± 3%   238µs ± 1%   -3.64%

It would be ideal to avoid using the interface in the execbuilder
that was needed to create planNodes in the sql package, but creation
of distSQL nodes is also heavily intertwined with the sql package.
Moving the new opt_distsql_factory into opt should probably be a
long-term goal, but it would be quite difficult in the short term
without duplicating a lot of code.

Release note: None

This is a proof-of-concept for how we can bypass creation of
planNodes and create the distSQL plan directly from the execbuilder.
Bypassing creation of the planNode saves 3.6% in total execution time
for the query `SELECT * FROM kv`.

name                  old time/op  new time/op  delta
Exec/kv-scan/ExecOpt  247µs ± 3%   238µs ± 1%   -3.64%

It would be ideal to avoid using the interface in the execbuilder
that was needed to create planNodes in the sql package, but creation
of distSQL nodes is also heavily intertwined with the sql package.
Moving the new opt_distsql_factory into opt should probably be a
long-term goal, but it would be quite difficult in the short term
without duplicating a lot of code.

Release note: None
@rytaft rytaft requested review from justinj, RaduBerinde, andy-kimball and a team October 4, 2018 18:13
@rytaft rytaft requested a review from a team as a code owner October 4, 2018 18:13
@rytaft rytaft requested a review from a team October 4, 2018 18:13
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@tbg tbg added the X-noremind Bots won't notify about PRs with X-noremind label Jun 19, 2019
@rytaft
Copy link
Collaborator Author

rytaft commented Jun 14, 2024

Keeping this open is no longer useful.

@rytaft rytaft closed this Jun 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
X-noremind Bots won't notify about PRs with X-noremind
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants