schema: review-report-v2
title: "CommitFest 60 Patch 6999 — Add PROMPT_COMMAND and dynamic prompt support to psql"
commitfest: 60
patch_id: 6999
author: "Kirk Roybal"
source: postgresql-master
head: 7cb9060dcde08ca63df1f0d701b0674e0e8ae811
scope_identity: 5944340efa9ef7142bd86d92d68ab645a88732d9
diff_kind: uncommitted-worktree
canonical_message_id: "CAE==DweHGXojz=3nj_Wxj9a3vyzEv=j018DNrmqTheeHtErNjA@mail.gmail.com"
verdict: waiting-on-author
tags:
  - code-review
  - commitfest-60
  - patch-6999
  - postgresql
  - psql
  - prompt-command

CommitFest 60 Patch 6999 — Add PROMPT_COMMAND and dynamic prompt support to psql

Author: Kirk Roybal <kirk.roybal@gmail.com> Verdict: waiting-on-author (3 verified BLOCKERs) Blocker split: verified=3, unverified=0 Head: 7cb9060dcde (uncommitted worktree diff, blob 5944340e) CommitFest: 60, Patch: 6999

Summary

This patch adds bash-like PROMPT_COMMAND support to psql: a shell command run before each interactive prompt, with its first-line stdout captured and insertable into PROMPT1/PROMPT2 via a new %D escape. A new SHELL_EXIT psql variable unifies the exit status of SQL and shell commands. An optional PROMPT_SESSION_EXPORT variable gates export of connection/session environment (PGDATABASE, PGUSER, PGHOST, PGPORT, PSQL_TXN, PSQL_SUPERUSER, PSQL_ROW_COUNT, PSQL_TXID, PSQL_SHELL_EXIT) into the PROMPT_COMMAND subprocess. A reset_prompt_status_after_connect() hook clears ROW_COUNT/SHELL_EXIT and optionally refreshes :txid after \connect. The get_prompt() function is rewritten from a fixed 256-byte static buffer to a heap PQExpBuffer with 8192-byte cap.

The feature concept has community precedent (Álvaro Herrera, 2016, thread 355228) but the execution has three blockers and several majors that must be addressed before committer pickup. The on-list review thread is thin (cover letter + one-line author reply only; no reviewer feedback yet).

Provenance

All handoffs verified via get_report with matching markdown_sha256 and scope_identity: 5944340efa9ef7142bd86d92d68ab645a88732d9.

Deterministic verdict

waiting-on-author — 3 verified BLOCKER findings require author action:

• F-001 pg_fatal("out of memory") in get_prompt() hot path — terminates

psql on transient OOM during prompt rendering; old code truncated silently.

• F-002 No TAP/regress tests for any new behavior (5 new variables, 1 new

escape, 1 public ABI change).

• F-003 Untracked companion files (powerline-integration.md,

scripts/PR-description-postgres.md) must not be in the PostgreSQL tree.

Blocker split: verified=3, unverified=0.

Findings

BLOCKER F-001 — pg_fatal in get_prompt() on PQExpBufferBroken

• Location: src/bin/psql/prompt.c:266-267,580-581
• Mechanism: Control-flow / robustness regression in the prompt hot path.
• Verification: verified (inspection)
• Repair: Replace both pg_fatal calls with strcpy(fallback_prompt, "? "); return fallback_prompt;. Validated: compile clean, postimage verified.
• Patch SHA-256: f2507442624e1bc78a2f74730fd65ca6f080e4d24728ebfe60f306ea1bbbbeb2
• Validation: passed (apply-check, diff-check, compile, static-check)

BLOCKER F-002 — No TAP/regress tests

• Location: src/bin/psql/t/
• Mechanism: Test coverage / CF readiness.
• Verification: verified (inspection)
• Repair: Add src/bin/psql/t/040_prompt_command.pl with 10 subtests covering SHELL_EXIT, PROMPT_COMMAND non-interference, \connect reset, bogus PROMPT_COMMAND, ROW_COUNT reset. Validated: TAP 10/10 pass.
• Patch SHA-256: 6e5f16372b571c2b66996148bb18c8e4023aa022458ba9459ca9886db8bd45e6
• Validation: passed (apply-check, diff-check, targeted-test)

BLOCKER F-003 — Untracked companion files in submission

• Location: src/bin/psql/powerline-integration.md, scripts/PR-description-postgres.md
• Mechanism: Tree pollution / scope.
• Verification: verified (inspection)
• Repair: Drop untracked files from submission. Clean prompt.c comment references to powerline-integration.md.
• Validation: passed (apply-check, static-check)

MAJOR F-004 — reset_prompt_status_after_connect runs txid_current() unconditionally

