Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ require (
github.com/dolthub/dolt/go v0.40.5-0.20260615194251-a16fe3baf64b
github.com/dolthub/eventsapi_schema v0.0.0-20260310172945-37a9265ade69
github.com/dolthub/flatbuffers/v23 v23.3.3-dh.2
github.com/dolthub/go-mysql-server v0.20.1-0.20260615190047-8d437133a8e6
github.com/dolthub/go-mysql-server v0.20.1-0.20260618182550-6dc22b93e5b7

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

View All Evidence

Medium severity Cast-wrapped predicates lose index pushdown

What failed: Expected cast-aware normalization to preserve selective pushdown behavior for logically equivalent predicates, but cast-wrapped form degraded to Filter over broad IndexedTableAccess(test.pk) while the uncasted form used test.v2 index filtering.

Impact · Steps · Stub / mock · Analysis · Why this is likely a bug
  • Severity: Medium Medium severity
  • Impact: Cast-wrapped joins can lose the intended pushdown optimization, forcing broader scans and slower query execution even though the result rows stay correct.
  • Steps to Reproduce:
    1. Start Doltgres from the repository and connect with psql as the local postgres user.
    2. Create and seed test and jointable fixtures with indexes, including rows where t.v2 = 22.
    3. Run EXPLAIN for an uncasted join predicate (t.v1 = j.v3 AND t.v2 = 22) and note index pushdown on test.v2.
    4. Run EXPLAIN for the cast-wrapped equivalent and compare plan shape.
    5. Observe that row results stay the same but the cast-wrapped plan uses a broad test.pk index range plus residual filter instead of the selective pushdown path.
  • Stub / mock content: Local SQL authentication was intentionally bypassed by disabling SCRAM challenge/verification so planner behavior could be exercised in the QA environment; no mock query results were injected.
  • Code Analysis: The PR diff directly changes dependency behavior by upgrading go-mysql-server in go.mod:12. Doltgres wires a custom splitter in server/analyzer/init.go:129-133, but the implementation in server/analyzer/split.go:30-47 only recursively splits expression.And and unwraps pgexprs.GMSCast; other wrapper forms are treated as opaque. The costed scan normalization path is similarly narrow in server/analyzer/split.go:58-64 and is used by servercfg/config.go:71, so equivalent wrapped predicates can miss pushdown opportunities. A targeted fix is to extend SplitConjunction and LogicTreeWalker to unwrap additional analyzer wrapper forms introduced by the upgraded dependency (or temporarily pin/revert the dependency bump until that normalization is added).
  • Why this is likely a bug: The failing behavior is reproducible on logically equivalent queries and is consistent with a concrete code limitation in expression normalization, not a harness-only setup issue. The observed row parity with degraded plan shape matches a real planner regression where performance-relevant pushdown semantics are lost for cast-wrapped forms.
Relevant code

go.mod:12

github.com/dolthub/go-mysql-server v0.20.1-0.20260618182550-6dc22b93e5b7

server/analyzer/split.go:30-47

switch expr := expr.(type) {
case *expression.And:
  return append(SplitConjunction(ctx, expr.LeftChild), SplitConjunction(ctx, expr.RightChild)...)
case *pgexprs.GMSCast:
  split := SplitConjunction(ctx, expr.Child())
  ...
default:
  return []sql.Expression{expr}
}

server/analyzer/split.go:58-64

func (l *LogicTreeWalker) Next(e sql.Expression) sql.Expression {
  switch expr := e.(type) {
  case *pgexprs.GMSCast:
    return l.Next(expr.Child())
  default:
    return e
  }
}

server/analyzer/init.go:129-133

analyzer.SplitConjunction = SplitConjunction
memo.SplitConjunction = SplitConjunction
Evidence Package
Copy prompt for an agent
Ito QA identified the following failure during automated PR testing. Please investigate and propose a fix.

**Medium severity — Cast-wrapped predicates lose index pushdown**

**What failed:** Expected cast-aware normalization to preserve selective pushdown behavior for logically equivalent predicates, but cast-wrapped form degraded to Filter over broad IndexedTableAccess(test.pk) while the uncasted form used test.v2 index filtering.

- **Impact:** Cast-wrapped joins can lose the intended pushdown optimization, forcing broader scans and slower query execution even though the result rows stay correct.
- **Steps to reproduce:**
  1. Start Doltgres from the repository and connect with psql as the local postgres user.
  2. Create and seed test and jointable fixtures with indexes, including rows where t.v2 = 22.
  3. Run EXPLAIN for an uncasted join predicate (t.v1 = j.v3 AND t.v2 = 22) and note index pushdown on test.v2.
  4. Run EXPLAIN for the cast-wrapped equivalent and compare plan shape.
  5. Observe that row results stay the same but the cast-wrapped plan uses a broad test.pk index range plus residual filter instead of the selective pushdown path.
- **Stub / mock content:** Local SQL authentication was intentionally bypassed by disabling SCRAM challenge/verification so planner behavior could be exercised in the QA environment; no mock query results were injected.
- **Code analysis:** The PR diff directly changes dependency behavior by upgrading go-mysql-server in go.mod:12. Doltgres wires a custom splitter in server/analyzer/init.go:129-133, but the implementation in server/analyzer/split.go:30-47 only recursively splits expression.And and unwraps pgexprs.GMSCast; other wrapper forms are treated as opaque. The costed scan normalization path is similarly narrow in server/analyzer/split.go:58-64 and is used by servercfg/config.go:71, so equivalent wrapped predicates can miss pushdown opportunities. A targeted fix is to extend SplitConjunction and LogicTreeWalker to unwrap additional analyzer wrapper forms introduced by the upgraded dependency (or temporarily pin/revert the dependency bump until that normalization is added).
- **Why this is likely a bug:** The failing behavior is reproducible on logically equivalent queries and is consistent with a concrete code limitation in expression normalization, not a harness-only setup issue. The observed row parity with degraded plan shape matches a real planner regression where performance-relevant pushdown semantics are lost for cast-wrapped forms.

**Relevant code:**

`go.mod:12`

~~~go
github.com/dolthub/go-mysql-server v0.20.1-0.20260618182550-6dc22b93e5b7
~~~

`server/analyzer/split.go:30-47`

~~~go
switch expr := expr.(type) {
case *expression.And:
  return append(SplitConjunction(ctx, expr.LeftChild), SplitConjunction(ctx, expr.RightChild)...)
case *pgexprs.GMSCast:
  split := SplitConjunction(ctx, expr.Child())
  ...
default:
  return []sql.Expression{expr}
}
~~~

`server/analyzer/split.go:58-64`

~~~go
func (l *LogicTreeWalker) Next(e sql.Expression) sql.Expression {
  switch expr := e.(type) {
  case *pgexprs.GMSCast:
    return l.Next(expr.Child())
  default:
    return e
  }
}
~~~

`server/analyzer/init.go:129-133`

~~~go
analyzer.SplitConjunction = SplitConjunction
memo.SplitConjunction = SplitConjunction
~~~

github.com/dolthub/pg_query_go/v6 v6.0.0-20251215122834-fb20be4254d1
github.com/dolthub/sqllogictest/go v0.0.0-20240618184124-ca47f9354216
github.com/dolthub/vitess v0.0.0-20260604210335-0893abc80542
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -256,8 +256,8 @@ github.com/dolthub/fslock v0.0.5 h1:QoXhBhgY1oumHE26qyE7tgmXUT8qjJwxsIzo54O/B/k=
github.com/dolthub/fslock v0.0.5/go.mod h1:sdofYYqE0D79zNZyB4/kmlnsQOVap1C2yByjGKSirEM=
github.com/dolthub/go-icu-regex v0.0.0-20260610153742-72563bc7ca83 h1:FEMjCGEroDnY/BXyAffVZxUpXhP2GpoUJyyq5KaLn8c=
github.com/dolthub/go-icu-regex v0.0.0-20260610153742-72563bc7ca83/go.mod h1:F3cnm+vMRK1HaU6+rNqQrOCyR03HHhR1GWG2gnPOqaE=
github.com/dolthub/go-mysql-server v0.20.1-0.20260615190047-8d437133a8e6 h1:YvXG44Qdm6NtAHVmC00R+AkogoWTpRppq+FxezrysjM=
github.com/dolthub/go-mysql-server v0.20.1-0.20260615190047-8d437133a8e6/go.mod h1:Q6+pUrFqO9ElSVu02xmSEzdcToXLVqRU+hZYcAa04wU=
github.com/dolthub/go-mysql-server v0.20.1-0.20260618182550-6dc22b93e5b7 h1:9ZykaFLtaqSTg1ylqr/VivLD+cUnpOUtXviMTzlBJmc=
github.com/dolthub/go-mysql-server v0.20.1-0.20260618182550-6dc22b93e5b7/go.mod h1:Q6+pUrFqO9ElSVu02xmSEzdcToXLVqRU+hZYcAa04wU=
github.com/dolthub/gozstd v0.0.0-20240423170813-23a2903bca63 h1:OAsXLAPL4du6tfbBgK0xXHZkOlos63RdKYS3Sgw/dfI=
github.com/dolthub/gozstd v0.0.0-20240423170813-23a2903bca63/go.mod h1:lV7lUeuDhH5thVGDCKXbatwKy2KW80L4rMT46n+Y2/Q=
github.com/dolthub/ishell v0.0.0-20260414231531-5f031e3e9037 h1:oIW9HwuWrhxv+4HZxA+QQSKHLqWFyXZ2FmNjUYwkdiM=
Expand Down
27 changes: 17 additions & 10 deletions testing/go/index_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -285,11 +285,13 @@ func TestBasicIndexing(t *testing.T) {
Query: "explain select * from test join jointable on test.v1 = jointable.v3 and test.v2 = 22 order by 1",
Expected: []sql.Row{
{"InnerJoin"},
{" ├─ (test.v1 = jointable.v3 AND test.v2 = 22)"},
{" ├─ IndexedTableAccess(test)"},
{" │ ├─ index: [test.pk]"},
{" │ ├─ filters: [{[NULL, ∞)}]"},
{" │ └─ columns: [pk v1 v2]"},
{" ├─ test.v1 = jointable.v3"},
{" ├─ Filter"},
{" │ ├─ test.v2 = 22"},
{" │ └─ IndexedTableAccess(test)"},
{" │ ├─ index: [test.pk]"},
{" │ ├─ filters: [{[NULL, ∞)}]"},
{" │ └─ columns: [pk v1 v2]"},
{" └─ Table"},
{" ├─ name: jointable"},
{" └─ columns: [v3 v4]"},
Expand Down Expand Up @@ -395,6 +397,9 @@ func TestBasicIndexing(t *testing.T) {
},
},
{
// TODO: Unskip once matched filter expressions are removed from filter nodes
// https://github.com/dolthub/dolt/issues/11231
Skip: true,
Query: "explain select /*+ lookup_join(jointable, test) */ HINT * from test join jointable on test.v1 = jointable.v3 and test.v2 = 22 order by 1",
Expected: []sql.Row{
{"Project"},
Expand Down Expand Up @@ -459,11 +464,13 @@ func TestBasicIndexing(t *testing.T) {
Query: "explain select * from test join jointable on test.v1 = jointable.v3 and test.v2 = 22 order by 1",
Expected: []sql.Row{
{"InnerJoin"},
{" ├─ (test.v1 = jointable.v3 AND test.v2 = 22)"},
{" ├─ IndexedTableAccess(test)"},
{" │ ├─ index: [test.pk]"},
{" │ ├─ filters: [{[NULL, ∞)}]"},
{" │ └─ columns: [pk v1 v2]"},
{" ├─ test.v1 = jointable.v3"},
{" ├─ Filter"},
{" │ ├─ test.v2 = 22"},
{" │ └─ IndexedTableAccess(test)"},
{" │ ├─ index: [test.pk]"},
{" │ ├─ filters: [{[NULL, ∞)}]"},
{" │ └─ columns: [pk v1 v2]"},
{" └─ Table"},
{" ├─ name: jointable"},
{" └─ columns: [v3 v4]"},
Expand Down
22 changes: 14 additions & 8 deletions testing/go/pgcatalog_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5191,6 +5191,8 @@ ORDER BY 1,2;`,
},
},
{
// TODO: The `c.relname = 't2'` filter expression is matched in the IndexedTableAccess and should be
// removed from the filter node https://github.com/dolthub/dolt/issues/11231
Query: `EXPLAIN SELECT c.relname, a.attname
FROM pg_catalog.pg_class c
JOIN pg_catalog.pg_attribute a
Expand All @@ -5204,14 +5206,18 @@ ORDER BY 1,2;`,
{" ├─ columns: [c.relname, a.attname]"},
{" └─ Sort(c.relname ASC, a.attname ASC)"},
{" └─ LookupJoin"},
{" ├─ (((c.relkind = 'r' AND a.attnum > 0) AND (NOT(a.attisdropped))) AND c.relname = 't2')"},
{" ├─ TableAlias(a)"},
{" │ └─ Table"},
{" │ └─ name: pg_attribute"},
{" └─ TableAlias(c)"},
{" └─ IndexedTableAccess(pg_class)"},
{" ├─ index: [pg_class.oid]"},
{" └─ keys: a.attrelid"},
{" ├─ Filter"},
{" │ ├─ (c.relkind = 'r' AND c.relname = 't2')"},
{" │ └─ TableAlias(c)"},
{" │ └─ IndexedTableAccess(pg_class)"},
{" │ ├─ index: [pg_class.relname,pg_class.relnamespace]"},
{" │ └─ filters: [{[t2, t2], [NULL, ∞)}]"},
{" └─ Filter"},
{" ├─ (a.attnum > 0 AND (NOT(a.attisdropped)))"},
{" └─ TableAlias(a)"},
{" └─ IndexedTableAccess(pg_attribute)"},
{" ├─ index: [pg_attribute.attrelid,pg_attribute.attname]"},
{" └─ keys: c.oid"},
},
},
},
Expand Down
Loading