From ed44bbdb432ae11058295b77adb7ab20c3eaa281 Mon Sep 17 00:00:00 2001 From: elianddb Date: Wed, 22 Oct 2025 10:06:13 -0700 Subject: [PATCH 1/3] fix nil deref on Dispose() in --- enginetest/memory_engine_test.go | 28 +++++++++++++++++----------- sql/rowexec/agg.go | 8 ++++++-- 2 files changed, 23 insertions(+), 13 deletions(-) diff --git a/enginetest/memory_engine_test.go b/enginetest/memory_engine_test.go index f1ec7b45d0..d9191960c5 100644 --- a/enginetest/memory_engine_test.go +++ b/enginetest/memory_engine_test.go @@ -200,23 +200,29 @@ func TestSingleQueryPrepared(t *testing.T) { // Convenience test for debugging a single query. Unskip and set to the desired query. func TestSingleScript(t *testing.T) { - t.Skip() + //t.Skip() var scripts = []queries.ScriptTest{ { - Name: "AS OF propagates to nested CALLs", - SetUpScript: []string{}, + // https://github.com/dolthub/dolt/issues/9987 + Name: "GROUP BY nil pointer dereference in Dispose when Next() never called", + SetUpScript: []string{ + "CREATE TABLE test_table (id INT PRIMARY KEY, value INT, category VARCHAR(50))", + "INSERT INTO test_table VALUES (1, 100, 'A'), (2, 200, 'B'), (3, 300, 'A')", + }, Assertions: []queries.ScriptTestAssertion{ { - Query: "create procedure create_proc() create table t (i int primary key, j int);", - Expected: []sql.Row{ - {types.NewOkResult(0)}, - }, + // LIMIT 0 causes the iterator to close without ever calling Next() on groupByIter + // This leaves all buffer elements as nil, causing panic in Dispose() + Query: "SELECT category, SUM(value) FROM test_table GROUP BY category LIMIT 0", + Expected: []sql.Row{}, }, { - Query: "call create_proc()", - Expected: []sql.Row{ - {types.NewOkResult(0)}, - }, + Query: "SELECT category, COUNT(*) FROM test_table GROUP BY category LIMIT 0", + Expected: []sql.Row{}, + }, + { + Query: "SELECT SUM(value) FROM test_table LIMIT 0", + Expected: []sql.Row{}, }, }, }, diff --git a/sql/rowexec/agg.go b/sql/rowexec/agg.go index e529432caf..c2d48c438f 100644 --- a/sql/rowexec/agg.go +++ b/sql/rowexec/agg.go @@ -103,7 +103,9 @@ func (i *groupByIter) Close(ctx *sql.Context) error { func (i *groupByIter) Dispose() { for _, b := range i.buf { - b.Dispose() + if b != nil { + b.Dispose() + } } } @@ -234,7 +236,9 @@ func (i *groupByGroupingIter) Dispose() { bs, _ := i.get(k) if bs != nil { for _, b := range bs { - b.Dispose() + if b != nil { + b.Dispose() + } } } } From 327b11dddcf8b90972e53ce673042a095e4ee384 Mon Sep 17 00:00:00 2001 From: elianddb Date: Wed, 22 Oct 2025 10:09:01 -0700 Subject: [PATCH 2/3] mv tests --- enginetest/memory_engine_test.go | 28 +++++++++++----------------- enginetest/queries/script_queries.go | 24 ++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 17 deletions(-) diff --git a/enginetest/memory_engine_test.go b/enginetest/memory_engine_test.go index d9191960c5..f1ec7b45d0 100644 --- a/enginetest/memory_engine_test.go +++ b/enginetest/memory_engine_test.go @@ -200,29 +200,23 @@ func TestSingleQueryPrepared(t *testing.T) { // Convenience test for debugging a single query. Unskip and set to the desired query. func TestSingleScript(t *testing.T) { - //t.Skip() + t.Skip() var scripts = []queries.ScriptTest{ { - // https://github.com/dolthub/dolt/issues/9987 - Name: "GROUP BY nil pointer dereference in Dispose when Next() never called", - SetUpScript: []string{ - "CREATE TABLE test_table (id INT PRIMARY KEY, value INT, category VARCHAR(50))", - "INSERT INTO test_table VALUES (1, 100, 'A'), (2, 200, 'B'), (3, 300, 'A')", - }, + Name: "AS OF propagates to nested CALLs", + SetUpScript: []string{}, Assertions: []queries.ScriptTestAssertion{ { - // LIMIT 0 causes the iterator to close without ever calling Next() on groupByIter - // This leaves all buffer elements as nil, causing panic in Dispose() - Query: "SELECT category, SUM(value) FROM test_table GROUP BY category LIMIT 0", - Expected: []sql.Row{}, - }, - { - Query: "SELECT category, COUNT(*) FROM test_table GROUP BY category LIMIT 0", - Expected: []sql.Row{}, + Query: "create procedure create_proc() create table t (i int primary key, j int);", + Expected: []sql.Row{ + {types.NewOkResult(0)}, + }, }, { - Query: "SELECT SUM(value) FROM test_table LIMIT 0", - Expected: []sql.Row{}, + Query: "call create_proc()", + Expected: []sql.Row{ + {types.NewOkResult(0)}, + }, }, }, }, diff --git a/enginetest/queries/script_queries.go b/enginetest/queries/script_queries.go index c409e3ac7c..82e56390e6 100644 --- a/enginetest/queries/script_queries.go +++ b/enginetest/queries/script_queries.go @@ -122,6 +122,30 @@ type ScriptTestAssertion struct { // Unlike other engine tests, ScriptTests must be self-contained. No other tables are created outside the definition of // the tests. var ScriptTests = []ScriptTest{ + { + // https://github.com/dolthub/dolt/issues/9987 + Name: "GROUP BY nil pointer dereference in Dispose when Next() never called", + SetUpScript: []string{ + "CREATE TABLE test_table (id INT PRIMARY KEY, value INT, category VARCHAR(50))", + "INSERT INTO test_table VALUES (1, 100, 'A'), (2, 200, 'B'), (3, 300, 'A')", + }, + Assertions: []ScriptTestAssertion{ + { + // LIMIT 0 causes the iterator to close without ever calling Next() on groupByIter + // This leaves all buffer elements as nil, causing panic in Dispose() + Query: "SELECT category, SUM(value) FROM test_table GROUP BY category LIMIT 0", + Expected: []sql.Row{}, + }, + { + Query: "SELECT category, COUNT(*) FROM test_table GROUP BY category LIMIT 0", + Expected: []sql.Row{}, + }, + { + Query: "SELECT SUM(value) FROM test_table LIMIT 0", + Expected: []sql.Row{}, + }, + }, + }, { // https://github.com/dolthub/dolt/issues/9935 Dialect: "mysql", From e1e23e40395c70d14f41de0373e628a065ff19d5 Mon Sep 17 00:00:00 2001 From: elianddb Date: Wed, 22 Oct 2025 10:44:20 -0700 Subject: [PATCH 3/3] rm nil check in since it uses a map --- enginetest/queries/script_queries.go | 2 +- sql/rowexec/agg.go | 4 +--- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/enginetest/queries/script_queries.go b/enginetest/queries/script_queries.go index 82e56390e6..77c524da65 100644 --- a/enginetest/queries/script_queries.go +++ b/enginetest/queries/script_queries.go @@ -132,7 +132,7 @@ var ScriptTests = []ScriptTest{ Assertions: []ScriptTestAssertion{ { // LIMIT 0 causes the iterator to close without ever calling Next() on groupByIter - // This leaves all buffer elements as nil, causing panic in Dispose() + // This leaves all buffer elements as nil causing panic in Dispose(), or empty depending data struct Query: "SELECT category, SUM(value) FROM test_table GROUP BY category LIMIT 0", Expected: []sql.Row{}, }, diff --git a/sql/rowexec/agg.go b/sql/rowexec/agg.go index c2d48c438f..0f72120641 100644 --- a/sql/rowexec/agg.go +++ b/sql/rowexec/agg.go @@ -236,9 +236,7 @@ func (i *groupByGroupingIter) Dispose() { bs, _ := i.get(k) if bs != nil { for _, b := range bs { - if b != nil { - b.Dispose() - } + b.Dispose() } } }