Re: pgbench and timestamps

From: Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: David Rowley <dgrowleyml(at)gmail(dot)com>, Jaime Soler <jaime(dot)soler(at)gmail(dot)com>, PostgreSQL mailing lists <pgsql-general(at)postgresql(dot)org>
Subject: Re: pgbench and timestamps
Date: 2020-06-29 15:33:12
Message-ID: alpine.DEB.2.22.394.2006291715360.805678@pseudo
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-general


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.

Attachment Content-Type Size
pgbench-var-fix-1.patch text/x-diff 17.0 KB

In response to

Browse pgsql-general by date

  From Date Subject
Next Message Paul Förster 2020-06-29 15:34:20 Re: EXTERNAL: Re: Netapp SnapCenter
Previous Message Stephen Frost 2020-06-29 15:29:52 Re: EXTERNAL: Re: Netapp SnapCenter