Skip to content

various analyzer and planbuilder optimizations#3345

Merged
jycor merged 9 commits intomainfrom
james/ranges2
Dec 16, 2025
Merged

various analyzer and planbuilder optimizations#3345
jycor merged 9 commits intomainfrom
james/ranges2

Conversation

@jycor
Copy link
Copy Markdown
Contributor

@jycor jycor commented Dec 15, 2025

Changes:

  • rewrote convertInt to call strings library fewer times
  • use casting to detect rounding in index builder
  • separate code for InspectExpr()
  • simplify isEvaluable into single pass
  • replace more Sprintf with simple string concatenation
  • have iScanAnd count children as they are added

benchmarks: dolthub/dolt#10210 (comment)

Copy link
Copy Markdown
Member

@zachmu zachmu left a comment

Choose a reason for hiding this comment

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

LGTM

if i64, err := strconv.ParseInt(value, base, 64); err == nil {
return expression.NewLiteral(int64(i64), types.Int64)
func (b *Builder) convertInt(value []byte, base int) *expression.Literal {
valStr := encodings.BytesToString(value)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should write a comment about what the below logic is doing and that it's for perf reasons

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Plan to check this in?

if ok {
return nil, SameTree, stopInspect
func InspectExpr(expr sql.Expression, f func(sql.Expression) bool) bool {
children := expr.Children()
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Kinda can't believe we were doing this

@jycor jycor merged commit fc7d045 into main Dec 16, 2025
8 checks passed
@jycor jycor deleted the james/ranges2 branch December 16, 2025 00:43
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