I wrote:
> Heikki Linnakangas <hlinnakangas@vmware.com> writes:
>> Hmm, something like the attached? Seems reasonable...
> This looks pretty unsafe to me: it assumes, without much justification,
> that there is no memory allocated during show_expression() that will be
> needed later.
> I suspect the O(N^2) consumption comes directly from some workspace
> allocated for variable deparsing in ruleutils.c. A safer fix would
> be to do retail pfrees on that.
I looked at this a bit more, and realized that we've got worse problems
than just O(N^2) space consumption: there's also O(N^2) time consumption
in ruleutils' set_simple_column_names(). The constant factor is not too
bad in this example but would be horrible with 1000 regular relations in
the rtable :-(. Safe or not, the extra-context solution doesn't improve
the code's time consumption.
Really the issue here is that explain.c assumes that
deparse_context_for_planstate() is negligibly cheap, which was once true
but is so no longer. What's more, most of the work is dependent only on
the rtable and so there is no good reason at all to be doing it more than
once per plan tree.
I think we can fix this by refactoring so that we construct a deparse
context once in ExplainPrintPlan(), and then just repoint it at
different planstate nodes as we work through the plan tree.
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)?
regards, tom lane
