fix: restrict DELETE scope in removeDuplicityDescribingEntities

[dkrizan]

Summary

When a batch job completes, BatchJobActivityFinalizer merges activity revisions from individual chunks into a single revision. As part of this, removeDuplicityDescribingEntities deletes duplicate (entity_class, entity_id) rows to avoid PK violations before reassigning all rows to the merge target.

The DELETE query was missing an activity_revision_id filter on its outer WHERE clause:

-- before
delete from activity_describing_entity
where (entity_class, entity_id) in (...)  -- no revision scope
and (activity_revision_id, entity_class, entity_id) not in (...)

This caused two problems:

1. Performance — without activity_revision_id as the leading predicate, PostgreSQL cannot use the existing (activity_revision_id, entity_class, entity_id) index and falls back to a full sequential scan of the entire table on every batch job completion.

2. Data correctness — the unscoped DELETE could match and delete rows from old, unrelated activity revisions that happen to reference the same (entity_class, entity_id) as entities in the batch being merged, silently corrupting historical activity data.

Fix

Add (activity_revision_id in (:revisionIds) or activity_revision_id = :activityRevisionIdToMergeInto) as the leading predicate on the outer DELETE. This restricts the operation strictly to the chunk revisions being merged, enables index usage, and eliminates the risk of touching unrelated historical rows.

Test plan

• Run a batch job with multiple chunks (e.g. machine translate ≥ 6 keys) and verify it completes successfully
• Verify activity history for affected keys is intact after the job completes
• Run EXPLAIN ANALYZE on the DELETE query and confirm it uses an Index Scan rather than a Seq Scan

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes
  • Batch job cleanup operations now process an expanded set of entities during merge scenarios, ensuring more thorough data consolidation.

[coderabbitai]

📝 Walkthrough

Walkthrough

The delete query in the batch job activity finalizer is expanded to remove activity entities linked to both the specified revision IDs and the merge-target revision, broadening the scope of activity cleanup during batch operations.

Changes

Cohort / File(s)	Summary
Batch Activity Cleanup backend/data/src/main/kotlin/io/tolgee/batch/BatchJobActivityFinalizer.kt	Modified delete query for activity_describing_entity table to additionally remove rows where activity_revision_id equals the merge-target revision, in addition to existing condition filtering by entity class and ID.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested reviewers

• Anty0

Poem

🐰 A query gets wider, catching more in its sights,
The merge-target revision now sees the deletion lights,
Activity cleanup hops with broader might,
Batch jobs finalize with expanded delight! ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately reflects the main change: restricting DELETE scope in removeDuplicityDescribingEntities by adding activity_revision_id predicates to prevent full table scans and unintended deletion of unrelated rows.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch fix/batch-job-activity-describing-entity-full-scan

Tip

Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (1)

backend/data/src/main/kotlin/io/tolgee/batch/BatchJobActivityFinalizer.kt (1)

173-189: Optional: reduce repeated scope predicate with a CTE.

The condition activity_revision_id in (:revisionIds) or activity_revision_id = :activityRevisionIdToMergeInto appears three times (outer WHERE, IN-subquery, NOT IN-subquery). A CTE that materialises relevant_revisions once would eliminate the repetition and give the planner a single scan to reuse:

♻️ Suggested CTE refactor

-        delete from activity_describing_entity
-        where (activity_revision_id in (:revisionIds) or activity_revision_id = :activityRevisionIdToMergeInto)
-        and (entity_class, entity_id) in
-              (select entity_class, entity_id
-               from activity_describing_entity
-               where activity_revision_id in (:revisionIds)
-                  or activity_revision_id = :activityRevisionIdToMergeInto
-               group by entity_class, entity_id
-               having count(*) > 1)
-        and
-            (activity_revision_id, entity_class, entity_id) not in (
-            select min(activity_revision_id), entity_class, entity_id
-                from activity_describing_entity
-                where activity_revision_id in (:revisionIds)
-                    or activity_revision_id = :activityRevisionIdToMergeInto
-                group by entity_class, entity_id
-                having count(*) > 1)
+        with scoped as (
+            select activity_revision_id, entity_class, entity_id
+            from activity_describing_entity
+            where activity_revision_id in (:revisionIds)
+               or activity_revision_id = :activityRevisionIdToMergeInto
+        ),
+        duplicates as (
+            select entity_class, entity_id
+            from scoped
+            group by entity_class, entity_id
+            having count(*) > 1
+        ),
+        to_keep as (
+            select min(activity_revision_id) as activity_revision_id, entity_class, entity_id
+            from scoped
+            where (entity_class, entity_id) in (select entity_class, entity_id from duplicates)
+            group by entity_class, entity_id
+        )
+        delete from activity_describing_entity
+        where (activity_revision_id in (:revisionIds) or activity_revision_id = :activityRevisionIdToMergeInto)
+          and (entity_class, entity_id) in (select entity_class, entity_id from duplicates)
+          and (activity_revision_id, entity_class, entity_id) not in (
+              select activity_revision_id, entity_class, entity_id from to_keep)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@backend/data/src/main/kotlin/io/tolgee/batch/BatchJobActivityFinalizer.kt`
around lines 173 - 189, The SQL in BatchJobActivityFinalizer.kt repeats the
predicate "activity_revision_id in (:revisionIds) or activity_revision_id =
:activityRevisionIdToMergeInto" three times; refactor the DELETE query to define
a CTE (e.g., relevant_revisions) that materializes those revision ids once
(selecting activity_revision_id from unnest(:revisionIds) union all select
:activityRevisionIdToMergeInto) and then replace the three occurrences with
joins/subqueries referencing relevant_revisions (use
relevant_revisions.activity_revision_id in the outer WHERE, the IN-subquery and
the NOT IN-subquery and update the GROUP BY/HAVING/select min(...) usages
accordingly) so the planner can reuse a single scan and the predicate is not
duplicated.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@backend/data/src/main/kotlin/io/tolgee/batch/BatchJobActivityFinalizer.kt`:
- Around line 173-189: The SQL in BatchJobActivityFinalizer.kt repeats the
predicate "activity_revision_id in (:revisionIds) or activity_revision_id =
:activityRevisionIdToMergeInto" three times; refactor the DELETE query to define
a CTE (e.g., relevant_revisions) that materializes those revision ids once
(selecting activity_revision_id from unnest(:revisionIds) union all select
:activityRevisionIdToMergeInto) and then replace the three occurrences with
joins/subqueries referencing relevant_revisions (use
relevant_revisions.activity_revision_id in the outer WHERE, the IN-subquery and
the NOT IN-subquery and update the GROUP BY/HAVING/select min(...) usages
accordingly) so the planner can reuse a single scan and the predicate is not
duplicated.