Add PostgreSQL 18 options to the Backup and Restore dialogs - #10060
Open
dpage wants to merge 2 commits into
Open
Add PostgreSQL 18 options to the Backup and Restore dialogs#10060dpage wants to merge 2 commits into
dpage wants to merge 2 commits into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (11)
🚧 Files skipped from review as they are similar to previous changes (7)
Included review availability: Your plan includes up to 8 reviews per rolling hour; 1 remains after this review. WalkthroughPostgreSQL 18 backup and restore options are added to pgAdmin. The UI applies version gating and option exclusions. Backend commands emit supported flags. Unit tests cover PostgreSQL 17 and 18 behavior. Dialog documentation describes the new options. ChangesPostgreSQL 18 Backup Options
PostgreSQL 18 Restore Options
Dialog Documentation
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to This PR adds PostgreSQL 18 backup and restore options behind version checks, with corresponding tests and documentation; no actionable merge-blocking risk remains after normal checks and review. Sequence Diagram(s)sequenceDiagram
participant User
participant BackupUI as Backup UI schema
participant BackupBackend as Backup backend
participant PgDump as pg_dump
User->>BackupUI: Select PostgreSQL 18 backup options
BackupUI->>BackupUI: Disable conflicting options
User->>BackupBackend: Submit backup job
BackupBackend->>BackupBackend: Check server version
BackupBackend->>PgDump: Pass supported flags
sequenceDiagram
participant User
participant RestoreUI as Restore UI schema
participant RestoreBackend as Restore backend
participant PgRestore as pg_restore
User->>RestoreUI: Select PostgreSQL 18 restore options
RestoreUI->>RestoreUI: Disable conflicting options
User->>RestoreBackend: Submit restore job
RestoreBackend->>RestoreBackend: Check server version
RestoreBackend->>PgRestore: Pass supported flags
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
web/pgadmin/tools/backup/__init__.py (1)
288-299:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRoot cause: backend does not enforce one-of semantics for "Type of objects" flags.
Both arg builders can emit contradictory combinations (e.g.,
--data-onlywith--statistics-only) when payloads bypass UI constraints. Enforce exclusivity in backend command construction to keep API behavior valid and deterministic.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/pgadmin/tools/backup/__init__.py` around lines 288 - 299, The backend currently may emit conflicting "only_*" flags; before calling set_param compute mutually exclusive choices for only_data, only_schema, only_tablespaces, only_roles, only_statistics (e.g., read raw booleans from data and manager.version, then enforce one-of semantics by selecting a single true flag according to a deterministic precedence or by refusing others), then pass those resolved booleans to set_param; update the logic around the existing set_param calls for only_data, only_schema, only_tablespaces, only_roles, and only_statistics (and keep the existing manager.version check for statistics) so only one of these flags is ever set in the constructed command.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@web/pgadmin/tools/backup/static/js/backup.ui.js`:
- Around line 166-182: The new switch with id 'only_statistics' was added but
sections that represent dump/restore sections were not wired to it, allowing
conflicting flags; update any section control definitions (the objects that have
properties 'deps' and 'disabled'—e.g., the section controls grouping under 'Type
of objects' that currently list deps like
['pre_data','data','post_data','only_data','only_schema','only_tablespaces','only_roles'])
to include 'only_statistics' in their deps arrays and add the corresponding
check in their disabled predicate (i.e., include state.only_statistics in the OR
chain used in disabled functions); make the same symmetric change in the restore
schema file where the section controls are defined so both backup and restore
honor the only_statistics switch.
---
Outside diff comments:
In `@web/pgadmin/tools/backup/__init__.py`:
- Around line 288-299: The backend currently may emit conflicting "only_*"
flags; before calling set_param compute mutually exclusive choices for
only_data, only_schema, only_tablespaces, only_roles, only_statistics (e.g.,
read raw booleans from data and manager.version, then enforce one-of semantics
by selecting a single true flag according to a deterministic precedence or by
refusing others), then pass those resolved booleans to set_param; update the
logic around the existing set_param calls for only_data, only_schema,
only_tablespaces, only_roles, and only_statistics (and keep the existing
manager.version check for statistics) so only one of these flags is ever set in
the constructed command.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 15e3abc2-446e-4e9c-bb62-b930f0b8cf5f
📒 Files selected for processing (10)
docs/en_US/backup_dialog.rstdocs/en_US/backup_server_dialog.rstdocs/en_US/release_notes_9_16.rstdocs/en_US/restore_dialog.rstweb/pgadmin/tools/backup/__init__.pyweb/pgadmin/tools/backup/static/js/backup.ui.jsweb/pgadmin/tools/backup/tests/test_backup_create_job_unit_test.pyweb/pgadmin/tools/restore/__init__.pyweb/pgadmin/tools/restore/static/js/restore.ui.jsweb/pgadmin/tools/restore/tests/test_restore_create_job_unit_test.py
There was a problem hiding this comment.
Pull request overview
Adds PostgreSQL 18 client-utility flags to pgAdmin’s Backup / Backup Server / Restore dialogs, updates backend argument builders to emit the new flags only for server versions >= 18, and extends documentation and unit tests accordingly.
Changes:
- Added new PG18 switches to Backup and Restore UI schemas (Only statistics; do-not-save/do-not-restore flags; misc flags).
- Updated backup/restore argument builders to emit the new flags when
manager.version >= 180000. - Updated docs and release notes; expanded unit tests for new options.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| web/pgadmin/tools/restore/tests/test_restore_create_job_unit_test.py | Adds a PG18 restore scenario asserting new flags are emitted. |
| web/pgadmin/tools/restore/static/js/restore.ui.js | Adds PG18 restore UI switches (Only statistics; no-policies/no-data/no-schema/no-statistics). |
| web/pgadmin/tools/restore/init.py | Adds PG18 restore arg-builder flags guarded by server version. |
| web/pgadmin/tools/backup/tests/test_backup_create_job_unit_test.py | Adds PG18 backup scenarios (emit flags for >=18, guard scenario for <18). |
| web/pgadmin/tools/backup/static/js/backup.ui.js | Adds PG18 backup UI switches (Only statistics; no-policies/no-data/no-schema; statistics/sequence-data). |
| web/pgadmin/tools/backup/init.py | Adds PG18 backup arg-builder flags guarded by server version. |
| docs/en_US/restore_dialog.rst | Documents new PG18 restore options. |
| docs/en_US/release_notes_9_16.rst | Adds release note entries for issues #9064/#9065/#9066. |
| docs/en_US/backup_server_dialog.rst | Documents new PG18 backup-server options. |
| docs/en_US/backup_dialog.rst | Documents new PG18 backup options. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+297
to
+298
| set_param('only_statistics', '--statistics-only', | ||
| manager.version >= 180000) |
Comment on lines
+411
to
+415
| ('When restore object with option - Do not restore Row security ' | ||
| 'policies, Data, Schema, Statistics and Only statistics (>= v18)', | ||
| dict( | ||
| class_params=dict( | ||
| sid=1, |
asheshv
requested changes
Jun 12, 2026
asheshv
left a comment
asheshv
left a comment
Contributor
There was a problem hiding this comment.
Three issues:
--no-statisticsis added to restore but missing from backup. The PG18 release notes introduce--no-statisticsforpg_dump/pg_dumpallalongside--statistics/--statistics-only. Restore exposes "Do not restore statistics" but backup has no matching "Do not dump statistics" — inconsistent and a feature gap. Addset_param('no_statistics', '--no-statistics', manager.version >= 180000)plus the corresponding UI field withmin_version: 180000.- Backend mutex missing for
only_statistics. The existingonly_data/only_schemapair has backend mutual-exclusion (e.g.only_schemaskipped ifonly_datais set), butonly_statisticshas no such guard. A direct POST with{"only_data": true, "only_statistics": true}will emit both flags andpg_dumpwill reject. UI handles it viadisabledfunctions, but the backend pattern is inconsistent. Sameassertionpattern asonly_schemais the fix. - "Unit" tests aren't unit tests.
test_backup_create_job_unit_test.pyusesserver_utils.connect_server()and a realself.server_information['server_version'], so PG18 scenarios are silently skipped on any CI without a live PG18 instance. The version-gate logic is never exercised by a mock. Worth adding a trueMagicMocktest withmanager.version = 170000asserting the PG18 flags are absent — applies to all six new options.
Confirmed correct: all six flags are genuinely new in PG18, version gate at 180000 is right, args passed as a list (no shell injection surface), gettext() wrapping present.
dpage
force-pushed
the
feature/backup-restore-pg18-options
branch
from
August 17, 2026 12:37
a8551a8 to
f3a320d
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
Adds the new pg_dump/pg_dumpall/pg_restore options introduced in PostgreSQL 18, gated to servers >= 18: - Only statistics (--statistics-only) in Type of objects, mutually exclusive with the other Only * switches. - Row security policies (--no-policies), Data (--no-data) and Schema (--no-schema) in Do not save (both dialogs); Statistics (--no-statistics) in the Restore dialog. - Dump statistics (--statistics) and Sequence data (--sequence-data) in Backup Miscellaneous. The pg_dumpall -F/--format option and the pg_restore --exclude-database / -g/--globals-only options are not included: the non-text pg_dumpall feature was reverted before PostgreSQL 18.0 and those options do not exist in the shipped 18.x client utilities. Includes backend arg-builder wiring, unit-test scenarios and docs. Closes pgadmin-org#9064 Closes pgadmin-org#9065 Closes pgadmin-org#9066
pg_dump and pg_dumpall both accept --no-statistics, but only the Restore dialog offered it, so the Backup dialogs could dump statistics or nothing but statistics whilst having no way to say "everything except statistics". Backup now has the matching Statistics switch under Do not save, documented alongside the others. The larger problem is that several of these options cannot be combined, which I checked against the shipped utilities rather than inferring: pg_dump 18 reports "options -s/--schema-only and --statistics-only cannot be used together", and likewise for --data-only with --statistics-only, --statistics-only with --no-statistics, --schema-only with --no-schema and --data-only with --no-data. pg_restore and pg_dumpall reject the same pairs. The dialog disabled some of those combinations and not others, and nothing stopped a request arriving with both halves set, in which case the utility rejected the whole command. The backend now suppresses the narrower option in each pair, following the pattern --schema-only already used, and the switches under Do not save follow the state of their matching Only switch in both dialogs. The tests for the v18 options previously only ran where a v18 server was available, and silently skipped otherwise, so the new test_*_pg18_args_unit_test.py files call the argument builders directly with a mocked manager version. They cover the options being emitted on 18, suppressed on 17, and each conflicting pair resolving to the wider option. One existing scenario asserted a command pg_restore would have rejected, setting both --statistics-only and --no-statistics, so it is now split into two.
dpage
force-pushed
the
feature/backup-restore-pg18-options
branch
from
August 17, 2026 14:54
f3a320d to
a891032
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds the new
pg_dump/pg_dumpall/pg_restoreoptions introduced inPostgreSQL 18 to the Backup, Backup Server and Restore dialogs.
All new options are gated to servers
>= 18(min_version: 180000in the UIand a
manager.version >= 180000check in the arg builders).Backup (
pg_dumpfor object backups,pg_dumpallfor server backups)--statistics-only) — Type of objects, mutuallyexclusive with the other Only … switches.
--no-policies), Data (--no-data),Schema (
--no-schema) — Do not save.--statistics) and Sequence data(
--sequence-data) — Miscellaneous.Restore (
pg_restore)--statistics-only) — Type of objects, mutuallyexclusive with the other Only … switches.
--no-policies), Data (--no-data),Schema (
--no-schema), Statistics (--no-statistics) — Do not save.Note on options that are not included
The
pg_dumpall -F/--formatoption (requested in #9065) and thepg_restore--exclude-database/-g/--globals-onlyoptions (requested in #9066) arenot added: the non-text
pg_dumpallfeature was reverted beforePostgreSQL 18.0, and those options do not exist in the shipped 18.x client
utilities (verified against
pg_dumpall --help/pg_restore --helpon18.4). The remaining, real PG 18 options from those issues are included.
Testing
flags are emitted at server version
>= 18and a guard scenario assertsthey are absent below 18.
pycodestyleandeslintclean; verified each flag (and combinations)against the bundled PostgreSQL 18.4 utilities.
Closes #9064
Closes #9065
Closes #9066
Summary by CodeRabbit
New Features
Documentation
Tests