From 35a2fb2c9ba5738c8ccff97164d5995fc0650055 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?coffeegoddd=E2=98=95=EF=B8=8F=E2=9C=A8?= Date: Wed, 8 Oct 2025 14:02:10 -0700 Subject: [PATCH 1/2] /sql/rowexec: attempt to fix panic --- sql/rowexec/builder.go | 32 ++++++++++++++++++++++++++++---- sql/rowexec/merge_join.go | 13 +++++++------ sql/rowexec/node_builder.gen.go | 14 ++++++++------ 3 files changed, 43 insertions(+), 16 deletions(-) diff --git a/sql/rowexec/builder.go b/sql/rowexec/builder.go index 9133b1c443..77451fbe0b 100644 --- a/sql/rowexec/builder.go +++ b/sql/rowexec/builder.go @@ -15,10 +15,11 @@ package rowexec import ( - "runtime/trace" + "fmt" + "runtime/trace" - "github.com/dolthub/go-mysql-server/sql" - "github.com/dolthub/go-mysql-server/sql/plan" + "github.com/dolthub/go-mysql-server/sql" + "github.com/dolthub/go-mysql-server/sql/plan" ) var DefaultBuilder = &BaseBuilder{} @@ -35,13 +36,36 @@ type BaseBuilder struct { override sql.NodeExecBuilder } +// panicSafeExecBuilder wraps another sql.NodeExecBuilder and converts panics to errors, +// preventing server-wide crashes when integrator-provided builders panic. +type panicSafeExecBuilder struct{ + inner sql.NodeExecBuilder +} + +func newPanicSafeExecBuilder(inner sql.NodeExecBuilder) sql.NodeExecBuilder { + if inner == nil { + return nil + } + return &panicSafeExecBuilder{inner: inner} +} + +func (p *panicSafeExecBuilder) Build(ctx *sql.Context, n sql.Node, r sql.Row) (iter sql.RowIter, err error) { + defer func() { + if rec := recover(); rec != nil { + err = fmt.Errorf("exec builder panic: %v", rec) + iter = nil + } + }() + return p.inner.Build(ctx, n, r) +} + func (b *BaseBuilder) Build(ctx *sql.Context, n sql.Node, r sql.Row) (sql.RowIter, error) { defer trace.StartRegion(ctx, "ExecBuilder.Build").End() return b.buildNodeExec(ctx, n, r) } func NewOverrideBuilder(override sql.NodeExecBuilder) sql.NodeExecBuilder { - return &BaseBuilder{override: override} + return &BaseBuilder{override: newPanicSafeExecBuilder(override)} } // FinalizeIters applies the final transformations on sql.RowIter before execution. diff --git a/sql/rowexec/merge_join.go b/sql/rowexec/merge_join.go index 1926697dda..40ac3b2f9c 100644 --- a/sql/rowexec/merge_join.go +++ b/sql/rowexec/merge_join.go @@ -46,11 +46,10 @@ func newMergeJoinIter(ctx *sql.Context, b sql.NodeExecBuilder, j *plan.JoinNode, return nil, err } - fullRow := make(sql.Row, len(row)+len(j.Left().Schema())+len(j.Right().Schema())) - fullRow[0] = row - if len(row) > 0 { - copy(fullRow[0:], row[:]) - } + fullRow := make(sql.Row, len(row)+len(j.Left().Schema())+len(j.Right().Schema())) + if len(row) > 0 { + copy(fullRow, row) + } // a merge join's first filter provides direction information // for which iter to update next @@ -79,7 +78,9 @@ func newMergeJoinIter(ctx *sql.Context, b sql.NodeExecBuilder, j *plan.JoinNode, typ: j.Op, fullRow: fullRow, scopeLen: j.ScopeLen, - parentLen: len(row) - j.ScopeLen, + // parentLen is the portion of the parent row beyond the current scope. + // Clamp to zero to avoid negative offsets when scopeLen > len(row). + parentLen: func() int { if len(row) >= j.ScopeLen { return len(row) - j.ScopeLen }; return 0 }(), leftRowLen: len(j.Left().Schema()), rightRowLen: len(j.Right().Schema()), isReversed: j.IsReversed, diff --git a/sql/rowexec/node_builder.gen.go b/sql/rowexec/node_builder.gen.go index a72dbdb0a0..f18e015ed7 100644 --- a/sql/rowexec/node_builder.gen.go +++ b/sql/rowexec/node_builder.gen.go @@ -26,12 +26,14 @@ import ( func (b *BaseBuilder) buildNodeExec(ctx *sql.Context, n sql.Node, row sql.Row) (sql.RowIter, error) { var iter sql.RowIter var err error - if b.override != nil { - iter, err = b.override.Build(ctx, n, row) - } - if err != nil { - return nil, err - } + if b.override != nil { + iter, err = b.override.Build(ctx, n, row) + if err != nil { + // If the override fails, fall back to the default builder + iter = nil + err = nil + } + } if iter == nil { iter, err = b.buildNodeExecNoAnalyze(ctx, n, row) } From d0c7bbff3363a2971d914f1a11604fcff2b43b2a Mon Sep 17 00:00:00 2001 From: coffeegoddd Date: Wed, 8 Oct 2025 21:04:40 +0000 Subject: [PATCH 2/2] [ga-format-pr] Run ./format_repo.sh to fix formatting --- sql/rowexec/builder.go | 36 ++++++++++++++++----------------- sql/rowexec/merge_join.go | 33 +++++++++++++++++------------- sql/rowexec/node_builder.gen.go | 16 +++++++-------- 3 files changed, 45 insertions(+), 40 deletions(-) diff --git a/sql/rowexec/builder.go b/sql/rowexec/builder.go index 77451fbe0b..4eacba2375 100644 --- a/sql/rowexec/builder.go +++ b/sql/rowexec/builder.go @@ -15,11 +15,11 @@ package rowexec import ( - "fmt" - "runtime/trace" + "fmt" + "runtime/trace" - "github.com/dolthub/go-mysql-server/sql" - "github.com/dolthub/go-mysql-server/sql/plan" + "github.com/dolthub/go-mysql-server/sql" + "github.com/dolthub/go-mysql-server/sql/plan" ) var DefaultBuilder = &BaseBuilder{} @@ -38,25 +38,25 @@ type BaseBuilder struct { // panicSafeExecBuilder wraps another sql.NodeExecBuilder and converts panics to errors, // preventing server-wide crashes when integrator-provided builders panic. -type panicSafeExecBuilder struct{ - inner sql.NodeExecBuilder +type panicSafeExecBuilder struct { + inner sql.NodeExecBuilder } func newPanicSafeExecBuilder(inner sql.NodeExecBuilder) sql.NodeExecBuilder { - if inner == nil { - return nil - } - return &panicSafeExecBuilder{inner: inner} + if inner == nil { + return nil + } + return &panicSafeExecBuilder{inner: inner} } func (p *panicSafeExecBuilder) Build(ctx *sql.Context, n sql.Node, r sql.Row) (iter sql.RowIter, err error) { - defer func() { - if rec := recover(); rec != nil { - err = fmt.Errorf("exec builder panic: %v", rec) - iter = nil - } - }() - return p.inner.Build(ctx, n, r) + defer func() { + if rec := recover(); rec != nil { + err = fmt.Errorf("exec builder panic: %v", rec) + iter = nil + } + }() + return p.inner.Build(ctx, n, r) } func (b *BaseBuilder) Build(ctx *sql.Context, n sql.Node, r sql.Row) (sql.RowIter, error) { @@ -65,7 +65,7 @@ func (b *BaseBuilder) Build(ctx *sql.Context, n sql.Node, r sql.Row) (sql.RowIte } func NewOverrideBuilder(override sql.NodeExecBuilder) sql.NodeExecBuilder { - return &BaseBuilder{override: newPanicSafeExecBuilder(override)} + return &BaseBuilder{override: newPanicSafeExecBuilder(override)} } // FinalizeIters applies the final transformations on sql.RowIter before execution. diff --git a/sql/rowexec/merge_join.go b/sql/rowexec/merge_join.go index 40ac3b2f9c..e8738ac296 100644 --- a/sql/rowexec/merge_join.go +++ b/sql/rowexec/merge_join.go @@ -46,10 +46,10 @@ func newMergeJoinIter(ctx *sql.Context, b sql.NodeExecBuilder, j *plan.JoinNode, return nil, err } - fullRow := make(sql.Row, len(row)+len(j.Left().Schema())+len(j.Right().Schema())) - if len(row) > 0 { - copy(fullRow, row) - } + fullRow := make(sql.Row, len(row)+len(j.Left().Schema())+len(j.Right().Schema())) + if len(row) > 0 { + copy(fullRow, row) + } // a merge join's first filter provides direction information // for which iter to update next @@ -71,16 +71,21 @@ func newMergeJoinIter(ctx *sql.Context, b sql.NodeExecBuilder, j *plan.JoinNode, } var iter sql.RowIter = &mergeJoinIter{ - left: l, - right: r, - filters: filters[1:], - cmp: cmp, - typ: j.Op, - fullRow: fullRow, - scopeLen: j.ScopeLen, - // parentLen is the portion of the parent row beyond the current scope. - // Clamp to zero to avoid negative offsets when scopeLen > len(row). - parentLen: func() int { if len(row) >= j.ScopeLen { return len(row) - j.ScopeLen }; return 0 }(), + left: l, + right: r, + filters: filters[1:], + cmp: cmp, + typ: j.Op, + fullRow: fullRow, + scopeLen: j.ScopeLen, + // parentLen is the portion of the parent row beyond the current scope. + // Clamp to zero to avoid negative offsets when scopeLen > len(row). + parentLen: func() int { + if len(row) >= j.ScopeLen { + return len(row) - j.ScopeLen + } + return 0 + }(), leftRowLen: len(j.Left().Schema()), rightRowLen: len(j.Right().Schema()), isReversed: j.IsReversed, diff --git a/sql/rowexec/node_builder.gen.go b/sql/rowexec/node_builder.gen.go index f18e015ed7..0f0955d40e 100644 --- a/sql/rowexec/node_builder.gen.go +++ b/sql/rowexec/node_builder.gen.go @@ -26,14 +26,14 @@ import ( func (b *BaseBuilder) buildNodeExec(ctx *sql.Context, n sql.Node, row sql.Row) (sql.RowIter, error) { var iter sql.RowIter var err error - if b.override != nil { - iter, err = b.override.Build(ctx, n, row) - if err != nil { - // If the override fails, fall back to the default builder - iter = nil - err = nil - } - } + if b.override != nil { + iter, err = b.override.Build(ctx, n, row) + if err != nil { + // If the override fails, fall back to the default builder + iter = nil + err = nil + } + } if iter == nil { iter, err = b.buildNodeExecNoAnalyze(ctx, n, row) }