Re: Large expressions in indexes can't be stored (non-TOASTable)

On Wed, Apr 09, 2025 at 08:54:21PM -0300, Euler Taveira wrote:
> LGTM. I have a few suggestions.

Thanks for reviewing.

> +   /*
> +    * To avoid needing a TOAST table for pg_replication_origin, we restrict
> +    * replication origin names to 512 bytes.  This should be more than enough
> +    * for all practical use.
> +    */
> +   if (strlen(roname) > MAX_RONAME_LEN)
> +       ereport(ERROR,
> 
> I wouldn't duplicate the comment. Instead, I would keep it only in origin.h.

Hm.  I agree that duplicating the comment isn't great, but I'm also not
wild about forcing readers to jump to the macro definition to figure out
why there is a length restriction.

> +                errdetail("Repilcation origin names must be no longer than %d bytes.",
> +                          MAX_RONAME_LEN)));
> 
> s/Repilcation/Replication/

Fixed.

> +#define MAX_RONAME_LEN (512)
> 
> It is just a cosmetic suggestion but I usually use parentheses when it is an
> expression but don't include it for a single number.

We use both styles, but the no-parentheses style does seem to be preferred.

    $ grep -E "^#define\s[A-Z_]+\s\([0-9]+\)$" src/* -rI | wc -l
          23
    $ grep -E "^#define\s[A-Z_]+\s[0-9]+$" src/* -rI | wc -l
         861

> Should we document this maximum length?

I've added a note.

-- 
nathan