Re: logrep stuck with 'ERROR: int2vector has too many elements'

Hi,

On 2023-01-15 15:17:16 -0500, Tom Lane wrote:
> Andres Freund <andres(at)anarazel(dot)de> writes:
> > On 2023-01-15 14:39:41 -0500, Tom Lane wrote:
> >> But I suppose we are stuck with that, seeing that this datatype choice
> >> is effectively part of the logrep protocol now. I think the only
> >> reasonable solution is to get rid of the FUNC_MAX_ARGS restriction
> >> in int2vectorin. We probably ought to back-patch that as far as
> >> pg_publication_rel.prattrs exists, too.
>
> > Are you thinking of introducing another, or just "rely" on too long arrays to
> > trigger errors when forming tuples?
>
> There's enough protections already, eg repalloc will complain if you
> try to go past 1GB.

We'll practically error out at a much lower limit than that, due to, due to
reaching the max length of a row and int2vector not being toastable. So I'm
just wondering if we want to set the limit to something that'll commonly avoid
erroring out out with something like
ERROR: 54000: row is too big: size 10048, maximum size 8160

For the purpose here a limit of MaxTupleAttributeNumber or such instead of
FUNC_MAX_ARGS would do the trick, I think?

> diff --git a/src/backend/utils/adt/int.c b/src/backend/utils/adt/int.c
> index e47c15a54f..44d1c7ad0c 100644
> --- a/src/backend/utils/adt/int.c
> +++ b/src/backend/utils/adt/int.c
> @@ -143,11 +143,13 @@ int2vectorin(PG_FUNCTION_ARGS)
> char *intString = PG_GETARG_CSTRING(0);
> Node *escontext = fcinfo->context;
> int2vector *result;
> + int nalloc;
> int n;
>
> - result = (int2vector *) palloc0(Int2VectorSize(FUNC_MAX_ARGS));
> + nalloc = 32; /* arbitrary initial size guess */
> + result = (int2vector *) palloc0(Int2VectorSize(nalloc));
>
> - for (n = 0; n < FUNC_MAX_ARGS; n++)
> + for (n = 0;; n++)
> {
> long l;
> char *endp;
> @@ -157,6 +159,12 @@ int2vectorin(PG_FUNCTION_ARGS)
> if (*intString == '\0')
> break;
>
> + if (n >= nalloc)
> + {
> + nalloc *= 2;
> + result = (int2vector *) repalloc(result, Int2VectorSize(nalloc));
> + }

Should this be repalloc0? I don't know if the palloc0 above was just used with
the goal of initializing the "header" fields, or also to avoid trailing
uninitialized bytes?

Greetings,

Andres Freund