feat: Add filter option for pg_dump#168
feat: Add filter option for pg_dump#168gautamsi wants to merge 10 commits intoeduardolat:developfrom
Conversation
Merge develop into main for v0.5.1-rc.1
🥇 Thanks FetchGoat for upgrading to Gold Sponsor
Co-authored-by: gautamsi <5769869+gautamsi@users.noreply.github.com>
Co-authored-by: gautamsi <5769869+gautamsi@users.noreply.github.com>
Co-authored-by: gautamsi <5769869+gautamsi@users.noreply.github.com>
- For PG 17+: use --filter parameter with temp file - For PG 13-16: convert filters to legacy arguments (--exclude-table, --exclude-table-data, --exclude-schema) - Added version compatibility notice in UI help text - Only exclude filters supported in legacy mode (include is default behavior) - Filter types without legacy equivalents are not supported in PG < 17 Co-authored-by: gautamsi <5769869+gautamsi@users.noreply.github.com>
- Added support for include table, schema, and extension filters using --table, --schema, and --extension arguments - Updated help text to reflect full legacy mode support - Both include and exclude filters now work in PostgreSQL 13-16 - Only foreign_data, table_and_children, and table_data_and_children remain unsupported in legacy mode Co-authored-by: gautamsi <5769869+gautamsi@users.noreply.github.com>
📝 WalkthroughWalkthroughThis PR adds a Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant WebUI as Web UI
participant Server as Go Server
participant Database as Database
participant PGDump as pg_dump
participant Temp as Temp File
User->>WebUI: Create/Edit backup with filter
WebUI->>Server: POST backup (includes filter_content)
Server->>Database: INSERT/UPDATE backup (filter_content stored)
Database-->>Server: ACK
Note over Server,PGDump: Execution time
Server->>Server: Check PostgreSQL version
alt PostgreSQL ≥ 17
Server->>Temp: Write filter_content to temp file
Server->>PGDump: Run pg_dump --filter <temp-file>
PGDump-->>Temp: Read filter rules
else PostgreSQL < 17
Server->>Server: Convert filter_content to legacy args
Server->>PGDump: Run pg_dump with legacy --table/--schema/--exclude flags
end
PGDump-->>Server: Return backup data
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@internal/view/web/dashboard/backups/create_backup.go`:
- Around line 328-381: The hidden field can get out of sync because
convertToText, convertToGuided, and removeRow modify state but never update the
hidden input; after each state-mutating method (convertToText, convertToGuided,
removeRow—and also addRow if you want immediate sync when adding a blank row)
call this.syncToHidden() so the DOM-hidden value [name=filter_content] is
updated whenever filterRows or textFilter or filterMode change; update the end
of those functions to invoke syncToHidden() (keeping the existing logic intact).
In `@internal/view/web/dashboard/backups/edit_backup.go`:
- Around line 66-68: The FilterContent field is being constructed as
sql.NullString{String: formData.FilterContent, Valid: formData.FilterContent !=
""} which sets Valid=false for an empty string and lets the DB COALESCE keep the
old value, preventing users from clearing the filter; change construction in the
struct (where FilterContent is set) to always set Valid:true (e.g.,
sql.NullString{String: formData.FilterContent, Valid: true}) so an empty string
overwrites the existing value and users can clear their filter, leaving
OptNoComments handling unchanged.
- Around line 430-435: The hidden input named "filter_content" is not
initialized so existing filters can be lost on submit; fix by setting its
initial value from the current filter state (e.g., supply the existing filter
JSON/string to nodx.Input as the value attribute for the input named
"filter_content") or alternatively ensure syncToHidden() runs on component
initialization (add an x-init that calls syncToHidden() on the parent element
that defines the filter UI). Update the nodx.Input call for the "filter_content"
hidden field or add x-init to the filter container so the hidden value is
populated before any user interaction.
🧹 Nitpick comments (3)
prebuild-simple.mjs (1)
8-48: Make output deterministic by sorting SQL files.
Filesystem traversal order can vary, which makesqueries.gen.sqlunstable across runs. A simple sort keeps builds reproducible.♻️ Proposed fix
- const files = findSqlFiles(serviceDir); + const files = findSqlFiles(serviceDir); + files.sort();prebuild.mjs (1)
27-39: Sort the SQL file list for deterministic output.
Without an explicit sort, file order can differ between environments and change the generated file content.♻️ Proposed fix
for (const sourceGlob of sourceGlobs) { const foundFiles = await glob(path.join(rootDir, sourceGlob)); files.push(...foundFiles); } + files.sort();internal/integration/postgres/postgres.go (1)
237-264: String-based version comparison is fragile.Line 239 uses string comparison (
version.Value.Version >= "17"), which works for current versions ("13"-"18") but could break with future double-digit versions or different version formats.Consider using a numeric comparison or comparing against the enum directly:
♻️ Suggested approach
- if version.Value.Version >= "17" { + // PG17+ supports --filter parameter + if version == PG17 || version == PG18 {Or add a helper method to check version capability.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@internal/view/web/dashboard/backups/edit_backup.go`:
- Around line 328-340: The syncToHidden function and the component x-init use
document.querySelector('[name=filter_content]') which always selects the first
matching element and can overwrite other modals; update syncToHidden (and any
x-init lookups) to use the component-scoped DOM via Alpine's $el (e.g., use
$el.querySelector('[name=filter_content]') or this.$el.querySelector when inside
the Alpine component) so each modal updates its own hidden input, and ensure you
null-check the result before setting .value to avoid runtime errors.
🧹 Nitpick comments (1)
internal/view/web/dashboard/backups/create_backup.go (1)
373-385: Scope the hidden-input lookup to the component or use Alpine'sx-refpattern.
document.querySelectorgrabs the first matching element globally. If multiple filter editors coexist, updates can target the wrong hidden field. You can scope the query usingthis.$el.querySelector('[name=filter_content]'), but Alpine's recommended approach is to usex-refwith$refs:<input x-ref="filterContent" name="filter_content" type="hidden">Then update the content via
this.$refs.filterContent.value = content;instead.🔧 Option 1: Scoped querySelector
- document.querySelector('[name=filter_content]').value = content; + this.$el.querySelector('[name=filter_content]').value = content;
| syncToHidden() { | ||
| let content = ''; | ||
| if (this.filterMode === 'text') { | ||
| content = this.textFilter; | ||
| } else { | ||
| this.filterRows.forEach(row => { | ||
| if (row.pattern) { | ||
| content += row.action + ' ' + row.type + ' ' + row.pattern + '\n'; | ||
| } | ||
| }); | ||
| } | ||
| document.querySelector('[name=filter_content]').value = content; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Alpine.js this.$el x-data methods documentation
💡 Result:
In Alpine, $el (not this.$el) is the documented way to access the current DOM element from within Alpine expressions and component methods.
x-datadefines an Alpine component and provides its reactive state (and methods) to the element and its children. You can define methods directly inside the object. [1]$elis a magic property that returns the current DOM node (the element whose directive/expression is running). Example:@click="$el.innerHTML = 'Hello World!'". [2]- To reuse an
x-datacomponent with methods, define it viaAlpine.data(name, factory), then use it likex-data="dropdown"; methods liketoggle()can usethis.open. [3] - If you need the whole current Alpine scope as an object (for passing into external functions), use
$data. [4]
Docs (official):
x-data: [1]$elmagic: [2]Alpine.data(...): [3]$datamagic: [4]
Sources: [1] (alpinejs.dev) [2] (alpinejs.dev) [3] (alpinejs.dev) [4] (alpinejs.dev)
Citations:
- 1: https://alpinejs.dev/directives/data?utm_source=openai
- 2: https://alpinejs.dev/magics/el?utm_source=openai
- 3: https://alpinejs.dev/globals/alpine-data?utm_source=openai
- 4: https://www.alpinejs.dev/magics/data?utm_source=openai
🏁 Script executed:
# Find Alpine.js files and patterns in the codebase
fd -e '.js' -e '.html' -e '.go' | head -30
# Search for Alpine.js x-data usage patterns
rg "x-data" --type html --type go -A 5 | head -50Repository: eduardolat/pgbackweb
Length of output: 1021
🏁 Script executed:
# Look at the specific file mentioned in the review
find . -name "edit_backup.go" -o -name "*edit_backup*" 2>/dev/nullRepository: eduardolat/pgbackweb
Length of output: 116
🏁 Script executed:
# Read the edit_backup.go file to see the Alpine.js code context
cat -n internal/view/web/dashboard/backups/edit_backup.go | sed -n '310,360p'Repository: eduardolat/pgbackweb
Length of output: 1952
🏁 Script executed:
# Get more context around the file - what's the overall structure?
wc -l internal/view/web/dashboard/backups/edit_backup.goRepository: eduardolat/pgbackweb
Length of output: 118
🏁 Script executed:
# Look for other instances of document.querySelector and syncToHidden patterns
rg "syncToHidden|document\.querySelector" --type go -B 3 -A 3Repository: eduardolat/pgbackweb
Length of output: 11419
🏁 Script executed:
# Search for x-data usage and modal patterns in the codebase
rg "x-data|x-init" --type go -B 2 -A 5 | head -80Repository: eduardolat/pgbackweb
Length of output: 46
Hidden input sync can target the wrong modal.
document.querySelector('[name=filter_content]') will always return the first match in the DOM. With multiple edit modals present, syncToHidden() (and x-init) can overwrite another backup's hidden input, causing silent data loss. Use a component-scoped lookup with the Alpine.js magic property $el instead.
🐛 Proposed fix
- document.querySelector('[name=filter_content]').value = content;
+ $el.querySelector('[name=filter_content]').value = content;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| syncToHidden() { | |
| let content = ''; | |
| if (this.filterMode === 'text') { | |
| content = this.textFilter; | |
| } else { | |
| this.filterRows.forEach(row => { | |
| if (row.pattern) { | |
| content += row.action + ' ' + row.type + ' ' + row.pattern + '\n'; | |
| } | |
| }); | |
| } | |
| document.querySelector('[name=filter_content]').value = content; | |
| } | |
| syncToHidden() { | |
| let content = ''; | |
| if (this.filterMode === 'text') { | |
| content = this.textFilter; | |
| } else { | |
| this.filterRows.forEach(row => { | |
| if (row.pattern) { | |
| content += row.action + ' ' + row.type + ' ' + row.pattern + '\n'; | |
| } | |
| }); | |
| } | |
| $el.querySelector('[name=filter_content]').value = content; | |
| } |
🤖 Prompt for AI Agents
In `@internal/view/web/dashboard/backups/edit_backup.go` around lines 328 - 340,
The syncToHidden function and the component x-init use
document.querySelector('[name=filter_content]') which always selects the first
matching element and can overwrite other modals; update syncToHidden (and any
x-init lookups) to use the component-scoped DOM via Alpine's $el (e.g., use
$el.querySelector('[name=filter_content]') or this.$el.querySelector when inside
the Alpine component) so each modal updates its own hidden input, and ensure you
null-check the result before setting .value to avoid runtime errors.
I needed a way to exclude table data for several large tables to make use of nightly export for developer use.
decided to add that feature, then I saw --filter option in latest postgres which makes use of a filter file containing multiple include exclude things
This PR adds support for --filter parameter in postgres 17+
Later I updated this to translate the value from this config to legacy --table, --exclude-table etc (only for table schema and table_data) for lower postgres version.
this is how it looks on backup config
there is another modal for help on this filter option
There is also a guided mode on this which allows you to select specific options and minimize human errors
I have tested that manually in local dev container with postgres 15 and 17
Summary by CodeRabbit
New Features
Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.