Skip to content

Commit 249e316

Browse files
committed
new algorithm for structural merge.
1 parent b4c53cb commit 249e316

File tree

4 files changed

+84
-64
lines changed

4 files changed

+84
-64
lines changed

fieldpath/pathelementmap.go

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ func MakePathElementValueMap(size int) PathElementValueMap {
4040
type pathElementValue struct {
4141
PathElement PathElement
4242
Value value.Value
43+
Index int
4344
}
4445

4546
type sortedPathElementValues []pathElementValue
@@ -48,38 +49,42 @@ type sortedPathElementValues []pathElementValue
4849
// be faster than doing it one at a time via Insert.
4950
func (spev sortedPathElementValues) Len() int { return len(spev) }
5051
func (spev sortedPathElementValues) Less(i, j int) bool {
51-
return spev[i].PathElement.Less(spev[j].PathElement)
52+
cmp := spev[i].PathElement.Compare(spev[j].PathElement)
53+
if cmp != 0 {
54+
return cmp < 0
55+
}
56+
return spev[i].Index < spev[j].Index
5257
}
5358
func (spev sortedPathElementValues) Swap(i, j int) { spev[i], spev[j] = spev[j], spev[i] }
5459

5560
// Insert adds the pathelement and associated value in the map.
56-
func (s *PathElementValueMap) Insert(pe PathElement, v value.Value) {
61+
func (s *PathElementValueMap) Insert(pe PathElement, v value.Value, index int) {
5762
loc := sort.Search(len(s.members), func(i int) bool {
5863
return !s.members[i].PathElement.Less(pe)
5964
})
6065
if loc == len(s.members) {
61-
s.members = append(s.members, pathElementValue{pe, v})
66+
s.members = append(s.members, pathElementValue{pe, v, index})
6267
return
6368
}
6469
if s.members[loc].PathElement.Equals(pe) {
6570
return
6671
}
6772
s.members = append(s.members, pathElementValue{})
6873
copy(s.members[loc+1:], s.members[loc:])
69-
s.members[loc] = pathElementValue{pe, v}
74+
s.members[loc] = pathElementValue{pe, v, index}
7075
}
7176

7277
// Get retrieves the value associated with the given PathElement from the map.
7378
// (nil, false) is returned if there is no such PathElement.
74-
func (s *PathElementValueMap) Get(pe PathElement) (value.Value, bool) {
79+
func (s *PathElementValueMap) Get(pe PathElement) (value value.Value, index int, found bool) {
7580
loc := sort.Search(len(s.members), func(i int) bool {
7681
return !s.members[i].PathElement.Less(pe)
7782
})
7883
if loc == len(s.members) {
79-
return nil, false
84+
return nil, 0, false
8085
}
8186
if s.members[loc].PathElement.Equals(pe) {
82-
return s.members[loc].Value, true
87+
return s.members[loc].Value, s.members[loc].Index, true
8388
}
84-
return nil, false
89+
return nil, 0, false
8590
}

fieldpath/pathelementmap_test.go

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -25,26 +25,30 @@ import (
2525
func TestPathElementValueMap(t *testing.T) {
2626
m := PathElementValueMap{}
2727

28-
if _, ok := m.Get(PathElement{FieldName: strptr("onion")}); ok {
28+
if _, _, ok := m.Get(PathElement{FieldName: strptr("onion")}); ok {
2929
t.Fatal("Unexpected path-element found in empty map")
3030
}
3131

32-
m.Insert(PathElement{FieldName: strptr("carrot")}, value.NewValueInterface("knife"))
33-
m.Insert(PathElement{FieldName: strptr("chive")}, value.NewValueInterface(2))
32+
m.Insert(PathElement{FieldName: strptr("carrot")}, value.NewValueInterface("knife"), 1)
33+
m.Insert(PathElement{FieldName: strptr("chive")}, value.NewValueInterface(2), 2)
3434

35-
if _, ok := m.Get(PathElement{FieldName: strptr("onion")}); ok {
35+
if _, _, ok := m.Get(PathElement{FieldName: strptr("onion")}); ok {
3636
t.Fatal("Unexpected path-element in map")
3737
}
3838

39-
if val, ok := m.Get(PathElement{FieldName: strptr("carrot")}); !ok {
39+
if val, index, ok := m.Get(PathElement{FieldName: strptr("carrot")}); !ok {
4040
t.Fatal("Missing path-element in map")
4141
} else if !value.Equals(val, value.NewValueInterface("knife")) {
4242
t.Fatalf("Unexpected value found: %#v", val)
43+
} else if index != 1 {
44+
t.Fatalf("Unexpected index found: %#v", index)
4345
}
4446

45-
if val, ok := m.Get(PathElement{FieldName: strptr("chive")}); !ok {
47+
if val, index, ok := m.Get(PathElement{FieldName: strptr("chive")}); !ok {
4648
t.Fatal("Missing path-element in map")
4749
} else if !value.Equals(val, value.NewValueInterface(2)) {
4850
t.Fatalf("Unexpected value found: %#v", val)
51+
} else if index != 2 {
52+
t.Fatalf("Unexpected index found: %#v", index)
4953
}
5054
}

typed/merge.go

Lines changed: 56 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -172,72 +172,83 @@ func (w *mergingWalker) visitListItems(t *schema.List, lhs, rhs value.List) (err
172172
}
173173
out := make([]interface{}, 0, int(math.Max(float64(rLen), float64(lLen))))
174174

175-
lhsOrder := make([]fieldpath.PathElement, 0, lLen)
176-
177-
// First, collect all LHS children.
178-
observedLHS := fieldpath.MakePathElementValueMap(lLen)
179-
if lhs != nil {
180-
for i := 0; i < lhs.Length(); i++ {
181-
child := lhs.At(i)
175+
createPathElementsValues := func(name string, list value.List) ([]fieldpath.PathElement, fieldpath.PathElementValueMap, ValidationErrors) {
176+
var errs ValidationErrors
177+
length := 0
178+
if list != nil {
179+
length = list.Length()
180+
}
181+
observed := fieldpath.MakePathElementValueMap(length)
182+
pes := make([]fieldpath.PathElement, 0, length)
183+
for i := 0; i < length; i++ {
184+
child := list.At(i)
182185
pe, err := listItemToPathElement(w.allocator, w.schema, t, i, child)
183186
if err != nil {
184-
errs = append(errs, errorf("lhs: element %v: %v", i, err.Error())...)
187+
errs = append(errs, errorf("%s: element %v: %v", name, i, err.Error())...)
185188
// If we can't construct the path element, we can't
186189
// even report errors deeper in the schema, so bail on
187190
// this element.
188191
continue
189192
}
190-
if _, ok := observedLHS.Get(pe); ok {
191-
errs = append(errs, errorf("lhs: duplicate entries for key %v", pe.String())...)
193+
if _, _, found := observed.Get(pe); found {
194+
errs = append(errs, errorf("%s: duplicate entries for key %v", name, pe.String())...)
195+
continue
192196
}
193-
observedLHS.Insert(pe, child)
194-
lhsOrder = append(lhsOrder, pe)
197+
observed.Insert(pe, child, i)
198+
pes = append(pes, pe)
195199
}
200+
return pes, observed, errs
196201
}
197202

198-
// Then merge with RHS children.
199-
observedRHS := fieldpath.MakePathElementSet(rLen)
200-
if rhs != nil {
201-
for i := 0; i < rhs.Length(); i++ {
202-
child := rhs.At(i)
203-
pe, err := listItemToPathElement(w.allocator, w.schema, t, i, child)
204-
if err != nil {
205-
errs = append(errs, errorf("rhs: element %v: %v", i, err.Error())...)
206-
// If we can't construct the path element, we can't
207-
// even report errors deeper in the schema, so bail on
208-
// this element.
209-
continue
210-
}
211-
if observedRHS.Has(pe) {
212-
errs = append(errs, errorf("rhs: duplicate entries for key %v", pe.String())...)
213-
continue
214-
}
215-
observedRHS.Insert(pe)
203+
lhsOrder, observedLHS, lhsErrs := createPathElementsValues("lhs", lhs)
204+
errs = append(errs, lhsErrs...)
205+
rhsOrder, observedRHS, rhsErrs := createPathElementsValues("rhs", rhs)
206+
errs = append(errs, rhsErrs...)
207+
208+
lLen, rLen = len(lhsOrder), len(rhsOrder)
209+
for lI, rI := 0, 0; lI < lLen || rI < rLen; {
210+
merge := func(pe fieldpath.PathElement, lChild, rChild value.Value) {
216211
w2 := w.prepareDescent(pe, t.ElementType)
217-
w2.rhs = child
218-
if lchild, ok := observedLHS.Get(pe); ok {
219-
w2.lhs = lchild
220-
}
212+
w2.lhs = lChild
213+
w2.rhs = rChild
221214
errs = append(errs, w2.merge(pe.String)...)
222215
if w2.out != nil {
223216
out = append(out, *w2.out)
224217
}
225218
w.finishDescent(w2)
226219
}
227-
}
228-
229-
for _, pe := range lhsOrder {
230-
if observedRHS.Has(pe) {
220+
if lI < lLen && rI < rLen && lhsOrder[lI].Equals(rhsOrder[rI]) {
221+
// merge LHS & RHS items
222+
pe := lhsOrder[lI]
223+
lChild, _, _ := observedLHS.Get(pe)
224+
rChild, _, _ := observedRHS.Get(pe)
225+
merge(pe, lChild, rChild)
226+
lI++
227+
rI++
231228
continue
232229
}
233-
value, _ := observedLHS.Get(pe)
234-
w2 := w.prepareDescent(pe, t.ElementType)
235-
w2.lhs = value
236-
errs = append(errs, w2.merge(pe.String)...)
237-
if w2.out != nil {
238-
out = append(out, *w2.out)
230+
if lI < lLen {
231+
if _, index, ok := observedRHS.Get(lhsOrder[lI]); ok && index < rI {
232+
// Skip the LHS item because it has already appeared
233+
lI++
234+
continue
235+
} else if !ok {
236+
// Take the LHS item, without a matching RHS item to merge with
237+
pe := lhsOrder[lI]
238+
lChild, _, _ := observedLHS.Get(pe)
239+
merge(pe, lChild, nil)
240+
lI++
241+
continue
242+
}
243+
}
244+
if rI < rLen {
245+
// Take the RHS item, merge with matching LHS item if possible
246+
pe := rhsOrder[rI]
247+
rChild, _, _ := observedRHS.Get(pe)
248+
lChild, _, _ := observedLHS.Get(pe) // may be nil
249+
merge(pe, lChild, rChild)
250+
rI++
239251
}
240-
w.finishDescent(w2)
241252
}
242253

243254
if len(out) > 0 {

typed/merge_test.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -283,11 +283,11 @@ var mergeCases = []mergeTestCase{{
283283
}, {
284284
`{"setBool":[true]}`,
285285
`{"setBool":[false]}`,
286-
`{"setBool":[false,true]}`,
286+
`{"setBool":[true, false]}`,
287287
}, {
288288
`{"setNumeric":[1,2,3.14159]}`,
289289
`{"setNumeric":[1,2,3]}`,
290-
`{"setNumeric":[1,2,3,3.14159]}`,
290+
`{"setNumeric":[1,2,3.14159,3]}`,
291291
}},
292292
}, {
293293
name: "associative list",
@@ -345,19 +345,19 @@ var mergeCases = []mergeTestCase{{
345345
}, {
346346
`{"list":[{"key":"a","id":1,"value":{"a":"a"}}]}`,
347347
`{"list":[{"key":"a","id":2,"value":{"a":"a"}}]}`,
348-
`{"list":[{"key":"a","id":2,"value":{"a":"a"}},{"key":"a","id":1,"value":{"a":"a"}}]}`,
348+
`{"list":[{"key":"a","id":1,"value":{"a":"a"}},{"key":"a","id":2,"value":{"a":"a"}}]}`,
349349
}, {
350350
`{"list":[{"key":"a","id":1},{"key":"b","id":1}]}`,
351351
`{"list":[{"key":"a","id":1},{"key":"a","id":2}]}`,
352-
`{"list":[{"key":"a","id":1},{"key":"a","id":2},{"key":"b","id":1}]}`,
352+
`{"list":[{"key":"a","id":1},{"key":"b","id":1},{"key":"a","id":2}]}`,
353353
}, {
354354
`{"list":[{"key":"b","id":2}]}`,
355355
`{"list":[{"key":"a","id":1},{"key":"b","id":2},{"key":"c","id":3}]}`,
356356
`{"list":[{"key":"a","id":1},{"key":"b","id":2},{"key":"c","id":3}]}`,
357357
}, {
358358
`{"list":[{"key":"a","id":1},{"key":"b","id":2},{"key":"c","id":3}]}`,
359359
`{"list":[{"key":"c","id":3},{"key":"b","id":2}]}`,
360-
`{"list":[{"key":"c","id":3},{"key":"b","id":2},{"key":"a","id":1}]}`,
360+
`{"list":[{"key":"a","id":1},{"key":"c","id":3},{"key":"b","id":2}]}`,
361361
}, {
362362
`{"list":[{"key":"a","id":1},{"key":"b","id":2},{"key":"c","id":3}]}`,
363363
`{"list":[{"key":"c","id":3},{"key":"a","id":1}]}`,

0 commit comments

Comments
 (0)