Skip to content

fix: use gc db builder for correct query placheolder#2737

Merged
omer-topal merged 1 commit intoPermify:masterfrom
KhokhlovDV:fix/gc-builder-placeholders
Jan 21, 2026
Merged

fix: use gc db builder for correct query placheolder#2737
omer-topal merged 1 commit intoPermify:masterfrom
KhokhlovDV:fix/gc-builder-placeholders

Conversation

@KhokhlovDV
Copy link
Copy Markdown

@KhokhlovDV KhokhlovDV commented Jan 21, 2026

Summary by CodeRabbit

Release Notes

  • Refactor
    • Updated internal database query generation to support flexible placeholder formats for improved compatibility with different database configurations.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Jan 21, 2026

📝 Walkthrough

Walkthrough

Three files in the PostgreSQL storage layer were updated to refactor GC query generation. The GenerateGCQuery and GenerateGCQueryForTenant functions now accept a Squirrel StatementBuilderType as their first parameter, enabling placeholder-aware query construction. Call sites and tests were updated accordingly to pass the builder explicitly.

Changes

Cohort / File(s) Summary
GC Query Generation Refactoring
internal/storage/postgres/utils/common.go
Updated function signatures for GenerateGCQuery and GenerateGCQueryForTenant to accept squirrel.StatementBuilderType as first parameter; implementation now uses the injected builder instead of package-level squirrel.Delete()
GC Call Site Update
internal/storage/postgres/gc/gc.go
Modified deleteRecordsForTenant to pass gc.database.Builder as the first argument when calling GenerateGCQueryForTenant
Test Coverage Extension
internal/storage/postgres/utils/common_test.go
Updated all test invocations to pass placeholder-aware builder as first argument; added new test cases for Dollar placeholder format alongside existing Question placeholder tests

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Poem

🐰 A builder passed through functions neat,
No more default squirrel calls to greet,
Placeholders dance in Question or Dollar form,
Tests confirm each query's proper norm,
Query construction, refactored and bright! ✨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title refers to a specific implementation detail (using gc db builder for correct query placeholder) that is directly reflected in the changeset modifications to GenerateGCQuery and GenerateGCQueryForTenant functions.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

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

✨ Finishing touches
  • 📝 Generate docstrings

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

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

@codecov
Copy link
Copy Markdown

codecov Bot commented Jan 21, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.60%. Comparing base (6bec092) to head (364beeb).
⚠️ Report is 2 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2737      +/-   ##
==========================================
+ Coverage   82.56%   82.60%   +0.04%     
==========================================
  Files          74       74              
  Lines        8125     8125              
==========================================
+ Hits         6708     6711       +3     
+ Misses        902      894       -8     
- Partials      515      520       +5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@omer-topal omer-topal merged commit 82dd860 into Permify:master Jan 21, 2026
12 of 13 checks passed
@omer-topal omer-topal linked an issue Jan 21, 2026 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] Garbage Collection fails on PostgreSQL with syntax error

2 participants