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

sql: reduce allocation in checkExprForDistSQL #121302

Closed
yuzefovich opened this issue Mar 28, 2024 · 0 comments · Fixed by #121903
Closed

sql: reduce allocation in checkExprForDistSQL #121302

yuzefovich opened this issue Mar 28, 2024 · 0 comments · Fixed by #121903
Assignees
Labels
C-cleanup Tech debt, refactors, loose ends, etc. Solution not expected to significantly change behavior. E-quick-win Likely to be a quick win for someone experienced. T-sql-queries SQL Queries Team

Comments

@yuzefovich
Copy link
Member

yuzefovich commented Mar 28, 2024

Here is a snippet of alloc_objects profile from 23.1.17:

ROUTINE ======================== github.com/cockroachdb/cockroach/pkg/sql.checkExpr in github.com/cockroachdb/cockroach/pkg/sql/distsql_physical_planner.go
1793616614 1793616614 (flat, cum)  1.24% of Total
         .          .    389:// supported by distSQL, like distSQL-blocklisted functions.
         .          .    390:func checkExpr(expr tree.Expr) error {
         .          .    391:	if expr == nil {
         .          .    392:		return nil
         .          .    393:	}
1793616614 1793616614    394:	v := distSQLExprCheckVisitor{}
         .          .    395:	tree.WalkExprConst(&v, expr)
         .          .    396:	return v.err
         .          .    397:}
         .          .    398:
         .          .    399:type distRecommendation int

I think we can reduce / eliminate allocations in checkExpr.

Jira issue: CRDB-37177

@yuzefovich yuzefovich added C-cleanup Tech debt, refactors, loose ends, etc. Solution not expected to significantly change behavior. T-sql-queries SQL Queries Team E-quick-win Likely to be a quick win for someone experienced. labels Mar 28, 2024
@yuzefovich yuzefovich self-assigned this Mar 28, 2024
@craig craig bot closed this as completed in 610688b Apr 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-cleanup Tech debt, refactors, loose ends, etc. Solution not expected to significantly change behavior. E-quick-win Likely to be a quick win for someone experienced. T-sql-queries SQL Queries Team
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

1 participant