Re: pgbench MAX_ARGS

On Wed, 27 Feb 2019 at 01:57, Simon Riggs <simon@2ndquadrant.com> wrote:
> I've put it as 256 args now.

I had a look at this and I see you've added some docs to mention the
number of parameters that are allowed; good.

+   <application>pgbench</application> supports up to 256 variables in one
+   statement.

However, the code does not allow 256 variables as the documents claim.
Per >= in:

  if (cmd->argc >= MAX_ARGS)
  {
  fprintf(stderr, "statement has too many arguments (maximum is %d): %s\n",

For it to be 256 that would have to be > MAX_ARGS.

I also don't agree with this change:

- MAX_ARGS - 1, cmd->lines.data);
+ MAX_ARGS, cmd->lines.data);

The 0th element of the argv array was for the sql, per:

cmd->argv[0] = sql;

then the 9 others were for the variables, so the MAX_ARGS - 1 was
correct originally. I think some comments in the area to explain the
0th is for the sql would be a good idea too, that might stop any
confusion in the future. I see that's documented in the struct header
comment, but maybe worth a small note around that error message just
to confirm the - 1 is not a mistake, and neither is the >= MAX_ARGS.

Probably it's fine to define MAX_ARGS to 256 then put back the
MAX_ARGS - 1 code so that we complain if we get more than 255....
unless 256 is really needed, of course, in which case MAX_ARGS will
need to be 257.

The test also seems to test that 256 variables in a statement gives an
error. That contradicts the documents that have been added, which say
256 is the maximum allowed.

Setting to WoA

-- 
 David Rowley                   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services