Skip to content

Commit d4c70a0

Browse files
LeftHandColdmergify[bot]XuPeng-SH
authored
fix(plan): bind CTEs lazily in declaration scope (#26035)
## What type of PR is this? - [ ] API-change - [x] BUG - [ ] Improvement - [ ] Documentation - [ ] Feature - [ ] Test and CI - [ ] Code Refactoring ## Which issue(s) this PR fixes: issue #26015 issue #26022 ## What this PR does / why we need it: `preprocessCte` eagerly bound every CTE declaration, then the table-resolution path bound a referenced CTE again. This rejected errors inside unused CTEs and mutated a shared ROLLUP AST twice, expanding a one-column ROLLUP into three aggregate branches with a duplicated grand total. Removing the eager bind alone would let a CTE incorrectly capture tables introduced by the declaring query block's FROM clause. This change records a declaration-scoped binding context for each CTE: it excludes that query block's later/current FROM bindings while preserving legitimate outer correlation, visible CTE definitions, masking, recursive state, default database, snapshot, rewrite, and view lineage. CTE bodies are then bound lazily from that scope. View dependencies discovered through the detached scope are forwarded to the root query owner. This PR fixes the duplicate-grand-total root cause of #26022. Its independent synthetic-ROLLUP-key predicate-pushdown bug is intentionally handled in a separate PR. ## Test - Focused RED/GREEN `TestCTELazyBinding*` planner regressions - Existing CTE and ROLLUP planner tests - Full `pkg/sql/plan` test package - Focused `-race` run - `go build -mod=readonly ./pkg/sql/plan` - `go vet -mod=readonly ./pkg/sql/plan` - `git diff --check` --------- Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> Co-authored-by: XuPeng-SH <xupeng3112@163.com>
1 parent 27765e7 commit d4c70a0

5 files changed

Lines changed: 292 additions & 68 deletions

File tree

pkg/sql/plan/bind_context.go

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,22 @@ func NewBindContext(builder *QueryBuilder, parent *BindContext) *BindContext {
6868
return bc
6969
}
7070

71+
// newCTEDeclarationContext records the name-resolution scope at a WITH
72+
// declaration without retaining bindings that the declaring query block adds
73+
// later while binding its FROM clause. The normal child-context constructor
74+
// carries default-database, snapshot, CTE-state, rewrite, and view metadata;
75+
// detaching the new context from ctx then leaves only the already-existing
76+
// outer query blocks available for correlation.
77+
func newCTEDeclarationContext(builder *QueryBuilder, ctx *BindContext) *BindContext {
78+
declarationCtx := NewBindContext(builder, ctx)
79+
declarationCtx.parent = ctx.parent
80+
declarationCtx.cteByName = ctx.cteByName
81+
for name, cteRef := range ctx.boundCtes {
82+
declarationCtx.boundCtes[name] = cteRef
83+
}
84+
return declarationCtx
85+
}
86+
7187
func (bc *BindContext) rootTag() int32 {
7288
if bc.bindingRecurCte() && bc.sinkTag > 0 {
7389
return bc.sinkTag
Lines changed: 224 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,224 @@
1+
// Copyright 2026 Matrix Origin
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
package plan
16+
17+
import (
18+
"testing"
19+
20+
planpb "github.com/matrixorigin/matrixone/pkg/pb/plan"
21+
"github.com/matrixorigin/matrixone/pkg/sql/parsers/tree"
22+
"github.com/stretchr/testify/require"
23+
)
24+
25+
type cteViewTrackingContext struct {
26+
CompilerContext
27+
views []string
28+
}
29+
30+
func (c *cteViewTrackingContext) GetViews() []string {
31+
return c.views
32+
}
33+
34+
func (c *cteViewTrackingContext) SetViews(views []string) {
35+
c.views = append([]string(nil), views...)
36+
}
37+
38+
type cteViewTrackingOptimizer struct {
39+
ctx CompilerContext
40+
}
41+
42+
func (o *cteViewTrackingOptimizer) CurrentContext() CompilerContext {
43+
return o.ctx
44+
}
45+
46+
func (o *cteViewTrackingOptimizer) Optimize(stmt tree.Statement) (*Query, error) {
47+
logicPlan, err := BuildPlan(o.ctx, stmt, false)
48+
if err != nil {
49+
return nil, err
50+
}
51+
return logicPlan.GetQuery(), nil
52+
}
53+
54+
func collectGroupingFlags(query *Query, rootIDs ...int32) [][]bool {
55+
seen := make(map[int32]bool)
56+
groupingFlags := make([][]bool, 0)
57+
var visit func(int32)
58+
visit = func(nodeID int32) {
59+
if seen[nodeID] {
60+
return
61+
}
62+
seen[nodeID] = true
63+
node := query.Nodes[nodeID]
64+
if node.NodeType == planpb.Node_AGG {
65+
groupingFlags = append(groupingFlags, append([]bool(nil), node.GroupingFlag...))
66+
}
67+
for _, childID := range node.Children {
68+
visit(childID)
69+
}
70+
}
71+
for _, rootID := range rootIDs {
72+
visit(rootID)
73+
}
74+
return groupingFlags
75+
}
76+
77+
func requireRepeatedCTEGroupingFlags(t *testing.T, logicPlan *Plan, expected [][]bool) {
78+
t.Helper()
79+
query := logicPlan.GetQuery()
80+
require.NotNil(t, query)
81+
82+
for _, node := range query.Nodes {
83+
if node.NodeType != planpb.Node_JOIN || len(node.Children) != 2 {
84+
continue
85+
}
86+
left := collectGroupingFlags(query, node.Children[0])
87+
right := collectGroupingFlags(query, node.Children[1])
88+
if len(left) == 0 || len(right) == 0 {
89+
continue
90+
}
91+
require.ElementsMatch(t, expected, left, "left CTE consumer grouping variants")
92+
require.ElementsMatch(t, expected, right, "right CTE consumer grouping variants")
93+
return
94+
}
95+
t.Fatal("expected a join with grouping-set CTE consumers on both sides")
96+
}
97+
98+
func TestCTELazyBindingDeclarationScope(t *testing.T) {
99+
mock := NewMockOptimizer(false)
100+
101+
t.Run("unused invalid body is not bound", func(t *testing.T) {
102+
_, err := runOneStmt(mock, t, `
103+
with bad as (select missing_column from nation)
104+
select 1`)
105+
require.NoError(t, err)
106+
})
107+
108+
t.Run("referenced invalid body is bound", func(t *testing.T) {
109+
_, err := runOneStmt(mock, t, `
110+
with bad as (select missing_column from nation)
111+
select * from bad`)
112+
require.ErrorContains(t, err, "missing_column")
113+
})
114+
115+
t.Run("body cannot capture declaring block FROM", func(t *testing.T) {
116+
_, err := runOneStmt(mock, t, `
117+
with qn as (select * from bvt_test2.t2 where t2.b = t3.a)
118+
select * from bvt_test2.t3 where exists (select * from qn)`)
119+
require.ErrorContains(t, err, "missing FROM-clause entry for table 't3'")
120+
})
121+
122+
t.Run("body can correlate to an outer query block", func(t *testing.T) {
123+
_, err := runOneStmt(mock, t, `
124+
select (
125+
with qn as (select t2.a * t1.a as a from cte_test.t1),
126+
qn2 as (select 3 * a as b from qn)
127+
select * from qn2 limit 1
128+
)
129+
from bvt_test2.t2`)
130+
require.NoError(t, err)
131+
})
132+
}
133+
134+
func TestCTELazyBindingRollupSingleExpansion(t *testing.T) {
135+
mock := NewMockOptimizer(false)
136+
logicPlan, err := runOneStmt(mock, t, `
137+
with totals as (
138+
select n_regionkey, count(*) as n
139+
from nation
140+
group by n_regionkey with rollup
141+
)
142+
select * from totals`)
143+
require.NoError(t, err)
144+
145+
query := logicPlan.GetQuery()
146+
require.NotNil(t, query)
147+
require.ElementsMatch(t, [][]bool{{true}, {false}}, collectGroupingFlags(query, query.Steps...))
148+
}
149+
150+
func TestCTELazyBindingRepeatedGroupingSets(t *testing.T) {
151+
mock := NewMockOptimizer(false)
152+
153+
t.Run("rollup keeps both variants for each reference", func(t *testing.T) {
154+
logicPlan, err := runOneStmt(mock, t, `
155+
with totals as (
156+
select n_regionkey, count(*) as n
157+
from nation
158+
group by n_regionkey with rollup
159+
)
160+
select *
161+
from totals a join totals b on a.n_regionkey = b.n_regionkey`)
162+
require.NoError(t, err)
163+
requireRepeatedCTEGroupingFlags(t, logicPlan, [][]bool{{true}, {false}})
164+
})
165+
166+
t.Run("cube keeps all variants for each reference", func(t *testing.T) {
167+
logicPlan, err := runOneStmt(mock, t, `
168+
with totals as (
169+
select count(*) as n
170+
from nation
171+
group by cube(n_regionkey, n_nationkey)
172+
)
173+
select *
174+
from totals a join totals b on a.n = b.n`)
175+
require.NoError(t, err)
176+
requireRepeatedCTEGroupingFlags(t, logicPlan, [][]bool{
177+
{false, false},
178+
{true, false},
179+
{true, true},
180+
{false, true},
181+
})
182+
})
183+
}
184+
185+
func TestCTELazyBindingVisibilityGuards(t *testing.T) {
186+
mock := NewMockOptimizer(false)
187+
188+
t.Run("forward reference stays rejected", func(t *testing.T) {
189+
_, err := runOneStmt(mock, t, `
190+
with qn2 as (select a from qn),
191+
qn as (select a from cte_test.t1)
192+
select * from qn2`)
193+
require.Error(t, err)
194+
})
195+
196+
t.Run("self reference stays rejected", func(t *testing.T) {
197+
_, err := runOneStmt(mock, t, `
198+
with qn as (select * from qn)
199+
select * from qn`)
200+
require.ErrorContains(t, err, "recursive table must be referenced only once")
201+
})
202+
203+
t.Run("recursive reference stays accepted", func(t *testing.T) {
204+
_, err := runOneStmt(mock, t, `
205+
with recursive c as (
206+
select a from cte_test.t1
207+
union all
208+
select a + 1 from c where a < 3
209+
)
210+
select * from c`)
211+
require.NoError(t, err)
212+
})
213+
}
214+
215+
func TestCTELazyBindingKeepsRootContextOwnership(t *testing.T) {
216+
ctx := &cteViewTrackingContext{CompilerContext: NewMockCompilerContext(false)}
217+
mock := &cteViewTrackingOptimizer{ctx: ctx}
218+
219+
_, err := runOneStmt(mock, t, `
220+
with qn as (select * from cte_test.v2)
221+
select * from qn`)
222+
require.NoError(t, err)
223+
require.Contains(t, ctx.views, "cte_test#v2")
224+
}

0 commit comments

Comments
 (0)