public inbox for [email protected]
help / color / mirror / Atom feedFrom: Lukas Fittl <[email protected]>
To: Robert Haas <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Cc: Tom Lane <[email protected]>
Subject: Re: pg_plan_advice
Date: Wed, 18 Mar 2026 11:59:46 -0700
Message-ID: <CAP53PkxpPsB+xtuC1zKt=6ANw8vHjX_SJe2k=0AguMAqaK67Lg@mail.gmail.com> (raw)
In-Reply-To: <CA+TgmoYRJdDh7MPK-cLs4w4gmYqEaWM3Y0o+2-YAKfJ=5rHNyA@mail.gmail.com>
References: <CA+TgmoZ-Jh1T6QyWoCODMVQdhTUPYkaZjWztzP1En4=ZHoKPzw@mail.gmail.com>
<CAKZiRmxtJAFG7e1+Vs9B8ngON=AOzJbuws+1ZeH4LsbJh5AzoQ@mail.gmail.com>
<CA+TgmoY9Ne_Sh10u6LSPc3wvOQPLp3kF9nBp3nqJEG2JGF2QiA@mail.gmail.com>
<CA+Tgmoa57S6mP=aTOXH2-gDAL4TMO1WbGgrHSg0s6J4zUH=04g@mail.gmail.com>
<[email protected]>
<CA+Tgmoaf__2B0BUL+vrg28P+3buX=Ti-kybqkHiLTtFrrCfzuA@mail.gmail.com>
<CA+TgmoYpcLNOuypOTdgCSLW7FuA=t6BtB3meTARHX2-Dj_81xQ@mail.gmail.com>
<[email protected]>
<CA+TgmoZjv9OyFu1Gkt78w0vWEti8S33w8trYHmErf-GMmGSi=w@mail.gmail.com>
<[email protected]>
<CA+TgmoaOSBQD9Ux4eG40w723ZN=c0J7p-+oX4+J8urUeyLMo5w@mail.gmail.com>
<CAOYmi+=g+MMoOpWkk2weXWKJcKH0eKey8gKHHdH0dF4Tiawrhw@mail.gmail.com>
<CA+TgmobwaT=PXPDDrgDup+jA8KHBbkxigtziD-zNzAKKkQYVgQ@mail.gmail.com>
<CAOYmi+mOmEW=amDRQMfw6-Fb3ZmDEQFaJzwk8Bc8W8DzaP85XQ@mail.gmail.com>
<CA+TgmoaX2AMW4cdFM3OngBJxmxpkdmzF33R7-CWhvRLfucbFMg@mail.gmail.com>
<CAOYmi+k4AyWCQHK=XVF99KVDuFkqxcADao61OWGLxu0nRYMONQ@mail.gmail.com>
<CA+TgmoZ0x3ym_oueXRWzbM_=6ucKoPZVGj3rRMLBDC_FnetXDw@mail.gmail.com>
<CAP53Pkycc=7N2bLzVT3x+qE1JamvRZWev5tFjdLJ1+-AV3Di+Q@mail.gmail.com>
<CA+TgmoaKhuD91RnazbRyGkmP7--JdNq8oNDC3UcgTZSWbMxC7w@mail.gmail.com>
<CAP53Pkw5-wMEeDJXFmqo_RTyL_spzCXb7HHKrbSnQqokVoZcNQ@mail.gmail.com>
<CA+Tgmob-69bzbdi3U_QtebqAf6u1y8js=5=oNK639csVe1VbhA@mail.gmail.com>
<CA+TgmoaZMOikxK=LqS+Jn+835h9S139JLGk-3LyETVXw5W5j=w@mail.gmail.com>
<[email protected]>
<CAP53PkwZ1ZTMARKg6iEfAw9qzBhkjBitj-9gr_Jvy7k2AwGgWA@mail.gmail.com>
<CAMbWs4--NuEUFE_xTo991TRXaZryE29jarJPDnVxoaQOYdt7tA@mail.gmail.com>
<CA+TgmobzR+XMGbRosVPbjHbSo4+cgJn=qZK6w05aF1sbj=C+9Q@mail.gmail.com>
<CA+TgmoawzvCoZAwFS85tE5+c8vBkqgcS8ZstQ_ohjXQ9wGT9sw@mail.gmail.com>
<CA+TgmoYS4ZCVAF2jTce=bMP0Oq_db_srocR4cZyO0OBp9oUoGg@mail.gmail.com>
<CAK98qZ2RzbgCHrSg4zLkvpzyBam_X6te-KF8w1+_vON9BAVMEw@mail.gmail.com>
<CA+TgmoaCdsuvNn6T6SfQ_0YD2Hh2+hgTXh9fTGHQhPg1zvy2rQ@mail.gmail.com>
<CAK98qZ1zWzRB0ABG7ULzTeWKRR5C7-FxLqM-6v8wQDiFM+DNAg@mail.gmail.com>
<CA+Tgmob7ozJAs5SU6bD2RfAt4w_AmsMGz-aaVA6WeLXHkBypOg@mail.gmail.com>
<CAK98qZ1J42RoAsHnYWGPPmHziZmzmqE=Lp_O6WJ-9aKK2qjikA@mail.gmail.com>
<CA+TgmoYjcBA6dw3nwiyfDzPXTCrxTZPXDMrc2TrDJcL1cPK6iA@mail.gmail.com>
<CA+TgmoYru-vxoTKfwjQby30r2OkTXfb18Km_=VLs6qk8Akr0-g@mail.gmail.com>
<CAKFQuwYxTMx1e1dvVLd5vs8CO2Xe__YRCW0m4v2UErG=5Lu7kQ@mail.gmail.com>
<CA+TgmoaPH3rOmH1zw8s10Az3TQKRbDnF3dKYZCAFvx_DgFV5_Q@mail.gmail.com>
<CAKFQuwZ7CRbvmJ60EjvEUJP8X9hWVCHJz8yJZ0-xRpYwL=fugA@mail.gmail.com>
<CA+TgmoYsG2pAOG4YYWwnGsaL+QfOZopi_m+f_hZqBZ3meNEkjg@mail.gmail.com>
<CAKFQuwY4i6xXCRHd=ccrFz0sQOfiWRxhobiBrzyK0rzu1-Y33Q@mail.gmail.com>
<CA+Tgmobbj_TaCsYmr1grJBTRKaFaxFfotXn1T6LSXs9GQ8_Kyw@mail.gmail.com>
<CA+TgmoZzM2i+p-Rxdphs4qx7sshn-kzxF91ASQ5duOo0dFRXLQ@mail.gmail.com>
<CA+Tgmoau7yJtvbeH-0kPt1Q=Gt_ezRdgM35Q1=LT665U_86Etg@mail.gmail.com>
<CA+TgmoYRJdDh7MPK-cLs4w4gmYqEaWM3Y0o+2-YAKfJ=5rHNyA@mail.gmail.com>
On Wed, Mar 18, 2026 at 10:08 AM Robert Haas <[email protected]> wrote:
> But this does raise two points which are perhaps worthy of some
> further consideration:
>
> 1. Maybe we should limit the length of the detail message. In some
> other cases, like reportDependentObjects, we limit the detail message
> to the first 100 problems found, and then say at the end of that
> detail message how many more problems there were afterwards. We could
> do that here, too. I'm not 100% sure it's worth the code, but then
> again it's not much code.
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.
> 2. The way the current code works is to transform the advice feedback
> into a Node-tree representation which is stashed in the PlannedStmt's
> extension_state, and then also passes that Node-tree representation to
> pgpa_planner_feedback_warning, which generates the actual warning.
> That Node-tree representation is currently used by EXPLAIN to display
> the advice feedback, which I think for most people will be a nicer
> interface than enabling pg_plan_advice.feedback_warnings, and it can
> also be used by other extensions. For instance, we could extend
> pg_stash_advice so that it looks at the advice feedback and updates
> the shared store, so that users can monitor whether their stashed
> advice is doing what they hoped it would.
Yeah, I think the ability for other extensions to retrieve this is
pretty important - whether in pg_stash_advice, or any other kind of
plan management extension that wants to know the outcome of applied
advice.
>
> However, in a case like this, that Node tree is actually quite large:
> about 16MB. I guess that's because pgpa_planner_append_feedback() has
> to do multiple allocations for every item of feedback: a C string,
> DefElem, an Integer, plus whatever lappend() charges us to add to a
> List. We could save that memory by adding code here to optimize for
> the case where we need to generate warnings but we don't need the Node
> tree for anything else. I'm inclined not to do that, because (A) I
> don't think temporarily using 16MB when the user specifies 10,000
> items of bogus advice is really that bad and (B) complicating the code
> has its own costs, such as maybe introducing more bugs. However, maybe
> someone else sees it differently.
>
> Another idea is to try to find a more economic Node representation.
> For instance, we could jam the flags into the DefElem's location
> field, instead of building a separate Integer node, or we could build
> the advice feedback as a giant binary blob and wrap it in a varlena
> and a Const node and leave consumers to make sense of that as best
> they're able, or we could invent a new Node type that's just to the
> perfect thing to hold a C string and an integer. I'm inclined to think
> that the first two are too hacky and the third is too special-purpose,
> but again someone else might see it otherwise.
Yeah, I feel like we're definitely constrained here by the fact that
advice tags are defined by a contrib module vs in-core - if they were
in-core we could just add a dedicated node type for them. I don't
think inventing a specialized binary format only defined in the
contrib module makes sense.
Two other ideas:
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)
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
Thanks,
Lukas
--
Lukas Fittl
view thread (184+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected]
Subject: Re: pg_plan_advice
In-Reply-To: <CAP53PkxpPsB+xtuC1zKt=6ANw8vHjX_SJe2k=0AguMAqaK67Lg@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox