Thread: Document parameter count limit

On Thu, Nov 10, 2022 at 11:01:18AM -0700, David G. Johnston wrote:
> On Thu, Nov 10, 2022 at 10:58 AM Corey Huinker <corey.huinker@gmail.com> wrote:
> 
> 
>         +    <entry>if you are reading this prepatorily, please redesign your
>         query to use temporary tables or arrays</entry>
> 
> 
>     I agree with the documentation of this parameter.
>     I agree with dissuading anyone from attempting to change it
>     The wording is bordering on snark (however well deserved) and I think the
>     voice is slightly off.
> 
>     Alternate suggestion:
> 
> 
>         Queries approaching this limit usually can be refactored to use arrays
>         or temporary tables, thus reducing parameter overhead.
> 
> 
>     The bit about parameter overhead appeals to the reader's desire for
>     performance, rather than just focusing on "you shouldn't want this".
> 
> 
> Yeah, the wording is a bit tongue-in-cheek.  Figured assuming a committer wants
> this at all we'd come up with better wording.  I like your suggestion.

Does this come up enough to document it?  I assume the error message the
user receives is clear.

-- 
  Bruce Momjian  <bruce@momjian.us>        https://momjian.us
  EDB                                      https://enterprisedb.com

  Indecision is a decision.  Inaction is an action.  Mark Batterson

Bruce Momjian <bruce@momjian.us> writes:
> Does this come up enough to document it?  I assume the error message the
> user receives is clear.

Looks like you get

    if (nParams < 0 || nParams > PQ_QUERY_PARAM_MAX_LIMIT)
    {
        libpq_append_conn_error(conn, "number of parameters must be between 0 and %d",
                           PQ_QUERY_PARAM_MAX_LIMIT);
        return 0;
    }

which seems clear enough.

I think the concern here is that somebody who's not aware that a limit
exists might write an application that thinks it can send lots of
parameters, and then have it fall over in production.  Now, I've got
doubts that an entry in the limits.sgml table will do much to prevent
that scenario.  But perhaps offering the advice to use an array parameter
will be worthwhile even after-the-fact.

            regards, tom lane

"David G. Johnston" <david.g.johnston@gmail.com> writes:
> I do believe that people who want to use a large parameter list likely have
> that question in the back of their mind, and looking at a page called
> "System Limits" is at least plausibly something they would do.  Since they
> are really caring about parse-bind-execute, and they aren't likely to dig
> into libpq, this seems like the best spot (as opposed to, say PREPARE)

This is a wire-protocol limitation; libpq is only the messenger.
So if we're going to document it, I agree that limits.sgml is the place.

