Skip to content

Commit 5d12b6a

Browse files
authored
fix indexing for GROUP BYs and WINDOWs in INSERT and REPLACE statements in TRIGGERS (#2979)
1 parent 2815e98 commit 5d12b6a

File tree

5 files changed

+117
-22
lines changed

5 files changed

+117
-22
lines changed

enginetest/queries/trigger_queries.go

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -368,6 +368,50 @@ create trigger insert_into_a
368368
after insert on a
369369
for each row replace into c
370370
select d.x+2, 0 from d join b using (x)
371+
where d.x = new.x`,
372+
"insert into a (x,z) values (2,2)",
373+
},
374+
Query: "select x, y from c order by 1",
375+
Expected: []sql.Row{
376+
{4, 0},
377+
},
378+
},
379+
{
380+
Name: "trigger insert projection group by index error",
381+
SetUpScript: []string{
382+
"create table a (x int primary key, y int default 1, z int)",
383+
"create table b (x int primary key)",
384+
"create table c (x int primary key, y tinyint)",
385+
"create table d (x int primary key)",
386+
"insert into b values (1), (2)",
387+
"insert into d values (1), (2)",
388+
`
389+
create trigger insert_into_a
390+
after insert on a
391+
for each row replace into c
392+
select max(d.x + new.x), 0 from d join b using (x)
393+
where d.x = new.x`,
394+
"insert into a (x,z) values (2,2)",
395+
},
396+
Query: "select x, y from c order by 1",
397+
Expected: []sql.Row{
398+
{4, 0},
399+
},
400+
},
401+
{
402+
Name: "trigger insert projection window index error",
403+
SetUpScript: []string{
404+
"create table a (x int primary key, y int default 1, z int)",
405+
"create table b (x int primary key)",
406+
"create table c (x int primary key, y tinyint)",
407+
"create table d (x int primary key)",
408+
"insert into b values (1), (2)",
409+
"insert into d values (1), (2)",
410+
`
411+
create trigger insert_into_a
412+
after insert on a
413+
for each row replace into c
414+
select first_value(d.x + new.x) over (partition by (x) order by x), 0 from d join b using (x)
371415
where d.x = new.x`,
372416
"insert into a (x,z) values (2,2)",
373417
},

sql/analyzer/fix_exec_indexes.go

Lines changed: 42 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ func assignExecIndexes(ctx *sql.Context, a *Analyzer, n sql.Node, scope *plan.Sc
3232
if !scope.IsEmpty() {
3333
// triggers
3434
s.triggerScope = true
35+
s.insertSourceScope = scope.InInsertSource()
3536
s.addSchema(scope.Schema())
3637
s = s.push()
3738
}
@@ -161,7 +162,33 @@ type idxScope struct {
161162
children []sql.Node
162163
expressions []sql.Expression
163164
checks sql.CheckConstraints
164-
triggerScope bool
165+
166+
triggerScope bool
167+
insertSourceScope bool
168+
}
169+
170+
func (s *idxScope) inTrigger() bool {
171+
if s == nil {
172+
return false
173+
}
174+
for _, ps := range s.parentScopes {
175+
if ps.inTrigger() {
176+
return true
177+
}
178+
}
179+
return s.triggerScope
180+
}
181+
182+
func (s *idxScope) inInsertSource() bool {
183+
if s == nil {
184+
return false
185+
}
186+
for _, ps := range s.parentScopes {
187+
if ps.inInsertSource() {
188+
return true
189+
}
190+
}
191+
return s.insertSourceScope
165192
}
166193

167194
func (s *idxScope) addSchema(sch sql.Schema) {
@@ -535,6 +562,20 @@ func (s *idxScope) visitSelf(n sql.Node) error {
535562
n.DestSch[colIdx].Default = newDef.(*sql.ColumnDefaultValue)
536563
}
537564
default:
565+
// Group By and Window functions already account for the new/old columns present from triggers
566+
// This means that when indexing the Projections, we should not include the trigger scope(s), which are
567+
// within s.parentScopes.
568+
if proj, isProj := n.(*plan.Project); isProj {
569+
switch proj.Child.(type) {
570+
case *plan.GroupBy, *plan.Window:
571+
if s.inTrigger() && s.inInsertSource() {
572+
for _, e := range proj.Expressions() {
573+
s.expressions = append(s.expressions, fixExprToScope(e, s.childScopes...))
574+
}
575+
return nil
576+
}
577+
}
578+
}
538579
if ne, ok := n.(sql.Expressioner); ok {
539580
scope := append(s.parentScopes, s.childScopes...)
540581
for _, e := range ne.Expressions() {

sql/analyzer/inserts.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,8 @@ import (
3131
func resolveInsertRows(ctx *sql.Context, a *Analyzer, n sql.Node, scope *plan.Scope, sel RuleSelector, qFlags *sql.QueryFlags) (sql.Node, transform.TreeIdentity, error) {
3232
if _, ok := n.(*plan.TriggerExecutor); ok {
3333
return n, transform.SameTree, nil
34-
} else if _, ok := n.(*plan.CreateProcedure); ok {
34+
}
35+
if _, ok := n.(*plan.CreateProcedure); ok {
3536
return n, transform.SameTree, nil
3637
}
3738
// We capture all INSERTs along the tree, such as those inside of block statements.
@@ -50,14 +51,15 @@ func resolveInsertRows(ctx *sql.Context, a *Analyzer, n sql.Node, scope *plan.Sc
5051

5152
source := insert.Source
5253
// TriggerExecutor has already been analyzed
53-
if _, ok := insert.Source.(*plan.TriggerExecutor); !ok && !insert.LiteralValueSource {
54+
if _, isTrigExec := insert.Source.(*plan.TriggerExecutor); !isTrigExec && !insert.LiteralValueSource {
5455
// Analyze the source of the insert independently
5556
if _, ok := insert.Source.(*plan.Values); ok {
5657
scope = scope.NewScope(plan.NewProject(
5758
expression.SchemaToGetFields(insert.Source.Schema()[:len(insert.ColumnNames)], sql.ColSet{}),
5859
plan.NewSubqueryAlias("dummy", "", insert.Source),
5960
))
6061
}
62+
scope.SetInInsertSource(true)
6163
source, _, err = a.analyzeWithSelector(ctx, insert.Source, scope, SelectAllBatches, newInsertSourceSelector(sel), qFlags)
6264
if err != nil {
6365
return nil, transform.SameTree, err

sql/plan/scope.go

Lines changed: 26 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -44,20 +44,8 @@ type Scope struct {
4444
inLateralJoin bool
4545
joinSiblings []sql.Node
4646
JoinTrees []string
47-
}
4847

49-
func (s *Scope) SetJoin(b bool) {
50-
if s == nil {
51-
return
52-
}
53-
s.inJoin = b
54-
}
55-
56-
func (s *Scope) SetLateralJoin(b bool) {
57-
if s == nil {
58-
return
59-
}
60-
s.inLateralJoin = b
48+
inInsertSource bool
6149
}
6250

6351
func (s *Scope) IsEmpty() bool {
@@ -318,18 +306,37 @@ func (s *Scope) Schema() sql.Schema {
318306
return schema
319307
}
320308

321-
func (s *Scope) InJoin() bool {
309+
func (s *Scope) SetJoin(b bool) {
310+
if s == nil {
311+
return
312+
}
313+
s.inJoin = b
314+
}
315+
316+
func (s *Scope) SetLateralJoin(b bool) {
322317
if s == nil {
323-
return false
318+
return
324319
}
325-
return s.inJoin
320+
s.inLateralJoin = b
326321
}
327322

328-
func (s *Scope) InLateralJoin() bool {
323+
func (s *Scope) SetInInsertSource(b bool) {
329324
if s == nil {
330-
return false
325+
return
331326
}
332-
return s.inLateralJoin
327+
s.inInsertSource = b
328+
}
329+
330+
func (s *Scope) InJoin() bool {
331+
return s != nil && s.inJoin
332+
}
333+
334+
func (s *Scope) InLateralJoin() bool {
335+
return s != nil && s.inLateralJoin
336+
}
337+
338+
func (s *Scope) InInsertSource() bool {
339+
return s != nil && s.inInsertSource
333340
}
334341

335342
func (s *Scope) JoinSiblings() []sql.Node {

sql/rowexec/show.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,7 @@ func (b *BaseBuilder) buildDescribeQuery(ctx *sql.Context, n *plan.DescribeQuery
7676
var rows []sql.Row
7777
if n.Format.Plan {
7878
formatString := sql.Describe(n.Child, n.Format)
79+
formatString = strings.Replace(formatString, "\r", "", -1)
7980
for _, l := range strings.Split(formatString, "\n") {
8081
if strings.TrimSpace(l) != "" {
8182
rows = append(rows, sql.NewRow(l))

0 commit comments

Comments
 (0)