refactor: use per-view IN clause for filter task deletion instead of batching

pkg/models/saved_filter_positions_test.go

@@ -24,7 +24,6 @@ import (
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
-	"xorm.io/builder"
 )
 
 func TestSavedFilterUpdateInsertsNonZeroPosition(t *testing.T) {
@@ -305,41 +304,3 @@ func TestIssue724_SortingOnFilteredViews(t *testing.T) {
 	assert.Zero(t, zeroCount,
 		"No position=0 records should exist in database for view %d", view.ID)
 }
-
-func TestUpsertRelatedTaskPropertiesBatchesDeletes(t *testing.T) {
-	db.LoadAndAssertFixtures(t)
-	s := db.NewSession()
-	defer s.Close()
-
-	viewID := int64(1)
-
-	// Insert many task buckets and positions that we'll then delete
-	for i := int64(1); i <= 1200; i++ {
-		_, err := s.Insert(&TaskBucket{TaskID: 10000 + i, ProjectViewID: viewID, BucketID: 1})
-		require.NoError(t, err)
-		_, err = s.Insert(&TaskPosition{TaskID: 10000 + i, ProjectViewID: viewID, Position: float64(i)})
-		require.NoError(t, err)
-	}
-
-	// Build 1200 delete conditions -- this would exceed SQLite's 1000-node limit
-	// if passed as a single builder.Or()
-	deleteCond := make([]builder.Cond, 0, 1200)
-	for i := int64(1); i <= 1200; i++ {
-		deleteCond = append(deleteCond, builder.And(
-			builder.Eq{"task_id": 10000 + i},
-			builder.Eq{"project_view_id": viewID},
-		))
-	}
-
-	// This should not panic or error -- it should batch the deletes
-	upsertRelatedTaskProperties(s, "[test] ", nil, nil, deleteCond)
-
-	// Verify all were deleted
-	count, err := s.Where("project_view_id = ? AND task_id > 10000", viewID).Count(&TaskBucket{})
-	require.NoError(t, err)
-	assert.Equal(t, int64(0), count)
-
-	count, err = s.Where("project_view_id = ? AND task_id > 10000", viewID).Count(&TaskPosition{})
-	require.NoError(t, err)
-	assert.Equal(t, int64(0), count)
-}

pkg/models/saved_filters.go

@@ -429,7 +429,7 @@ func RegisterAddTaskToFilterViewCron() {
 		filterTasksCache := make(map[int64][]*Task)
 		newTaskBuckets := []*TaskBucket{}
 		newTaskPositions := []*TaskPosition{}
-		deleteCond := []builder.Cond{}
+
 		viewsToRecalc := map[int64]struct {
 			view    *ProjectView
 			ownerID int64
@@ -507,24 +507,10 @@ func RegisterAddTaskToFilterViewCron() {
 			}
 
 			// Remove tasks that should not be there
-			for taskID := range savedTaskBucketMap {
+			deleteStaleFilterTasks(s, logPrefix, savedTaskBucketMap, tasks, view.ID)
-				found := false
-				for _, task := range tasks {
-					if task.ID == taskID {
-						found = true
-						break
-					}
-				}
-				if !found {
-					deleteCond = append(deleteCond, builder.And(
-						builder.Eq{"task_id": taskID},
-						builder.Eq{"project_view_id": view.ID},
-					))
-				}
-			}
 		}
 
-		upsertRelatedTaskProperties(s, logPrefix, newTaskBuckets, newTaskPositions, deleteCond)
+		upsertRelatedTaskProperties(s, logPrefix, newTaskBuckets, newTaskPositions)
 
 		for _, data := range viewsToRecalc {
 			if err := RecalculateTaskPositions(s, data.view, &user.User{ID: data.ownerID}); err != nil {
@@ -541,13 +527,7 @@ func RegisterAddTaskToFilterViewCron() {
 	}
 }
 
-// deleteCondBatchSize is the maximum number of OR conditions in a single delete query.
+func upsertRelatedTaskProperties(s *xorm.Session, logPrefix string, newTaskBuckets []*TaskBucket, newTaskPositions []*TaskPosition) {
-// SQLite has a hard limit of 1000 expression tree nodes; each builder.And(...) with two
-// Eq conditions contributes ~3 nodes to the Or-tree, so 500 conditions stays safely
-// under the limit on all supported databases.
-const deleteCondBatchSize = 500
-
-func upsertRelatedTaskProperties(s *xorm.Session, logPrefix string, newTaskBuckets []*TaskBucket, newTaskPositions []*TaskPosition, deleteCond []builder.Cond) {
 	var err error
 	if len(newTaskBuckets) > 0 {
 		_, err = s.Insert(newTaskBuckets)
@@ -561,22 +541,34 @@ func upsertRelatedTaskProperties(s *xorm.Session, logPrefix string, newTaskBucke
 			log.Errorf("%sError inserting task positions: %s", logPrefix, err)
 		}
 	}
-	if len(deleteCond) > 0 {
+}
-		for i := 0; i < len(deleteCond); i += deleteCondBatchSize {
-			end := i + deleteCondBatchSize
-			if end > len(deleteCond) {
-				end = len(deleteCond)
-			}
-			batch := deleteCond[i:end]
 
-			_, err = s.Where(builder.Or(batch...)).Delete(&TaskBucket{})
+func deleteStaleFilterTasks(s *xorm.Session, logPrefix string, savedTaskBucketMap map[int64]*TaskBucket, tasks []*Task, viewID int64) {
-			if err != nil {
+	var taskIDsToDelete []int64
-				log.Errorf("%sError deleting task buckets: %s", logPrefix, err)
+	for taskID := range savedTaskBucketMap {
-			}
+		found := false
-			_, err = s.Where(builder.Or(batch...)).Delete(&TaskPosition{})
+		for _, task := range tasks {
-			if err != nil {
+			if task.ID == taskID {
-				log.Errorf("%sError deleting task positions: %s", logPrefix, err)
+				found = true
+				break
 			}
 		}
+		if !found {
+			taskIDsToDelete = append(taskIDsToDelete, taskID)
+		}
+	}
+	if len(taskIDsToDelete) > 0 {
+		_, err := s.Where(builder.Eq{"project_view_id": viewID}).
+			And(builder.In("task_id", taskIDsToDelete)).
+			Delete(&TaskBucket{})
+		if err != nil {
+			log.Errorf("%sError deleting task buckets: %s", logPrefix, err)
+		}
+		_, err = s.Where(builder.Eq{"project_view_id": viewID}).
+			And(builder.In("task_id", taskIDsToDelete)).
+			Delete(&TaskPosition{})
+		if err != nil {
+			log.Errorf("%sError deleting task positions: %s", logPrefix, err)
+		}
 	}
 }