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

Optimization: Defer Projections for Server Queries #2676

Merged
merged 48 commits into from
Oct 3, 2024
Merged

Optimization: Defer Projections for Server Queries #2676

merged 48 commits into from
Oct 3, 2024

Conversation

jycor
Copy link
Contributor

@jycor jycor commented Sep 23, 2024

This PR speeds up spooling queries from the server to client when there is a top level projection.
Changes include:

  • remove unnecessary double allocation of NewROw
  • defer projections until RowToSQL to avoid one extra allocation of sql.Row

Additionally, this PR cleans up and refactors some code surrounding projections.

@jycor jycor changed the base branch from james/proj to main September 25, 2024 23:03
@jycor jycor changed the title prototype Optimization: Defer Projections for Server Queries Sep 25, 2024
Copy link
Contributor

@max-hoffman max-hoffman left a comment

Choose a reason for hiding this comment

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

There are two orthogonal adjustments we can make to avoid extra work for small queries, basically mark the calling contexts when we want to defer projections, mark when we see a default expression that invalidates deferring, mark the top-level projection (not in a subquery/union) in planbuilder when the positive flags are set and no negative flags are set. I think this extra control logic should be similar complexity to switching on the max1RowIter case

// deferProjections marks projection nodes that can defer projections to the end of the query execution.
// This can avoid an unnecessary slice allocation specifically in the case where we are spooling rows
// from the server to the client.
func deferProjections(ctx *sql.Context, a *Analyzer, node sql.Node, scope *plan.Scope, sel RuleSelector, qFlags *sql.QueryFlags) (sql.Node, transform.TreeIdentity, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

we can inline this during binding/in planbuilder when we build a default expression

@@ -511,6 +512,24 @@ func resultForEmptyIter(ctx *sql.Context, iter sql.RowIter, resultFields []*quer
return &sqltypes.Result{Fields: resultFields}, nil
}

// GetDeferredProjections looks for a top-level deferred projection, marks it as deferred, and retrieves its projections
func GetDeferredProjections(iter sql.RowIter) []sql.Expression {
Copy link
Contributor

Choose a reason for hiding this comment

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

the alternative to this tagging queryFlags with a doProjectionDefer in the handler context and having planbuilder mark the defer for the top-level projection, which you identify with flags.IsSet(DoProjectionDefer) && notInSubquery() when building final projection

Copy link
Contributor

@max-hoffman max-hoffman left a comment

Choose a reason for hiding this comment

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

LGTM, just one more comment, perf wins look great

sql/planbuilder/scalar.go Outdated Show resolved Hide resolved
@jycor jycor merged commit f43dafa into main Oct 3, 2024
7 of 8 checks passed
@jycor jycor deleted the james/proj3 branch October 3, 2024 23:32
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.

2 participants