From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
---|---|
To: | Alvaro Herrera <alvherre(at)2ndquadrant(dot)com> |
Cc: | Heikki Linnakangas <hlinnakangas(at)vmware(dot)com>, Alexey Bashtanov <bashtanov(at)imap(dot)cc>, pgsql-hackers(at)postgresql(dot)org |
Subject: | Re: OOM on EXPLAIN with lots of nodes |
Date: | 2015-01-13 17:24:12 |
Message-ID: | 22652.1421169852@sss.pgh.pa.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Alvaro Herrera <alvherre(at)2ndquadrant(dot)com> writes:
> Tom Lane wrote:
>> A difficulty with either your patch or my idea is that they require adding
>> another field to ExplainState, which is an ABI break for any third-party
>> code that might be declaring variables of that struct type. That's fine
>> for HEAD but would be risky to back-patch. Any thoughts about whether we
>> can get away with that (ie, anybody have an idea if there are third-party
>> extensions that call explain.c)?
> codesearch.debian.net shows a couple of hits for ExplainState in
> multicorn (an extension for FDW from Python data sources); I didn't look
> but it seems that the FDW API is using that struct.
It is, but FDWs are not at risk here: they merely reference ExplainStates
that were allocated by core backend code. So as long as we add the new
field at the end it's not a problem for them. Problematic usage would be
like what auto_explain does:
ExplainState es;
ExplainInitState(&es);
...
In hindsight, that's a bad API and we should change it to something like
ExplainState *es = NewExplainState();
so that the sizeof the struct isn't embedded in extension code. But we
definitely can't do that in back branches.
regards, tom lane
From | Date | Subject | |
---|---|---|---|
Next Message | Heikki Linnakangas | 2015-01-13 17:42:56 | Re: OOM on EXPLAIN with lots of nodes |
Previous Message | Alvaro Herrera | 2015-01-13 17:13:49 | Re: OOM on EXPLAIN with lots of nodes |