Skip to content

Commit 54dd6b6

Browse files
wcharczukwcharczuk
committed
fixes cycle detection with detectcycleiflinked
1 parent 6015072 commit 54dd6b6

File tree

5 files changed

+163
-30
lines changed

5 files changed

+163
-30
lines changed

‎detect_cycle_if_linked.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,9 @@ import "fmt"
1111
func DetectCycleIfLinked(child, parent INode) error {
1212
getParents := func(n INode) []INode {
1313
if n.Node().ID() == child.Node().ID() {
14-
return append(n.Node().Parents(), parent)
14+
return append(n.Parents(), parent)
1515
}
16-
return n.Node().Parents()
16+
return n.Parents()
1717
}
1818
if detectCycleFast(child.Node().ID(), parent /*startAt*/, getParents) {
1919
return fmt.Errorf("adding %v as child of %v would cause a cycle", child, parent)

‎detect_cycle_if_linked_test.go

+154
Original file line numberDiff line numberDiff line change
@@ -1 +1,155 @@
11
package incr
2+
3+
import (
4+
"testing"
5+
6+
"github.com/wcharczuk/go-incr/testutil"
7+
)
8+
9+
func Test_DetectCycleIfLinked(t *testing.T) {
10+
g := New()
11+
n0 := MapN[any, any](g, identMany)
12+
n01 := MapN[any, any](g, identMany)
13+
n02 := MapN[any, any](g, identMany)
14+
n03 := MapN[any, any](g, identMany)
15+
n1 := MapN[any, any](g, identMany)
16+
n11 := MapN[any, any](g, identMany)
17+
n12 := MapN[any, any](g, identMany)
18+
n13 := MapN[any, any](g, identMany)
19+
20+
n01.AddInput(n0)
21+
n02.AddInput(n01)
22+
n03.AddInput(n02)
23+
n1.AddInput(n02)
24+
n11.AddInput(n1)
25+
n12.AddInput(n11)
26+
27+
err := DetectCycleIfLinked(n13, n12)
28+
testutil.Nil(t, err)
29+
30+
err = DetectCycleIfLinked(n1, n12)
31+
testutil.NotNil(t, err)
32+
}
33+
34+
func detectCycleNode(label string) MapNIncr[any, any] {
35+
g := New()
36+
n := MapN[any, any](g, identMany)
37+
n.Node().SetLabel(label)
38+
return n
39+
}
40+
41+
func Test_DetectCycleIfLinked_complex(t *testing.T) {
42+
n0 := detectCycleNode("n0")
43+
n1 := detectCycleNode("n1")
44+
n2 := detectCycleNode("n2")
45+
46+
n1.AddInput(n0)
47+
n2.AddInput(n1)
48+
49+
n01 := detectCycleNode("n01")
50+
n02 := detectCycleNode("n02")
51+
52+
n01.AddInput(n2)
53+
n02.AddInput(n01)
54+
55+
n11 := detectCycleNode("n11")
56+
n12 := detectCycleNode("n12")
57+
n13 := detectCycleNode("n13")
58+
59+
n11.AddInput(n01)
60+
n12.AddInput(n11)
61+
n13.AddInput(n12)
62+
63+
n21 := detectCycleNode("n21")
64+
n22 := detectCycleNode("n22")
65+
n23 := detectCycleNode("n23")
66+
n24 := detectCycleNode("n24")
67+
68+
n21.AddInput(n11)
69+
n22.AddInput(n21)
70+
n23.AddInput(n22)
71+
n24.AddInput(n23)
72+
73+
err := DetectCycleIfLinked(n2, n02)
74+
testutil.NotNil(t, err)
75+
76+
err = DetectCycleIfLinked(n2, n13)
77+
testutil.NotNil(t, err)
78+
79+
err = DetectCycleIfLinked(n2, n24)
80+
testutil.NotNil(t, err)
81+
82+
err = DetectCycleIfLinked(n02, n13)
83+
testutil.Nil(t, err, "this should _not_ cause a cycle")
84+
85+
err = DetectCycleIfLinked(n01, n13)
86+
testutil.NotNil(t, err)
87+
}
88+
89+
func Test_DetectCycleIfLinked_complex2(t *testing.T) {
90+
n0 := detectCycleNode("n0")
91+
n10 := detectCycleNode("n10")
92+
n11 := detectCycleNode("n11")
93+
n12 := detectCycleNode("n12")
94+
n21 := detectCycleNode("n21")
95+
n22 := detectCycleNode("n22")
96+
97+
n10.AddInput(n0)
98+
n11.AddInput(n10)
99+
n12.AddInput(n11)
100+
n21.AddInput(n11)
101+
n22.AddInput(n21)
102+
103+
err := DetectCycleIfLinked(n10, n22)
104+
testutil.NotNil(t, err)
105+
}
106+
107+
func Test_DetectCycleIfLinked_2(t *testing.T) {
108+
/* these are some trivial cases to make sure bases are covered */
109+
g := New()
110+
n0 := MapN[any, any](g, identMany)
111+
n1 := MapN[any, any](g, identMany)
112+
n2 := MapN[any, any](g, identMany)
113+
114+
err := DetectCycleIfLinked(n0, n0)
115+
testutil.NotNil(t, err)
116+
n1.AddInput(n0)
117+
118+
err = DetectCycleIfLinked(n2, n1)
119+
testutil.Nil(t, err)
120+
121+
n2.AddInput(n1)
122+
123+
err = DetectCycleIfLinked(n0, n2)
124+
testutil.NotNil(t, err)
125+
}
126+
127+
func Test_DetectCycleIfLinked_regression(t *testing.T) {
128+
g := New()
129+
table := Var(g, "table")
130+
columnDownload := Map(g, table, ident)
131+
lastDownload := Map(g, columnDownload, ident)
132+
targetUpload := Map(g, lastDownload, ident)
133+
134+
columnUpload := Map(g, table, ident)
135+
lastUpload := Map(g, columnUpload, ident)
136+
uploadRemaining := MapN(g, identMany, lastUpload)
137+
138+
err := DetectCycleIfLinked(uploadRemaining, targetUpload)
139+
testutil.Nil(t, err, "this should _not_ cause a cycle!")
140+
}
141+
142+
func Test_DetectCycleIfLinked_regression2(t *testing.T) {
143+
g := New()
144+
table := Var(g, "table")
145+
columnDownload := MapN(g, identMany[string])
146+
lastDownload := Map(g, columnDownload, ident)
147+
_ = Map(g, lastDownload, ident)
148+
149+
columnUpload := Map(g, table, ident)
150+
lastUpload := Map(g, columnUpload, ident)
151+
_ = MapN(g, identMany, lastUpload)
152+
153+
err := DetectCycleIfLinked(columnDownload, table)
154+
testutil.Nil(t, err, "this should _not_ cause a cycle!")
155+
}

‎incrutil/dependency_graph.go

+1-3
Original file line numberDiff line numberDiff line change
@@ -75,9 +75,7 @@ func (dg DependencyGraph[Result]) createDependencyIncrLookup(ctx context.Context
7575
}
7676
for _, p := range dg.Dependencies {
7777
for _, d := range p.DependsOn {
78-
if err = output[p.Name].AddInput(output[d]); err != nil {
79-
return
80-
}
78+
output[p.Name].AddInput(output[d])
8179
}
8280
}
8381
return

‎map_n.go

+3-17
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ type MapNContextFunc[A, B any] func(context.Context, ...A) (B, error)
3232
// MapNIncr is a type of incremental that can add inputs over time.
3333
type MapNIncr[A, B any] interface {
3434
Incr[B]
35-
AddInput(Incr[A]) error
35+
AddInput(Incr[A])
3636
}
3737

3838
var (
@@ -58,23 +58,9 @@ func (mi *mapNIncr[A, B]) Parents() []INode {
5858
return output
5959
}
6060

61-
func (mn *mapNIncr[A, B]) AddInput(i Incr[A]) error {
61+
func (mn *mapNIncr[A, B]) AddInput(i Incr[A]) {
6262
mn.inputs = append(mn.inputs, i)
63-
graph := mn.n.createdIn.scopeGraph()
64-
if mn.n.isNecessary() {
65-
if i.Node().height >= mn.n.height {
66-
if err := graph.adjustHeightsHeap.setHeight(mn, i.Node().height+1); err != nil {
67-
return err
68-
}
69-
}
70-
if err := mn.n.createdIn.scopeGraph().addChild(mn, i); err != nil {
71-
return err
72-
}
73-
if mn.n.heightInRecomputeHeap == heightUnset {
74-
graph.recomputeHeap.add(mn)
75-
}
76-
}
77-
return nil
63+
7864
}
7965

8066
func (mn *mapNIncr[A, B]) Node() *Node { return mn.n }

‎stabilize_test.go

+3-8
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ import (
99
"testing"
1010
"time"
1111

12-
"github.com/wcharczuk/go-incr/testutil"
1312
. "github.com/wcharczuk/go-incr/testutil"
1413
)
1514

@@ -1135,13 +1134,9 @@ func Test_Stabilize_MapN_AddInput(t *testing.T) {
11351134
c1 := Return(g, 2)
11361135
c2 := Return(g, 3)
11371136
mn := MapN(g, sum)
1138-
var err error
1139-
err = mn.AddInput(c0)
1140-
testutil.NoError(t, err)
1141-
err = mn.AddInput(c1)
1142-
testutil.NoError(t, err)
1143-
err = mn.AddInput(c2)
1144-
testutil.NoError(t, err)
1137+
mn.AddInput(c0)
1138+
mn.AddInput(c1)
1139+
mn.AddInput(c2)
11451140

11461141
_ = MustObserve(g, mn)
11471142

0 commit comments

Comments
 (0)