Hi,
On 2020-04-05 15:04:30 -0400, Tom Lane wrote:
> Andres Freund <andres@anarazel.de> writes:
> > Another avenue could be to make ParseFuncOrColumn et al use less stack,
> > and hope that it avoids the problem. It's a bit insane that we use this
> > much.
>
> That would only reduce the chance of getting a stack overflow there,
> and not by that much, especially not for a CLOBBER_CACHE_ALWAYS animal
> which is going to be doing catalog accesses inside there too.
It'd certainly not be bullet proof. But I don't think we ever were? If I
understood you correctly we were just not noticing the stack overflow
danger before? We did catalog accesses from within there before too,
that's not changed by the addition of equal(), no?
Reminds me: I'll try to dust up my patch to make cache invalidation
processing non-recursive for 14 (I wrote an initial version as part of a
bugfix that we ended up fixing differently). Besides making
CLOBBER_CACHE_ALWAYS vastly less expensive, it also reduces the cost of
logical decoding substantially.
> > We don't have to go there in this case, but I've before wondered about
> > adding helpers that use an on-stack var for small allocations, and falls
> > back to palloc otherwise. Something boiling down to:
>
> Seems like that adds a lot of potential for memory leakage?
Depends on the case, I'd say. Certainly might be useful to add a helper
for a corresponding conditional free.
For parsing cases like this it could be better to bulk free at the
end. Compared to the memory needed for all the transformed arguments etc
it'd probably not matter in the short term (especially if only done for
4+ args).
Greetings,
Andres Freund
