Fix empty sequence in getCompositeChildSequence (#2167)

Based on a patch by mike@mikegray.org

go/types/meta_sequence.go

@@ -162,6 +162,10 @@ func (ms metaSequenceObject) beginFetchingChildSequences(start, length uint64) c
 // Returns the sequences pointed to by all items[i], s.t. start <= i < end, and returns the
 // concatentation as one long composite sequence
 func (ms metaSequenceObject) getCompositeChildSequence(start uint64, length uint64) sequence {
+	if length == 0 {
+		return emptySequence{}
+	}
+
 	metaItems := []metaTuple{}
 	mapItems := []mapEntry{}
 	valueItems := []Value{}
@@ -241,3 +245,41 @@ func metaHashValueBytes(item sequenceItem, rv *rollingValueHasher) {
 	hashValueBytes(mt.ref, rv)
 	hashValueBytes(v, rv)
 }
+
+type emptySequence struct{}
+
+func (es emptySequence) getItem(idx int) sequenceItem {
+	panic("empty sequence")
+}
+
+func (es emptySequence) seqLen() int {
+	return 0
+}
+
+func (es emptySequence) numLeaves() uint64 {
+	return 0
+}
+
+func (es emptySequence) valueReader() ValueReader {
+	return nil
+}
+
+func (es emptySequence) Chunks() (chunks []Ref) {
+	return
+}
+
+func (es emptySequence) Type() *Type {
+	panic("empty sequence")
+}
+
+func (es emptySequence) getCompareFn(other sequence) compareFn {
+	return func(idx, otherIdx int) bool { panic("empty sequence") }
+}
+
+func (es emptySequence) getKey(idx int) orderedKey {
+	panic("empty sequence")
+}
+
+func (es emptySequence) getOffset(idx int) uint64 {
+	panic("empty sequence")
+}

go/types/ordered_sequences_diff_test.go

@@ -7,6 +7,7 @@ package types
 import (
 	"testing"
 
+	"github.com/attic-labs/testify/assert"
 	"github.com/attic-labs/testify/suite"
 )
 
@@ -175,3 +176,48 @@ func TestOrderedSequencesDiffCloseWithoutReading(t *testing.T) {
 	runTest(orderedSequenceDiffLeftRight)
 	runTest(orderedSequenceDiffTopDown)
 }
+
+func TestOrderedSequenceDiffWithMetaNodeGap(t *testing.T) {
+	assert := assert.New(t)
+
+	newSetSequenceMt := func(v ...Value) metaTuple {
+		seq := newSetLeafSequence(nil, v...)
+		set := newSet(seq)
+		return newMetaTuple(NewRef(set), newOrderedKey(v[len(v)-1]), uint64(len(v)), set)
+	}
+
+	m1 := newSetSequenceMt(Number(1), Number(2))
+	m2 := newSetSequenceMt(Number(3), Number(4))
+	m3 := newSetSequenceMt(Number(5), Number(6))
+	s1 := newSetMetaSequence([]metaTuple{m1, m3}, nil)
+	s2 := newSetMetaSequence([]metaTuple{m1, m2, m3}, nil)
+
+	runTest := func(df diffFn) {
+		changes := make(chan ValueChanged)
+		go func() {
+			df(s1, s2, changes, nil)
+			changes <- ValueChanged{}
+			df(s2, s1, changes, nil)
+			close(changes)
+		}()
+
+		expected := []ValueChanged{
+			{DiffChangeAdded, Number(3)},
+			{DiffChangeAdded, Number(4)},
+			{},
+			{DiffChangeRemoved, Number(3)},
+			{DiffChangeRemoved, Number(4)},
+		}
+
+		i := 0
+		for c := range changes {
+			assert.Equal(expected[i], c)
+			i++
+		}
+		assert.Equal(len(expected), i)
+	}
+
+	runTest(orderedSequenceDiffBest)
+	runTest(orderedSequenceDiffLeftRight)
+	runTest(orderedSequenceDiffTopDown)
+}

go/util/orderedparallel/ordered_parallel.go

@@ -7,12 +7,16 @@ package orderedparallel
 import (
 	"container/heap"
 	"sync"
+
+	"github.com/attic-labs/noms/go/d"
 )
 
 type ProcessFn func(in interface{}) (out interface{})
 
 // Creates a pool of |parallelism| goroutines to process values off of |input| by calling |fn| and guarentees that results of each call will be sent on |out| in the order the corresponding input was received.
 func New(input chan interface{}, fn ProcessFn, parallelism int) chan interface{} {
+	d.Chk.True(parallelism > 0)
+
 	mu := &sync.Mutex{}
 	inCount := uint(0)
 	outCount := uint(0)

js/src/meta-sequence.js

@@ -20,6 +20,7 @@ import {
   makeRefType,
   makeSetType,
   makeUnionType,
+  valueType,
 } from './type.js';
 import {IndexedSequence} from './indexed-sequence.js';
 import {invariant, notNull} from './assert.js';
@@ -211,8 +212,11 @@ export class IndexedMetaSequence extends IndexedSequence<MetaTuple> {
 
   // Returns the sequences pointed to by all items[i], s.t. start <= i < end, and returns the
   // concatentation as one long composite sequence
-  getCompositeChildSequence(start: number, length: number):
-      Promise<IndexedSequence> {
+  getCompositeChildSequence(start: number, length: number): Promise<IndexedSequence> {
+    if (length === 0) {
+      return Promise.resolve(new EmptySequence());
+    }
+
     const childrenP = [];
     for (let i = start; i < start + length; i++) {
       childrenP.push(this.items[i].getChildSequence(this.vr));
@@ -341,3 +345,9 @@ export function newIndexedMetaSequenceChunkFn(kind: NomsKind, vr: ?ValueReader):
 function getMetaSequenceChunks(ms: MetaSequence): Array<Ref> {
   return ms.items.map(mt => mt.ref);
 }
+
+class EmptySequence extends IndexedSequence {
+  constructor() {
+    super(null, valueType, []);
+  }
+}

js/src/ordered-sequence-diff-test.js

@@ -6,11 +6,13 @@
 
 import {suite, suiteSetup, suiteTeardown, test} from 'mocha';
 import {assert} from 'chai';
-import {fastForward} from './ordered-sequence-diff.js';
-import Set from './set.js';
+import {MetaTuple, newSetMetaSequence, OrderedKey} from './meta-sequence.js';
+import {default as diff, fastForward} from './ordered-sequence-diff.js';
+import {default as Set, newSetLeafSequence} from './set.js';
+import Ref from './ref.js';
 import {smallTestChunks, normalProductionChunks} from './rolling-value-hasher.js';
 
-suite('OrderedSequenceCursor', () => {
+suite('OrderedSequence', () => {
   suiteSetup(() => {
     smallTestChunks();
   });
@@ -91,4 +93,30 @@ suite('OrderedSequenceCursor', () => {
       assert.isFalse(cur2.valid);
     }
   });
+
+  test('diff with meta node gap', async () => {
+    const newSetSequenceMt = values => {
+      const seq = newSetLeafSequence(null, values);
+      const set = Object.create(Set.prototype);
+      set.sequence = seq;
+      return new MetaTuple(
+          new Ref(set), new OrderedKey(values[values.length - 1]), values.length, set);
+    };
+
+    const m1 = newSetSequenceMt([1, 2]);
+    const m2 = newSetSequenceMt([3, 4]);
+    const m3 = newSetSequenceMt([5, 6]);
+    const s1 = newSetMetaSequence(null, [m1, m3]);
+    const s2 = newSetMetaSequence(null, [m1, m2, m3]);
+
+    let [add, rem, mod] = await diff(s1, s2);
+    assert.deepEqual([3, 4], add);
+    assert.deepEqual([], rem);
+    assert.deepEqual([], mod);
+
+    [add, rem, mod] = await diff(s2, s1);
+    assert.deepEqual([], add);
+    assert.deepEqual([3, 4], rem);
+    assert.deepEqual([], mod);
+  });
 });