refactor: first pass at adding pg commands from pg-extras plugin to cli PR 1 of 3#3770
Conversation
Add postgres words to cspell dictionary
…without approval from UX.
eablack
left a comment
There was a problem hiding this comment.
Migration looks faithful to heroku-pg-extras — SQL bodies for all six commands match verbatim, and the pg_stat_statements schema check correctly expanded from 'public' to ('public', 'heroku_ext') to match pg/outliers.ts. Approving, with three notes worth addressing here or in a quick follow-up.
1. ux.error is swallowed in src/lib/pg/extras.ts (regression hidden by tests)
The three helpers wrap ux.error(..., {exit: 1}) in try/catch. ux.error throws an oclif ExitError, which the catch then catches and replaces with a generic message. Net effect: a user whose DB is missing pg_stat_statements never sees the install instructions — they see "Failed to check pg_stat_statements extension availability". The pg:calls test at test/unit/commands/pg/calls.unit.test.ts even encodes this:
// ensurePGStatStatement raises the "need to be installed" error inside its own
// try/catch, which then re-raises the generic availability-check failure.
execQueryStub.onCall(0).resolves('f')
// ...
expect(error?.message).to.contain('Failed to check pg_stat_statements extension availability')The simplest fix is to drop the try/catch entirely (this matches pg/outliers.ts, which lets the error propagate with the right message). Same applies to newTotalExecTimeField and newBlkTimeFields. Tests asserting the generic string would need to be updated to assert the real one — which is the user-visible improvement.
2. database arg description drifts from existing pg commands
The new commands hardcode description: 'database name'. Existing commands in this directory use the localized form:
// src/commands/pg/bloat.ts, src/commands/pg/outliers.ts
database: Args.string({
description: `${nls('pg:database:arg:description')} ${nls('pg:database:arg:description:default:suffix')}`,
}),Worth aligning so --help output stays consistent across pg:*.
3. SQL/identifier injection on args.prefix in pg:fdwsql
Pre-existing in heroku-pg-extras, but worth fixing while we're here. The user-supplied prefix is interpolated into SQL outside any quote_ident:
ux.stdout(`DROP SERVER IF EXISTS ${args.prefix}_db;`)
ux.stdout(`CREATE SERVER ${args.prefix}_db ...`)
// and inside generateFdwsqlQuery:
// || ' SERVER ${prefix}_db OPTIONS'A prefix containing ', ;, or whitespace produces malformed (or hostile) SQL. Risk is low because the user is operating on their own DB and pasting the output themselves, but a simple identifier-shape validation would close it: e.g. if (!/^[A-Za-z_][A-Za-z0-9_]*$/.test(args.prefix)) ux.error(...).
|
Thank you @eablack all notes addressed. |
Summary
We need to move the commands from the pg-extras plugin to the core CLI, which will require us to update the commands to use oclif/core v4 and heroku-cli-command v12.
This is PR 1 of 3 to accomplish this task and we will be merging these PRs in feature/migrate_pg_extras_to_cli
Things done:
This work item covers migrating the following 6 pg-extras commands to the core CLI:
ALSO did:
What I'm not doing and why
Type of Change
Breaking Changes (major semver update)
!after your change type to denote a change that breaks current behaviorFeature Additions (minor semver update)
Patch Updates (patch semver update)
Testing
Notes:
The --help changes (should be limited to --prompt and GLOBAL FLAGS additions that came with cli upgrades) ALSO examples as the cli requires:
diff <(./bin/run pg:cache-hit --help) <(heroku pg:cache-hit --help)
diff <(./bin/run pg:calls --help) <(heroku pg:calls --help)
diff <(./bin/run pg:extensions --help) <(heroku pg:extensions --help)
diff <(./bin/run pg:fdwsql --help) <(heroku pg:fdwsql --help)
diff <(./bin/run pg:index-size --help) <(heroku pg:index-size --help)
diff <(./bin/run pg:index-usage --help) <(heroku pg:index-usage --help)
New Command Testing Steps:
ratio)
./bin/run pg:cache-hit -a $APP
./bin/run pg:cache_hit -a $APP
descending; tables with no scans show Insufficient data)
./bin/run pg:index-usage -a $APP
./bin/run pg:index_usage -a $APP
./bin/run pg:index-size -a $APP
./bin/run pg:index_size -a $APP
plpgsql row)
./bin/run pg:extensions -a $APP
./bin/run pg:calls -a $APP
./bin/run pg:calls --truncate -a $APP
./bin/run pg:fdwsql example_prefix -a $APP
./bin/run pg:cache-hit DATABASE_URL -a $APP
./bin/run pg:fdwsql example_prefix DATABASE_URL -a $APP
Screenshots (if applicable)
Related Issues
GUS work item: https://gus.lightning.force.com/lightning/r/ADM_Work__c/a07EE00002Yzk1lYAB/view