(BTW, I'm not certain that PREPARE has the same limit.  It'd fall over
at INT_MAX likely, or maybe sooner for lack of memory, but I don't
recall that there's any uint16 fields in that code path.)

            regards, tom lane

On Wed, Nov 23, 2022 at 12:35:59PM -0700, David G. Johnston wrote:
> On Wed, Nov 23, 2022 at 11:47 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> 
> > Bruce Momjian <bruce@momjian.us> writes:
> > > Does this come up enough to document it?  I assume the error message the
> > > user receives is clear.
> >
> > Looks like you get
> >
> >     if (nParams < 0 || nParams > PQ_QUERY_PARAM_MAX_LIMIT)
> >     {
> >         libpq_append_conn_error(conn, "number of parameters must be between 0 and %d",
> >                            PQ_QUERY_PARAM_MAX_LIMIT);
> >         return 0;
> >     }
> >
> > which seems clear enough.
> >
> > I think the concern here is that somebody who's not aware that a limit
> > exists might write an application that thinks it can send lots of
> > parameters, and then have it fall over in production.  Now, I've got
> > doubts that an entry in the limits.sgml table will do much to prevent
> > that scenario.  But perhaps offering the advice to use an array parameter
> > will be worthwhile even after-the-fact.

Yes, that's what happens :)

I hit that error after increasing the number of VALUES(),() a loader
used in a prepared statement (and that was with our non-wide tables).

+1 to document the limit along with the other limits.

-- 
Justin

On Wed, Nov 23, 2022 at 02:33:27PM -0600, Justin Pryzby wrote:
> On Wed, Nov 23, 2022 at 12:35:59PM -0700, David G. Johnston wrote:
> > On Wed, Nov 23, 2022 at 11:47 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> > 
> > > Bruce Momjian <bruce@momjian.us> writes:
> > > > Does this come up enough to document it?  I assume the error message the
> > > > user receives is clear.
> > >
> > > Looks like you get
> > >
> > >     if (nParams < 0 || nParams > PQ_QUERY_PARAM_MAX_LIMIT)
> > >     {
> > >         libpq_append_conn_error(conn, "number of parameters must be between 0 and %d",
> > >                            PQ_QUERY_PARAM_MAX_LIMIT);
> > >         return 0;
> > >     }
> > >
> > > which seems clear enough.
> > >
> > > I think the concern here is that somebody who's not aware that a limit
> > > exists might write an application that thinks it can send lots of
> > > parameters, and then have it fall over in production.  Now, I've got
> > > doubts that an entry in the limits.sgml table will do much to prevent
> > > that scenario.  But perhaps offering the advice to use an array parameter
> > > will be worthwhile even after-the-fact.
> 
> Yes, that's what happens :)
> 
> I hit that error after increasing the number of VALUES(),() a loader
> used in a prepared statement (and that was with our non-wide tables).
> 
> +1 to document the limit along with the other limits.

Here is a patch to add this.

-- 
  Bruce Momjian  <bruce@momjian.us>        https://momjian.us
  EDB                                      https://enterprisedb.com

  Only you can decide what is important to you.

Bruce Momjian <bruce@momjian.us> writes:
> Here is a patch to add this.

"function arguments" seems like a completely wrong description
(and if we do want to document that limit, it's 100).

"query parameters" would work, perhaps.

            regards, tom lane

On Thu, Oct 26, 2023 at 06:56:40PM -0400, Tom Lane wrote:
> Bruce Momjian <bruce@momjian.us> writes:
> > Here is a patch to add this.
> 
> "function arguments" seems like a completely wrong description
> (and if we do want to document that limit, it's 100).
> 
> "query parameters" would work, perhaps.

Ah, I was confused.  I documented both in the attached patch.

-- 
  Bruce Momjian  <bruce@momjian.us>        https://momjian.us
  EDB                                      https://enterprisedb.com

  Only you can decide what is important to you.

Bruce Momjian <bruce@momjian.us> writes:
> Ah, I was confused.  I documented both in the attached patch.

The function one should have the same annotation as some others:

     <entry>can be increased by recompiling <productname>PostgreSQL</productname></entry>

            regards, tom lane

"David G. Johnston" <david.g.johnston@gmail.com> writes:
> Cleanups for consistency:

> Move "identifier length" after "partition keys" (before the new "function
> arguments")

Yeah, the existing ordering of this table seems quite random.
That would help some, by separating items having to do with
database/table size from SQL-query-related limits.

            regards, tom lane

On Thu, Oct 26, 2023 at 04:17:19PM -0700, David G. Johnston wrote:
> On Thu, Oct 26, 2023 at 4:13 PM David G. Johnston <david.g.johnston@gmail.com>
> wrote:
> 
>     On Thu, Oct 26, 2023 at 4:08 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> 
>         Bruce Momjian <bruce@momjian.us> writes:
>         > Ah, I was confused.  I documented both in the attached patch.
> 
>         The function one should have the same annotation as some others:
> 
>              <entry>can be increased by recompiling <productname>PostgreSQL</
>         productname></entry>
> 
> 
> 
>     I'd like to see a comment on the parameter count one too.
> 
>     "Alternatives include using a temporary table or passing them in as a
>     single array parameter."
> 
>     About the only time this is likely to come up is with many parameters of
>     the same type and meaning, pointing that out with the array option seems
>     excessively wordy for the comment area.
> 
>     Needs a comma: 65,535
> 
>     Kinda think both should be tacked on to the end of the table.  I'd also put
>     function arguments first so it appears under the compile time partition
>     keys limit.
> 
> 
> 
> Cleanups for consistency:
> 
> Move "identifier length" after "partition keys" (before the new "function
> arguments")
> 
> Add commas to: 1,600 and 1,664 and 8,192

Okay, I made all the suggested changes in ordering and adding commas,
plus the text about the ability to change function arguments via
recompiling.

I didn't put commas in 8192 since that is a power-of-two and kind of a
magic number used in many places.

I am not sure where to put text about using arrays to handle many
function arguments.  I just don't see it fitting in the table, or the
paragraph below the table.

Patch attached.

-- 
  Bruce Momjian  <bruce@momjian.us>        https://momjian.us
  EDB                                      https://enterprisedb.com

  Only you can decide what is important to you.

On Thu, Oct 26, 2023 at 11:04:47PM -0400, Bruce Momjian wrote:
> On Thu, Oct 26, 2023 at 04:17:19PM -0700, David G. Johnston wrote:
> > On Thu, Oct 26, 2023 at 4:13 PM David G. Johnston <david.g.johnston@gmail.com>
> > wrote:
> > 
> >     On Thu, Oct 26, 2023 at 4:08 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> > 
> >         Bruce Momjian <bruce@momjian.us> writes:
> >         > Ah, I was confused.  I documented both in the attached patch.
> > 
> >         The function one should have the same annotation as some others:
> > 
> >              <entry>can be increased by recompiling <productname>PostgreSQL</
> >         productname></entry>
> > 
> > 
> > 
> >     I'd like to see a comment on the parameter count one too.
> > 
> >     "Alternatives include using a temporary table or passing them in as a
> >     single array parameter."
> > 
> >     About the only time this is likely to come up is with many parameters of
> >     the same type and meaning, pointing that out with the array option seems
> >     excessively wordy for the comment area.
> > 
> >     Needs a comma: 65,535
> > 
> >     Kinda think both should be tacked on to the end of the table.  I'd also put
> >     function arguments first so it appears under the compile time partition
> >     keys limit.
> > 
> > 
> > 
> > Cleanups for consistency:
> > 
> > Move "identifier length" after "partition keys" (before the new "function
> > arguments")
> > 
> > Add commas to: 1,600 and 1,664 and 8,192
> 
> Okay, I made all the suggested changes in ordering and adding commas,
> plus the text about the ability to change function arguments via
> recompiling.
> 
> I didn't put commas in 8192 since that is a power-of-two and kind of a
> magic number used in many places.
> 
> I am not sure where to put text about using arrays to handle many
> function arguments.  I just don't see it fitting in the table, or the
> paragraph below the table.

Patch applied back to Postgres 12.

-- 
  Bruce Momjian  <bruce@momjian.us>        https://momjian.us
  EDB                                      https://enterprisedb.com

  Only you can decide what is important to you.