diff --git a/pkg/models/saved_filter_positions_test.go b/pkg/models/saved_filter_positions_test.go index d233460d2..63220d383 100644 --- a/pkg/models/saved_filter_positions_test.go +++ b/pkg/models/saved_filter_positions_test.go @@ -24,6 +24,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "xorm.io/builder" ) func TestSavedFilterUpdateInsertsNonZeroPosition(t *testing.T) { @@ -304,3 +305,41 @@ 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) +} diff --git a/pkg/models/saved_filters.go b/pkg/models/saved_filters.go index 872f75588..00ebc96b3 100644 --- a/pkg/models/saved_filters.go +++ b/pkg/models/saved_filters.go @@ -541,6 +541,12 @@ func RegisterAddTaskToFilterViewCron() { } } +// deleteCondBatchSize is the maximum number of OR conditions in a single delete query. +// 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 { @@ -556,13 +562,21 @@ func upsertRelatedTaskProperties(s *xorm.Session, logPrefix string, newTaskBucke } } if len(deleteCond) > 0 { - _, err = s.Where(builder.Or(deleteCond...)).Delete(&TaskBucket{}) - if err != nil { - log.Errorf("%sError deleting task buckets: %s", logPrefix, err) - } - _, err = s.Where(builder.Or(deleteCond...)).Delete(&TaskPosition{}) - if err != nil { - log.Errorf("%sError deleting task positions: %s", logPrefix, err) + 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{}) + if err != nil { + log.Errorf("%sError deleting task buckets: %s", logPrefix, err) + } + _, err = s.Where(builder.Or(batch...)).Delete(&TaskPosition{}) + if err != nil { + log.Errorf("%sError deleting task positions: %s", logPrefix, err) + } } } }