Re: Don't deform column-by-column in composite_to_json

On 2019-02-05 22:53:37 -0300, Alvaro Herrera wrote:
> On 2019-Feb-01, Andres Freund wrote:
>
> > diff --git a/src/backend/utils/adt/json.c b/src/backend/utils/adt/json.c
> > index de0d0723b71..8724022df54 100644
> > --- a/src/backend/utils/adt/json.c
> > +++ b/src/backend/utils/adt/json.c
> > @@ -1755,6 +1755,8 @@ composite_to_json(Datum composite, StringInfo result, bool use_line_feeds)
> > int i;
> > bool needsep = false;
> > const char *sep;
> > + Datum values[MaxHeapAttributeNumber];
> > + bool nulls[MaxHeapAttributeNumber];
> >
> > sep = use_line_feeds ? ",\n " : ",";
>
> Isn't this putting much more than needed in the stack? Seems like we
> could just allocate tupdesc->natts members dynamically. Not sure if we
> care: it's about 12 kB; maybe considering palloc overhead, using the
> stack is better.

I addressed that:

> > A short test shows that it'd be slower to allocate nulls/values with
> > palloc rather than using MaxHeapAttributeNumber. Given that only output
> > functions are called from within composite_to_json(), I think that's ok.

> Worth asking. But if this is worth doing here, then it's worth doing in
> a lot more places, isn't it?

"it" being allocating values/nulls on the stack? I think there's plenty
of places that do that. But it's also worth considering whether the
relevant piece of code calls more deeply into other code, in which case
the stack usage might be more problematic.

Greetings,

Andres Freund