Horton
Menu

Re: pg_plan_advice

Robert Haas <robertmhaas@gmail.com>

View thread
From: Robert Haas <robertmhaas@gmail.com>
To: Lukas Fittl <lukas@fittl.com>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>, Tom Lane <tgl@sss.pgh.pa.us>
Date: 2026-03-19T16:54:43Z
Lists: pgsql-hackers 
On Wed, Mar 18, 2026 at 3:00 PM Lukas Fittl <lukas@fittl.com> wrote:
> I think we have similar problems elsewhere in Postgres where a large
> user input causes an even larger log output - e.g. a case I'm familiar
> with are complicated queries with long IN list inputs and their
> associated EXPLAIN plans being logged by auto_explain - I recently had
> a case where someone reported an OOM due to auto_explain trying to log
> a > 100 MB sized query plan.
>
> I think its actually less a problem with plan advice, since presumably
> we won't have ORMs generating plan advice, and even if we do it'd be
> per-table - so I think its unlikely a genuine use case would use 1000s
> of advice tags.
>
> That said, I also don't think super long long messages are actually
> helpful. I do wonder if we should have a more coarse GUC that limits
> DETAIL lines of any kind to a maximum length (e.g. 100 kB) across the
> board instead of special casing every emitter.

I think it would be difficult for generic code to do something
sensible when the message is really long. I mean, it could just cut it
off after N lines, but then you have no idea how much more there would
have been, and you probably want to tell the user something about
that. You could add a completely generic message to the end like "plus
10525 more lines of output that were truncated for display," but
that's pretty unsatisfying. If you want to show something contextually
appropriate, the implementation needs to be separate for each case
even if the limit is common. Anyway, the question here is not about
such a generic mechanism, but about whether somebody wants to argue
for sticking a limit of 100 on feedback messages on the theory that
log spam is bad, or whether it's fine as-is either because (a) the
likelihood of a significant number of people hitting that limit is
thought to be too low to worry about or (b) the likelihood of someone
wanting all of those messages (e.g. for machine-parsing) is thought to
be high enough that a limit is actually worse than no limit. I do not
really have a horse in the race, so if nobody else has a strong
opinion, I'm going to leave it alone for now and consider changing it
if a strong opinion materializes later.

> 1) What if we return the utilized advice string as a separate DefElem
> with a list of strings, and then the feedback just has to reference an
> index into that list? (though I suppose that doesn't actually save
> memory, now that I think that through -- unless we assume the caller
> already has the advice string, but I don't think we can rely on that)

I think that if we're using Integer nodes, it's just never going to be
very economical. We could use an IntList, which I believe would be
better, but there are a few complications that make me not like this
idea very much. One, what the feedback is actually about is a
reconstruction of a particular advice item into string form, not the
original string, e.g. if you input SEQ_SCAN(foo bar), the feedback
will be on SEQ_SCAN(foo) and SEQ_SCAN(bar). Two, even if you ignore
that, this would leave consumers of the data with the problem of
finding the end of the advice item unless you stored both starting and
ending indexes, which would have its own costs.

> 2) We could consider having separate DefElems for the different flag
> types (i.e. "feedback_failed", "feedback_match_full", etc), and then a
> list of strings attached to each - that'd save the nested DefElem and
> the Integer node

But it would also very often duplicate a bunch of the strings, which
seems likely to work out to a loss more often than not. You could
avoid that by have a list of strings per unique flag combination, but
that would be extra work to compute and I think it would be less
convenient for consumers. One user-visible consequence would be that
the advice feedback in EXPLAIN output would have much less to do with
the original order of the advice string.

-- 
Robert Haas
EDB: http://www.enterprisedb.com

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. pg_plan_advice: Fix another unique-semijoin bug.

  2. pg_plan_advice: Export feedback-related definitions.

  3. pg_plan_advice: Fix a bug when a subquery is pruned away entirely.

  4. pg_plan_advice: Add alternatives test to Makefile.

  5. pg_plan_advice: Handle non-repeatable TABLESAMPLE scans.

  6. pg_stash_advice: Allow stashed advice to be persisted to disk.

  7. Add pg_stash_advice contrib module.

  8. pg_plan_advice: Avoid assertion failure with partitionwise aggregate.

  9. pg_plan_advice: Invent DO_NOT_SCAN(relation_identifier).

  10. Add an alternative_plan_name field to PlannerInfo.

  11. pg_plan_advice: Refactor to invent pgpa_planner_info

  12. Respect disabled_nodes in fix_alternative_subplan.

  13. get_memoize_path: Don't exit quickly when PGS_NESTLOOP_PLAIN is unset.

  14. test_plan_advice: Set TAP test priority 50 in meson.build.

  15. pg_plan_advice: Avoid a crash under GEQO.

  16. Test pg_plan_advice using a new test_plan_advice module.

  17. pg_plan_advice: Always install pg_plan_advice.h, and in the right place

  18. pg_plan_advice: Fix failures to accept identifier keywords.

  19. Add pg_plan_advice contrib module.

  20. Allow extensions to mark an individual index as disabled.

  21. Replace get_relation_info_hook with build_simple_rel_hook.

  22. Store information about Append node consolidation in the final plan.

  23. Store information about elided nodes in the final plan.

  24. Store information about range-table flattening in the final plan.

  25. Pass cursorOptions to planner_setup_hook.

  26. Fix PGS_CONSIDER_NONPARTIAL interaction with Materialize nodes.

  27. Fix mistakes in commit 4020b370f214315b8c10430301898ac21658143f

  28. Allow for plugin control over path generation strategies.

  29. Update some comments for fasthash

  30. Allow passing a pointer to GetNamedDSMSegment()'s init callback.

  31. Don't reset the pathlist of partitioned joinrels.

  32. Treat number of disabled nodes in a path as a separate cost metric.