memstore: don't increment node ref count when duplicate quad is inserted

cayleygraph · Oct 14, 2019 · 57ebfa2 · 57ebfa2
1 parent 58a00f3
commit 57ebfa2
Show file tree

Hide file tree

Showing 2 changed files with 59 additions and 3 deletions.
diff --git a/graph/graphtest/graphtest.go b/graph/graphtest/graphtest.go
@@ -57,6 +57,7 @@ var graphTests = []struct {
 	{"compare typed values", TestCompareTypedValues},
 	{"schema", TestSchema},
 	{"delete reinserted", TestDeleteReinserted},
+	{"delete reinserted dup", TestDeleteReinsertedDup},
 }
 
 func TestAll(t *testing.T, gen testutil.DatabaseFunc, conf *Config) {
@@ -1195,13 +1196,67 @@ func TestDeleteReinserted(t testing.TB, gen testutil.DatabaseFunc, _ *Config) {
 	})
 	require.NoError(t, err, "Add quadset failed")
 
+	ctx := context.TODO()
+
+	q := quad.Make("<bob>", "<follows>", "<sally>", nil)
 	for i := 0; i < 2; i++ {
-		err = w.AddQuad(quad.Make("<bob>", "<follows>", "<sally>", nil))
+		err = w.AddQuad(q)
 		require.NoError(t, err, "Add quad failed")
-		err = w.RemoveQuad(quad.Make("<bob>", "<follows>", "<sally>", nil))
+		err = w.RemoveQuad(q)
 		require.NoError(t, err, "Remove quad failed")
+		refs, err := graph.RefsOf(ctx, qs, []quad.Value{
+			q.Subject, q.Predicate, q.Object,
+		})
+		require.NoError(t, err, "Get values failed")
+		require.Len(t, refs, 3)
+		for _, r := range refs {
+			require.NotNil(t, r)
+		}
 	}
+}
+
+func TestDeleteReinsertedDup(t testing.TB, gen testutil.DatabaseFunc, _ *Config) {
+	qs, opts, closer := gen(t)
+	defer closer()
+
+	w := testutil.MakeWriter(t, qs, opts, MakeQuadSet()...)
+
+	err := w.AddQuadSet([]quad.Quad{
+		quad.Make("<bob>", "<status>", "Feeling happy", nil),
+		quad.Make("<sally>", "<follows>", "<jim>", nil),
+	})
+	require.NoError(t, err, "Add quadset failed")
+
+	ctx := context.TODO()
+
+	q := quad.Make("<bob>", "<follows>", "<x>", nil)
+	for i := 0; i < 2; i++ {
+		err = w.AddQuad(q)
+		require.NoError(t, err, "Add quad failed")
+		// must be ignored
+		err = w.AddQuad(q)
+		require.NoError(t, err, "Add quad failed")
+		err = w.RemoveQuad(q)
+		require.NoError(t, err, "Remove quad failed")
+
+		refs, err := graph.RefsOf(ctx, qs, []quad.Value{
+			q.Subject, q.Predicate,
+		})
+		require.NoError(t, err, "Get values failed")
+		require.Len(t, refs, 2)
+		for _, r := range refs {
+			require.NotNil(t, r)
+		}
 
+		// the node should be garbage-collected
+		refs, err = graph.RefsOf(ctx, qs, []quad.Value{
+			q.Object,
+		})
+		if err == nil {
+			// FIXME(dennwc): the graphlog.SplitDeltas adds an increment even though the quad is duplicated and ignored
+			t.Skip("value must be garbage-collected")
+		}
+	}
 }
 
 func irif(format string, args ...interface{}) quad.IRI {

diff --git a/graph/memstore/quadstore.go b/graph/memstore/quadstore.go
@@ -283,10 +283,11 @@ func (qs *QuadStore) indexesForQuad(q internalQuad) []*Tree {
 // AddQuad adds a quad to quad store. It returns an id of the quad.
 // False is returned as a second parameter if quad exists already.
 func (qs *QuadStore) AddQuad(q quad.Quad) (int64, bool) {
-	p, _ := qs.resolveQuad(q, true)
+	p, _ := qs.resolveQuad(q, false)
 	if id := qs.quads[p]; id != 0 {
 		return id, false
 	}
+	p, _ = qs.resolveQuad(q, true)
 	pr := &primitive{Quad: p}
 	id := qs.addPrimitive(pr)
 	qs.quads[p] = id