An Opus 4.8 review of pg_*_advice commits wrote test cases, attached, for
several malfunctions. The test patch also contains its markdown report. A
lot of its findings are more like long-term bugfix opportunities than open
items, but I recommend treating at least these as an open item batch:
### 1. An empty (non-NULL) stashed advice string persists a TSV line the loader rejects
### 3. pg_start_stash_advice_worker() destroys an existing pg_stash_advice.tsv when persistence was off at boot,
silentlylosing all persisted advice
I wasn't 100% convinced this is unintentional, but Opus is more convinced.
Worth a look from you.
### 14. Stash-supplied advice silently overrides an explicitly-set pg_plan_advice.advice for the same query; the
precedenceis documented nowhere
I do feel the GUC should preempt the stash. Best to solidify this behavior
before release.
Here's the second tier, as I see it:
### 5. pg_plan_advice.advice / stash advice changes are silently ignored by an already-cached generic plan
### 8. pg_set_stashed_advice stores advice with no syntax validation, so malformed advice is accepted then emits a
plan-timeWARNING on every execution and applies nothing
Bounded harm, but changing this would be a relatively-large boost to the user
experience.
