SPOC-389: Fix resource leaks

[danolivo]

A few functions failed to release resources in early returns. I noticed the one with row filters while working on snapshot management changes in PostgreSQL when I added an assertion that when a historic snapshot is free'd, it must not still be marked as active. The leaks in row filter processing are otherwise harmlesss AFAICS, but if a lot of rows are filtered out, the resource owner holding all the resources can grow very large.

cherry-pick of: 2ndQuadrant/pglogical@2da0c43

[coderabbitai]

📝 Walkthrough

Walkthrough

In handle_insert (in src/spock_apply.c) the per-tuple path for same-relation multi-insert now calls end_replication_step() after spock_apply_heap_mi_add_tuple(rel, &newtup); and returns early; no other logic or error handling was changed.

Changes

Cohort / File(s)	Summary
Multi-insert replication step src/spock_apply.c	Added an explicit end_replication_step() call and early return in the same-relation multi-insert branch after adding a tuple; no other signature or error-handling changes.

Poem

🐇 I hopped through tuples, one by one,

Called the step's end when the insert was done;
No lingering steps, no stray affair,
A tidy relay handled with care.

🚥 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
Title check	✅ Passed	The title 'SPOC-389: Fix resource leaks' clearly summarizes the main objective of the changeset, which is to fix resource leaks in the handle_insert function by calling end_replication_step() for proper resource cleanup.
Description check	✅ Passed	The description is directly related to the changeset, explaining the resource leak issue in row filter processing and early returns that motivated the fix, matching the code changes in src/spock_apply.c.

_{✏️ 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 spoc-389

---

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]

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@src/spock_apply.c`:
- Around line 1250-1256: Remove the premature spock_relation_close(rel, NoLock)
call inside the multi-insert buffering path: when you call
spock_apply_heap_mi_add_tuple(...) you must keep the underlying relation open
because spkmistate->rel is shared and multi_insert_finish() (which calls
spock_apply_heap_mi_flush()) expects spkmistate->rel->rel to be valid; instead
close the SpockRelation only in multi_insert_finish() after flushing buffered
tuples. Ensure references to spock_apply_heap_mi_add_tuple,
spock_relation_close, spkmistate->rel, spock_apply_heap_mi_flush, and
multi_insert_finish are updated accordingly so the relation remains open while
tuples are buffered and is closed only after flush.