Skip to content

Commit f33f0be

Browse files
zachmuzachmu
authored
Merge pull request #2123 from dolthub/zachmu/index-logic
Bug fix: ensure OR expressions on a single table use an index when possible
2 parents c33a8da + 7f1444c commit f33f0be

5 files changed

Lines changed: 84 additions & 59 deletions

File tree

‎go.mod‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,10 @@ require (
66
github.com/PuerkitoBio/goquery v1.8.1
77
github.com/cockroachdb/apd/v2 v2.0.3-0.20200518165714-d020e156310a
88
github.com/cockroachdb/errors v1.7.5
9-
github.com/dolthub/dolt/go v0.40.5-0.20251213003033-5aae073ad198
9+
github.com/dolthub/dolt/go v0.40.5-0.20251216020900-9c1c5ed2c8fd
1010
github.com/dolthub/eventsapi_schema v0.0.0-20250915094920-eadfd39051ca
1111
github.com/dolthub/flatbuffers/v23 v23.3.3-dh.2
12-
github.com/dolthub/go-mysql-server v0.20.1-0.20251215235453-b3613827cc44
12+
github.com/dolthub/go-mysql-server v0.20.1-0.20251216223848-500454bc6d5f
1313
github.com/dolthub/pg_query_go/v6 v6.0.0-20251215122834-fb20be4254d1
1414
github.com/dolthub/sqllogictest/go v0.0.0-20240618184124-ca47f9354216
1515
github.com/dolthub/vitess v0.0.0-20251210200925-1d33d416d162

‎go.sum‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -230,6 +230,8 @@ github.com/dolthub/dolt-mcp v0.2.2 h1:bpROmam74n95uU4EA3BpOIVlTDT0pzeFMBwe/YRq2m
230230
github.com/dolthub/dolt-mcp v0.2.2/go.mod h1:S++DJ4QWTAXq+0TNzFa7Oq3IhoT456DJHwAINFAHgDQ=
231231
github.com/dolthub/dolt/go v0.40.5-0.20251213003033-5aae073ad198 h1:2DdAh70y/xSC4Ej/TWyRWJsjvd4R3T7lF0Nb3pzJhpo=
232232
github.com/dolthub/dolt/go v0.40.5-0.20251213003033-5aae073ad198/go.mod h1:eBvkNDgUSm/z17brovrRzfL57dwlQXBoXfqeisMUQ9E=
233+
github.com/dolthub/dolt/go v0.40.5-0.20251216020900-9c1c5ed2c8fd h1:GcS8p05zGvduMKRB2O2KeTwPrWC3a5M3VX/tkR5fAYU=
234+
github.com/dolthub/dolt/go v0.40.5-0.20251216020900-9c1c5ed2c8fd/go.mod h1:9p1QMq5hTZAExGlwK0+VJqfsMnV+TPw6ANk0QaxTZh8=
233235
github.com/dolthub/eventsapi_schema v0.0.0-20250915094920-eadfd39051ca h1:BGFz/0OlKIuC6qHIZQbvPapFvdAJkeEyGXWVgL5clmE=
234236
github.com/dolthub/eventsapi_schema v0.0.0-20250915094920-eadfd39051ca/go.mod h1:CoDLfgPqHyBtth0Cp+fi/CmC4R81zJNX4wPjShdZ+Bw=
235237
github.com/dolthub/flatbuffers/v23 v23.3.3-dh.2 h1:u3PMzfF8RkKd3lB9pZ2bfn0qEG+1Gms9599cr0REMww=
@@ -238,12 +240,8 @@ github.com/dolthub/fslock v0.0.3 h1:iLMpUIvJKMKm92+N1fmHVdxJP5NdyDK5bK7z7Ba2s2U=
238240
github.com/dolthub/fslock v0.0.3/go.mod h1:QWql+P17oAAMLnL4HGB5tiovtDuAjdDTPbuqx7bYfa0=
239241
github.com/dolthub/go-icu-regex v0.0.0-20250916051405-78a38d478790 h1:zxMsH7RLiG+dlZ/y0LgJHTV26XoiSJcuWq+em6t6VVc=
240242
github.com/dolthub/go-icu-regex v0.0.0-20250916051405-78a38d478790/go.mod h1:F3cnm+vMRK1HaU6+rNqQrOCyR03HHhR1GWG2gnPOqaE=
241-
github.com/dolthub/go-mysql-server v0.20.1-0.20251212235309-4422a1ca9f43 h1:3OiTNnL3rFiSXGo7/xTt64FmJGtGU1igvyPishbsM4o=
242-
github.com/dolthub/go-mysql-server v0.20.1-0.20251212235309-4422a1ca9f43/go.mod h1:NjewWKoa5bVSLdKwL7fg7eAfrcIxDybWUKoWEHWRTw4=
243-
github.com/dolthub/go-mysql-server v0.20.1-0.20251215224112-7cb4535802a5 h1:gjUTlYUVTWnKoZBizqLQzWL/rxwT4ZC8IGt49pzpCQg=
244-
github.com/dolthub/go-mysql-server v0.20.1-0.20251215224112-7cb4535802a5/go.mod h1:NjewWKoa5bVSLdKwL7fg7eAfrcIxDybWUKoWEHWRTw4=
245-
github.com/dolthub/go-mysql-server v0.20.1-0.20251215235453-b3613827cc44 h1:/e1XKLp5D200IwFq7uKAFgza30NuNTmbpD7VgcrAqhI=
246-
github.com/dolthub/go-mysql-server v0.20.1-0.20251215235453-b3613827cc44/go.mod h1:NjewWKoa5bVSLdKwL7fg7eAfrcIxDybWUKoWEHWRTw4=
243+
github.com/dolthub/go-mysql-server v0.20.1-0.20251216223848-500454bc6d5f h1:trWK6M/ouvxhmA+V6bsXUGIFLinUC+oAG7gnoa73+F0=
244+
github.com/dolthub/go-mysql-server v0.20.1-0.20251216223848-500454bc6d5f/go.mod h1:NjewWKoa5bVSLdKwL7fg7eAfrcIxDybWUKoWEHWRTw4=
247245
github.com/dolthub/gozstd v0.0.0-20240423170813-23a2903bca63 h1:OAsXLAPL4du6tfbBgK0xXHZkOlos63RdKYS3Sgw/dfI=
248246
github.com/dolthub/gozstd v0.0.0-20240423170813-23a2903bca63/go.mod h1:lV7lUeuDhH5thVGDCKXbatwKy2KW80L4rMT46n+Y2/Q=
249247
github.com/dolthub/ishell v0.0.0-20240701202509-2b217167d718 h1:lT7hE5k+0nkBdj/1UOSFwjWpNxf+LCApbRHgnCA17XE=

‎server/analyzer/split.go‎

Lines changed: 13 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -22,34 +22,6 @@ import (
2222
pgtypes "github.com/dolthub/doltgresql/server/types"
2323
)
2424

25-
// SplitDisjunction breaks OR expressions into their left and right parts, recursively. Also handles expressions that
26-
// can be approximated as OR expressions, such as IN for tuples.
27-
func SplitDisjunction(expr sql.Expression) []sql.Expression {
28-
if expr == nil {
29-
return nil
30-
}
31-
switch expr := expr.(type) {
32-
case *expression.Or:
33-
return append(
34-
SplitDisjunction(expr.LeftChild),
35-
SplitDisjunction(expr.RightChild)...,
36-
)
37-
case *pgexprs.GMSCast:
38-
// We should check to see if we need to preserve the cast on each child individually
39-
split := SplitDisjunction(expr.Child())
40-
for i := range split {
41-
if _, ok := split[i].Type().(*pgtypes.DoltgresType); !ok {
42-
split[i] = pgexprs.NewGMSCast(split[i])
43-
}
44-
}
45-
return split
46-
case *pgexprs.InTuple:
47-
return SplitDisjunction(expr.Decay())
48-
default:
49-
return []sql.Expression{expr}
50-
}
51-
}
52-
5325
// SplitConjunction breaks AND expressions into their left and right parts, recursively.
5426
func SplitConjunction(expr sql.Expression) []sql.Expression {
5527
if expr == nil {
@@ -75,28 +47,19 @@ func SplitConjunction(expr sql.Expression) []sql.Expression {
7547
}
7648
}
7749

78-
// SplitDisjunctions performs the same operation as SplitDisjunction, except that it applies to a slice.
79-
func SplitDisjunctions(exprs []sql.Expression) []sql.Expression {
80-
if len(exprs) == 0 {
81-
return nil
82-
}
83-
// New slice will be at least the size of the incoming slice
84-
newExprs := make([]sql.Expression, 0, len(exprs))
85-
for _, expr := range exprs {
86-
newExprs = append(newExprs, SplitDisjunction(expr)...)
87-
}
88-
return newExprs
89-
}
50+
// LogicTreeWalker is a walker that removes GMSCast and other Doltgres specific expression nodes from
51+
// logic expression trees. This allows the analyzer logic to correctly reason about expressions in filters
52+
// to apply indexes.
53+
type LogicTreeWalker struct{}
9054

91-
// SplitConjunctions performs the same operation as SplitConjunction, except that it applies to a slice.
92-
func SplitConjunctions(exprs []sql.Expression) []sql.Expression {
93-
if len(exprs) == 0 {
94-
return nil
95-
}
96-
// New slice will be at least the size of the incoming slice
97-
newExprs := make([]sql.Expression, 0, len(exprs))
98-
for _, expr := range exprs {
99-
newExprs = append(newExprs, SplitConjunction(expr)...)
55+
var _ sql.ExpressionTreeFilter = &LogicTreeWalker{}
56+
57+
// Next implements the sql.ExpressionTreeFilter interface.
58+
func (l *LogicTreeWalker) Next(e sql.Expression) sql.Expression {
59+
switch expr := e.(type) {
60+
case *pgexprs.GMSCast:
61+
return l.Next(expr.Child())
62+
default:
63+
return e
10064
}
101-
return newExprs
10265
}

‎servercfg/config.go‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,8 @@ import (
2121
"github.com/dolthub/go-mysql-server/sql"
2222
"gopkg.in/yaml.v2"
2323

24+
"github.com/dolthub/doltgresql/server/analyzer"
25+
2426
pgsql "github.com/dolthub/doltgresql/postgres/parser/parser/sql"
2527
"github.com/dolthub/doltgresql/server/expression"
2628
"github.com/dolthub/doltgresql/servercfg/cfgdetails"
@@ -43,7 +45,8 @@ func (*DoltgresConfig) Overrides() sql.EngineOverrides {
4345
ParseTableAsColumn: expression.NewTableToComposite,
4446
Parser: pgsql.NewPostgresParser(),
4547
},
46-
SchemaFormatter: pgsql.NewPostgresSchemaFormatter(),
48+
SchemaFormatter: pgsql.NewPostgresSchemaFormatter(),
49+
CostedIndexScanExpressionFilter: &analyzer.LogicTreeWalker{},
4750
}
4851
}
4952

‎testing/go/index_test.go‎

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,39 @@ func TestBasicIndexing(t *testing.T) {
6666
{" └─ columns: [pk v1]"},
6767
},
6868
},
69+
{
70+
Query: "SELECT * FROM test WHERE (v1 > 3 OR v1 < 2) AND v1 <> 5 ORDER BY pk;",
71+
Expected: []sql.Row{
72+
{11, 1},
73+
{14, 4}},
74+
},
75+
{
76+
Query: "explain SELECT * FROM test WHERE (v1 > 3 OR v1 < 2) AND v1 <> 5 ORDER BY pk;",
77+
Expected: []sql.Row{
78+
{"Sort(test.pk ASC)"},
79+
{" └─ IndexedTableAccess(test)"},
80+
{" ├─ index: [test.v1]"},
81+
{" ├─ filters: [{(NULL, 2)}, {(3, 5)}, {(5, ∞)}]"},
82+
{" └─ columns: [pk v1]"},
83+
},
84+
},
85+
{
86+
Query: "SELECT * FROM test WHERE v1 = 2 OR v1 = 4 ORDER BY pk;",
87+
Expected: []sql.Row{
88+
{12, 2},
89+
{14, 4},
90+
},
91+
},
92+
{
93+
Query: "explain SELECT * FROM test WHERE v1 = 2 OR v1 = 4 ORDER BY pk;",
94+
Expected: []sql.Row{
95+
{"Sort(test.pk ASC)"},
96+
{" └─ IndexedTableAccess(test)"},
97+
{" ├─ index: [test.v1]"},
98+
{" ├─ filters: [{[2, 2]}, {[4, 4]}]"},
99+
{" └─ columns: [pk v1]"},
100+
},
101+
},
69102
{
70103
Query: "SELECT * FROM test WHERE v1 IN (2, 4) ORDER BY pk;",
71104
Expected: []sql.Row{
@@ -151,6 +184,24 @@ func TestBasicIndexing(t *testing.T) {
151184
{12, "twelve"},
152185
},
153186
},
187+
{
188+
Query: "SELECT * FROM test WHERE v1 > 't' OR v1 < 'f' ORDER BY pk;",
189+
Expected: []sql.Row{
190+
{11, "eleven"},
191+
{12, "twelve"},
192+
{13, "thirteen"},
193+
},
194+
},
195+
{
196+
Query: "explain SELECT * FROM test WHERE v1 > 't' OR v1 < 'f' ORDER BY pk;",
197+
Expected: []sql.Row{
198+
{"Sort(test.pk ASC)"},
199+
{" └─ IndexedTableAccess(test)"},
200+
{" ├─ index: [test.pk,test.v1]"},
201+
{" ├─ filters: [{[NULL, ∞), (NULL, f)}, {[NULL, ∞), (t, ∞)}]"},
202+
{" └─ columns: [pk v1]"},
203+
},
204+
},
154205
{
155206
Query: "DELETE FROM test WHERE v1 = 'twelve'",
156207
SkipResultsCheck: true,
@@ -1261,6 +1312,16 @@ func TestBasicIndexing(t *testing.T) {
12611312
{5, 9},
12621313
},
12631314
},
1315+
{
1316+
Query: "explain SELECT * FROM test WHERE v1 BETWEEN 3 AND 5 OR v1 BETWEEN 7 AND 9 order by 1;",
1317+
Expected: []sql.Row{
1318+
{"Sort(test.pk ASC)"},
1319+
{" └─ IndexedTableAccess(test)"},
1320+
{" ├─ index: [test.v1]"},
1321+
{" ├─ filters: [{[3, 5]}, {[7, 9]}]"},
1322+
{" └─ columns: [pk v1]"},
1323+
},
1324+
},
12641325
},
12651326
},
12661327
{

0 commit comments

Comments
 (0)