Repository navigation
fix(ivory): honor ordinary per-database SQL targets - #89
Hanabi9248 wants to merge 1 commit into
Conversation
Keep the maintenance catalog connection explicit while allowing PGDATABASE in the ordinary target loop to select each enumerated database. Preserve the legacy monitoring script and cover its real caller command contract. Signed-off-by: Hanabi <317387557+Hanabi9248@users.noreply.github.com>
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Target command and regression coverage internal/ivory/exec.go, internal/ivory/exec_database_target_test.go, internal/ivory/exec_test.go |
The standard target-query command uses PGDATABASE without a database override argument. New and updated tests check the target and catalog command arguments, and verify legacy monitoring command compatibility. |
Priority:
Estimated code review effort: 2 (Simple) | ~10 minutes
Change: Bug fix
Suggested reviewers: balinorliang
Merge Risk: 
b5b4e
-
No concrete merge-blocking issue remains. Named database targets now use PGDATABASE; catalog and monitoring behavior remains unchanged. Full integration validation remains outstanding.
Pre-merge checks: 
-
Check name Status Explanation Resolution Docstring Coverage Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 3 files. Write docstrings for the functions missing them to satisfy the coverage threshold. -
Check name Status Explanation Title check The title clearly and concisely identifies the main fix: ordinary per-database SQL targets now honor their database selection. Description check The description explains the current and new behavior, identifies the bug, documents the scope, lists automated validation, records test limitations, and categorizes the change as a bug fix and testin… Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Full details: Description check
-
Explanation
The description explains the current and new behavior, identifies the bug, documents the scope, lists automated validation, records test limitations, and categorizes the change as a bug fix and testing enhancement.
-
-
Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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 help to get the list of available commands.
PostGIS and pgAudit call
ExecInAllDatabasesto install their extensions in every connectable database. The ordinary loop enumerates those databases and setsPGDATABASE, but its target command also specifiespsql -d postgres. That explicit database argument overrides the per-iteration environment value, so the target SQL is repeatedly directed to the maintenance database.Remove the overriding argument from the ordinary target loop. Keep the catalog query and
Executor.Execexplicitly connected to postgres. KeepPGDATABASEfor literal database names, including names containing spaces or looking like connection strings. The special monitoring/PgBouncer script remains byte-for-byte unchanged: the existing pgMonitor caller supplies\c postgres, which is a different input contract. Variable forwarding and other executor behavior are outside this change.Validation on base
0240ce2a2c451164c666fb769b6e837ee244175f:exec.gocompiled in Go named-file mode with the added regression. The real generated Bash script runs with a clearly scoped psql command fixture that supplies catalog output and observes arguments, stdin and PGDATABASE. It does not emulate libpq or a database.go vet exec.go exec_database_target_test.gopasses. Two existing ordinary-script expectations are updated../internal/ivorytesting could not start because pinned dependencies are absent from the existing offline cache (GOPROXY=off). Full package/repository tests, ShellCheck, actual psql/libpq, IvorySQL image compatibility, extensions and Kubernetes integration were not run.An initial two-loop candidate was rejected during caller review and is not included. An initial Windows test attempt lacked a usable /dev/stdin pipe; the fixture now provides the same SQL through a real temporary input file. Final baseline and candidate above were both rerun after narrowing the production change.
The client option/environment distinction is documented in psql connection parameters and agrees with the current IvorySQL client source.
Checklist
Type of changes
Summary by CodeRabbit
postgres.postgres, preserving their existing behavior.