From 4438649ee51cce13c455046c9a0dce6609d59991 Mon Sep 17 00:00:00 2001
From: Alec Merdler <alecmerdler@gmail.com>
Date: Thu, 28 Mar 2024 15:55:14 -0400
Subject: [PATCH] Fix re-creating deleted relationships

Ensures it is possible to re-create relationships that were
previously deleted using filters. This bug was caused by not
including the 'created_xid' column during deletion.

Addresses #1842
---
 internal/datastore/postgres/readwrite.go |  4 ++-
 pkg/datastore/test/datastore.go          |  1 +
 pkg/datastore/test/tuples.go             | 44 ++++++++++++++++++++++++
 3 files changed, 48 insertions(+), 1 deletion(-)

diff --git a/internal/datastore/postgres/readwrite.go b/internal/datastore/postgres/readwrite.go
index ae4a734dc3..1c827ce2de 100644
--- a/internal/datastore/postgres/readwrite.go
+++ b/internal/datastore/postgres/readwrite.go
@@ -59,6 +59,7 @@ var (
 		colUsersetNamespace,
 		colUsersetObjectID,
 		colUsersetRelation,
+		colCreatedXid,
 	).From(tableTuple).Where(sq.Eq{colDeletedXid: liveDeletedTxnID})
 )
 
@@ -412,7 +413,7 @@ func (rwt *pgReadWriteTXN) deleteRelationshipsWithLimit(ctx context.Context, fil
 
 	// Construct a CTE to update the relationships as removed.
 	cteSQL := fmt.Sprintf(
-		"WITH found_tuples AS (%s)\nUPDATE %s SET %s = $%d WHERE (%s, %s, %s, %s, %s, %s) IN (select * from found_tuples)",
+		"WITH found_tuples AS (%s)\nUPDATE %s SET %s = $%d WHERE (%s, %s, %s, %s, %s, %s, %s) IN (select * from found_tuples)",
 		selectSQL,
 		tableTuple,
 		colDeletedXid,
@@ -423,6 +424,7 @@ func (rwt *pgReadWriteTXN) deleteRelationshipsWithLimit(ctx context.Context, fil
 		colUsersetNamespace,
 		colUsersetObjectID,
 		colUsersetRelation,
+		colCreatedXid,
 	)
 
 	result, err := rwt.tx.Exec(ctx, cteSQL, args...)
diff --git a/pkg/datastore/test/datastore.go b/pkg/datastore/test/datastore.go
index 40c007c6f0..9e8ec85b8e 100644
--- a/pkg/datastore/test/datastore.go
+++ b/pkg/datastore/test/datastore.go
@@ -95,6 +95,7 @@ func AllWithExceptions(t *testing.T, tester DatastoreTester, except Categories)
 	t.Run("TestDeleteRelationships", func(t *testing.T) { DeleteRelationshipsTest(t, tester) })
 	t.Run("TestDeleteNonExistant", func(t *testing.T) { DeleteNotExistantTest(t, tester) })
 	t.Run("TestDeleteAlreadyDeleted", func(t *testing.T) { DeleteAlreadyDeletedTest(t, tester) })
+	t.Run("TestRecreateRelationshipsAfterDeleteWithFilter", func(t *testing.T) { RecreateRelationshipsAfterDeleteWithFilter(t, tester) })
 	t.Run("TestWriteDeleteWrite", func(t *testing.T) { WriteDeleteWriteTest(t, tester) })
 	t.Run("TestCreateAlreadyExisting", func(t *testing.T) { CreateAlreadyExistingTest(t, tester) })
 	t.Run("TestTouchAlreadyExistingWithoutCaveat", func(t *testing.T) { TouchAlreadyExistingTest(t, tester) })
diff --git a/pkg/datastore/test/tuples.go b/pkg/datastore/test/tuples.go
index 996229b912..61e43e65c7 100644
--- a/pkg/datastore/test/tuples.go
+++ b/pkg/datastore/test/tuples.go
@@ -987,6 +987,50 @@ func DeleteRelationshipsWithVariousFiltersTest(t *testing.T, tester DatastoreTes
 	}
 }
 
+func RecreateRelationshipsAfterDeleteWithFilter(t *testing.T, tester DatastoreTester) {
+	require := require.New(t)
+
+	rawDS, err := tester.New(0, veryLargeGCInterval, veryLargeGCWindow, 1)
+	require.NoError(err)
+
+	ds, _ := testfixtures.StandardDatastoreWithSchema(rawDS, require)
+	ctx := context.Background()
+
+	relationships := make([]*core.RelationTuple, 100)
+	for i := 0; i < 100; i++ {
+		relationships[i] = tuple.MustParse(fmt.Sprintf("document:%d#owner@user:first", i))
+	}
+
+	writeRelationships := func() error {
+		_, err := common.WriteTuples(ctx, ds, core.RelationTupleUpdate_CREATE, relationships...)
+		return err
+	}
+
+	deleteRelationships := func() error {
+		_, err := ds.ReadWriteTx(ctx, func(ctx context.Context, rwt datastore.ReadWriteTransaction) error {
+			delLimit := uint64(100)
+			_, err := rwt.DeleteRelationships(ctx, &v1.RelationshipFilter{
+				OptionalRelation: "owner",
+				OptionalSubjectFilter: &v1.SubjectFilter{
+					SubjectType:       "user",
+					OptionalSubjectId: "first",
+				},
+			}, options.WithDeleteLimit(&delLimit))
+			return err
+		})
+		return err
+	}
+
+	// Write the initial relationships.
+	require.NoError(writeRelationships())
+
+	// Delete the relationships.
+	require.NoError(deleteRelationships())
+
+	// Write the same relationships again.
+	require.NoError(writeRelationships())
+}
+
 // QueryRelationshipsWithVariousFiltersTest tests various relationship filters for query relationships.
 func QueryRelationshipsWithVariousFiltersTest(t *testing.T, tester DatastoreTester) {
 	tcs := []struct {