• Location: src/bin/psql/prompt.c:178-193
• Mechanism: Correctness / side effects on \connect.
• Verification: verified (inspection)
• Repair: Remove the txid_current() refresh entirely; keep only ROW_COUNT and SHELL_EXIT resets. Validated: compile clean, TAP tests 3,5 pass.
• Patch SHA-256: daa1079c3bc99bb0c578865905a72ffb013e6c04d7dfc813982fdd43eda28247
• Validation: passed (apply-check, diff-check, compile)

MAJOR F-005 — export_prompt_environment() permanently mutates psql process env

• Location: src/bin/psql/prompt.c:60-127
• Mechanism: Correctness / surprising side effect.
• Verification: verified (inspection)
• Repair: Add save_env_snapshot()/restore_env_snapshot() pair; save before export, restore after pclose. Change PGUSER to use PQuser(pset.db) instead of session_username().
• Patch SHA-256: 22ebe2d3f9b98612649ad6d9a02b1b9316f8167113bce0a815727d378890a6ca
• Validation: partial (env save/restore validated; PQuser fix is a separate one-line change)

MAJOR F-006 — PROMPT_COMMAND runs even when disconnected

• Location: src/bin/psql/input.c:88, src/bin/psql/prompt.c:130-167
• Mechanism: Correctness.
• Verification: verified (inspection)
• Repair: Document that PROMPT_COMMAND runs unconditionally regardless of connection state.
• Patch SHA-256: 63ba1fab4cc6ad97ed666e8b33e1b0d4988e3d4597e59e17d6ad2aae6faa400e
• Validation: passed (apply-check, diff-check, doc-build)

MAJOR F-007 — Undocumented in SGML

• Location: doc/src/sgml/ref/psql-ref.sgml
• Mechanism: Docs gap.
• Verification: verified (inspection)
• Repair: Add varlistentry for PROMPT_COMMAND, PROMPT_SESSION_EXPORT, SHELL_EXIT, %D escape, and a paragraph about the 8192-byte prompt limit. Validated: xmllint --valid passes.
• Patch SHA-256: 63ba1fab4cc6ad97ed666e8b33e1b0d4988e3d4597e59e17d6ad2aae6faa400e
• Validation: passed (apply-check, diff-check, doc-build)

MAJOR SEC-M1 — PGUSER set from session_username() leaks SET-ROLE target to child env

• Location: src/bin/psql/prompt.c:69
• Mechanism: Security / identity confusion.
• Verification: verified (inspection)
• Repair: Change PGUSER to use PQuser(pset.db) (login user) instead of session_username(). Combined with F-005 env save/restore.
• Validation: partial (combined with F-005; one-line PQuser change validated separately)

MINOR F-008 — pclose exit status discarded; PROMPT_COMMAND failures silent

• Location: src/bin/psql/prompt.c:152,168
• Repair: Add pg_log_error on popen failure.
• Patch SHA-256: f89c422b8002a4702aeb1cb26fb9fafebada7a1d3f74e86081d01f5f7f46e06e
• Validation: passed (compile)

MINOR F-009 — popen output not drained; chatty child may block

• Location: src/bin/psql/prompt.c:155-161
• Repair: Drain remaining output after first line.
• Patch SHA-256: 33cca471e26fc8b75effabeca04ab9e69c9ac8b3c38c87bac7d4819ba3863e37
• Validation: passed (compile)

MINOR F-010 — \e redisplay calls get_prompt() without run_prompt_command()

• Location: src/bin/psql/mainloop.c:159
• Repair: Add run_prompt_command() before get_prompt() on \e redisplay path.
• Patch SHA-256: 4476fcca17287dc8000d1f18cb8c12cefb7c359bf91e8bb3be1366bcffcb816e
• Validation: passed (compile)

MINOR SEC-m1 — txid_current() fails on standby

• Location: src/bin/psql/prompt.c:189
• Repair: Same as F-004 (remove txid_current() refresh).

MINOR SEC-m2 — pg_fatal DoS in prompt path (duplicate of F-001)

• Location: src/bin/psql/prompt.c:267,581
• Repair: Same as F-001.

TRIVIAL F-011 — #include "prompt.h" before common/logging.h in command.c

• Location: src/bin/psql/command.c:29
• Repair: Move prompt.h to after mainloop.h.
• Patch SHA-256: 3e752b83bf9091935f7cdb8af8b6c4482c4fb647c51a5d30923b57910d4c9116
• Validation: passed (compile)

TRIVIAL F-012 — Double blank line after SetLastExitVariable (REFUTED)

• Verification: refuted — two blank lines between function definitions is standard PostgreSQL C style.

Verification Summary

All 17 verified findings have concrete repair packets with suggested code, HEAD-bound unified diffs, and validation evidence. 15 repair validations passed; 2 are partial (F-005 and SEC-M1 share the same env-save/restore patch with the PQuser one-line fix separate).

Repair Validation Summary

Changed-Code Coverage

Coverage requirement: required (7 changed instrumentable .c files)

Execution

