Re: pgbench and timestamps (bounced)

[Resent on hackers for CF registration, sorry for the noise]

Hello Tom,

The attached patch fixes some of the underlying problems reported by delaying
the :var to $1 substitution to the last possible moments, so that what
variables are actually defined is known. PREPARE-ing is also delayed to after
these substitutions are done.

It requires a mutex around the commands, I tried to do some windows
implementation which may or may not work.

The attached patch fixes (2) & (3) for extended & prepared.

I have a doubt about fixing (1) because it would be a significant behavioral
change and it requires changing the replace routine significantly to check for
quoting, comments, and so on. This means that currently ':var' is still broken
under -M extended & prepared, I could only break it differently by providing a
nicer error message and also break it under simple whereas it currently works
there. I'm not thrilled by spending efforts to do that.

The patches change the name of "parseQuery" to "makeVariablesParameters",
because it was not actually parsing any query. Maybe the new name could be
improved.

In passing, there was a bug in how NULL was passed, which I tried to fix
as well.

>>> I don't often do much with pgbench and variables, but there are a few
>>> things that surprise me here.
>>> 1) That pgbench replaces variables within single quotes, and;
>>> 2) that we still think it's a variable name when it starts with a digit,
>>> and;
>>> 3) We replace variables that are undefined.
>
>> Also (4) this only happens when in non-simple query mode --- the
>> example works fine without "-M prepared".
>
> After looking around in the code, it seems like the core of the issue
> is that pgbench.c's parseQuery() doesn't check whether a possible
> variable name is actually defined, unlike assignVariables() which is
> what does the same job in simple query mode. So that explains the
> behavioral difference.

Yes.

> The reason for doing that probably was that parseQuery() is run when
> the input file is read, so that relevant variables might not be set
> yet. We could fix that by postponing the work to be done at first
> execution of the query, as is already the case for PQprepare'ing the
> query.

Yep, done at first execution of the Command, so that variables are known.

> Also, after further thought I realize that (1) absolutely is a bug
> in the non-simple query modes, whatever you think about it in simple
> mode. The non-simple modes are trying to pass the variable values
> as extended-query-protocol parameters, and the backend is not going
> to recognize $n inside a literal as being a parameter.

Yep. See my comments above.

> If we fixed (1) and (3) I think there wouldn't be any great need
> to tighten up (2).

I did (2) but not (1), for now.

--
Fabien.