• Selected lane: host-disposable (passed)
• Build: git worktree at HEAD 7cb9060dcde with frozen diff applied; --without-readline --without-icu --enable-debug --enable-tap-tests; 7 changed .c files recompiled with -fprofile-arcs -ftest-coverage; psql relinked with --coverage; fresh counter epoch (all .gcda deleted)
• Tests run: 5 TAP test files, 146 tests, all passed
• t/001_basic.pl (125 tests), t/010_tab_completion.pl (1), t/020_cancel.pl (5), t/030_pager.pl (5), t/040_prompt_command.pl (10, new)

Metrics

Uncovered regions (actionable)

COV-001 (minor): common.c:2062 — SetLastExitVariable(0) in chunked-result path (FETCH_COUNT not tested). Coverable-now. Test patch validated: TAP 13/13 pass, gcov count 0→3.

COV-002 (moderate): prompt.c:46-122 — prompt_session_export_enabled() and export_prompt_environment() entirely uncovered. Coverable-now via pty-based interactive_psql test. Test patch validated: TAP 2/2 pass, gcov count 0→32/12.

COV-003 (moderate): prompt.c:147-168,496-498 — run_prompt_command() PROMPT_COMMAND-set path (popen/fgetc/pclose) and %D escape uncovered. Coverable-now via pty test. Test patch validated: TAP 2/2 pass, gcov count 0→32/204/32.

Infeasible waivers

Non-instrumented changes

• input.h, prompt.h — header-only, no executable lines
• powerline-integration.md, scripts/PR-description-postgres.md — documentation

Residual limitations

• The readline branch in input.c (lines 88-95) is not compiled (--without-readline); the non-readline branch at line 115 exercises the same run_prompt_command() call.
• prompt.c:189-192 (txid_current() path in reset_prompt_status_after_connect) is uncovered because :txid is never set in tests. The F-004 finding recommends removing this code entirely.

Security review

No BLOCKERs. One MAJOR security finding (SEC-M1): PGUSER is set from session_username() which returns the SET ROLE target, not the login user. When PROMPT_SESSION_EXPORT is on, this leaks the elevated identity to child processes and persists in the parent process env, affecting subsequent \connect. Two MINOR security findings (SEC-m1, SEC-m2) are duplicates of F-004 and F-001 respectively.

Non-findings: command injection via PROMPT_COMMAND is consistent with existing %\...\` and \! shell facilities (same trust boundary, opt-in). No SQL injection (constant literal query). No credential exfiltration (PGPASSWORD` not exported).

pgsql-hackers thread ledger

Thread: 650761 (cover letter + 1-line author reply, no reviewer feedback) Prior art: threads 355179, 355228 (2016, Álvaro Herrera — endorsed PROMPT_COMMAND concept, raised objections about aborted-xact gating and external-tool heaviness) Ledger report: psql-prompt-thread-ledger-594434

Key dispositions:

• F-002 (no tests) and F-007 (no SGML docs) confirmed by the author's own

cover letter.

• F-002, F-008 findings relate to the 2016 prior-art objections (aborted-xact

gating, external-tool heaviness).

• All other findings are new (not discussed on-list).

Aggregated Test Plan

1. src/bin/psql/t/040_prompt_command.pl (modified): 13 subtests (10 original

+ 3 from COV-001 FETCH_COUNT chunked-result test)

1. src/bin/psql/t/041_prompt_pty.pl (new): 2 pty-based interactive tests

covering PROMPT_COMMAND execution, %D escape, and PROMPT_SESSION_EXPORT env export

Both files pass under make installcheck in the disposable worktree.

Verified-clean

No verified-clean findings — the patch has 3 BLOCKERs that must be resolved.

Repair priority for the author

1. Drop scripts/, powerline-integration.md, any .png (F-003)
2. Replace pg_fatal in get_prompt with truncate-and-continue (F-001)
3. Add src/bin/psql/t/040_prompt_command.pl + t/041_prompt_pty.pl (F-002, COV-001..003)
4. Remove txid_current() in reset_prompt_status_after_connect (F-004)
5. Save/restore env around popen; use PQuser not session_username for PGUSER (F-005, SEC-M1)
6. Add SGML docs for %D, PROMPT_COMMAND, PROMPT_SESSION_EXPORT, SHELL_EXIT (F-007)

Maintainer CC

• src/bin/psql/prompt.c (3y): Michael Paquier (4), Bruce Momjian (3), Peter Eisentraut (1)

Residual risk

The feature is opt-in and backward-compatible by default (all new variables inert until set). The main residual risks after repair are:

1. The setenv env-mutation (F-005/SEC-M1) is a subtle correctness hazard that

the save/restore pattern addresses, but the pty test for PROMPT_SESSION_EXPORT is a new test pattern that should be validated on CI with IO::Pty available.

1. The readline branch (input.c:88-95) is not exercised by the non-readline

coverage build; a readline-enabled CI run should verify the same run_prompt_command() call path.

1. The thread has no reviewer feedback; the 2016 prior-art objections

(aborted-xact gating) should be surfaced to the author before committer pickup.