pgbench: \gset and \aset should store SQL NULL as the null value
Hackorum builds and tests every patch posted to the lists, not only commitfest submissions. This is Hackorum's own CI rather than the PostgreSQL project's, and it is still under testing - please report anything that looks wrong.
This thread has been committed, so CI has stopped here. Anything below is the last result it produced.
You can run a PostgreSQL built from this patch straight from Docker, with no checkout and no build:
docker run --rm -p 5432:5432 ghcr.io/hackorum-dev/postgres-patch:t253460psql -h localhost -U postgresBuilt from patchset v3 (message #3), September 09, 2026 at 01:01 PM.
Every patchset is also pushed to a branch of our PostgreSQL fork, so you can check out the same tree CI built. Without a PostgreSQL checkout:
git clone --branch t253460_3 https://github.com/hackorum-dev/postgres.gitIn a checkout you already have, add the fork once:
git remote add hackorum https://github.com/hackorum-dev/postgres.gitthen, for this patchset and every later one:
git fetch hackorum t253460_3 && git checkout t253460_3Patchset v3 (message #3) is on t253460_3
Hi hackers,
I am proposing a patch that makes pgbench's \gset and \aset store a
SQL NULL as the null value, so that capturing a NULL no longer kills
the client.
readCommandResponse() stores every column with PQgetvalue(), which
returns an empty string for SQL NULL. An empty string is not a valid
value in the pgbench expression language, so the first expression use
of such a variable aborts the client. Neither --max-tries nor
--continue-on-error recovers from it.
I ran into this writing a TPC-C like script, where min() over an empty
set is a normal branch and not an error. Reduced to the smallest form:
```
$ cat repro.sql
SELECT NULL AS nv \gset
\set i debug(:nv)
$ pgbench -n -t 1 -f repro.sql postgres
pgbench: error: client 0 aborted in command 1 (set) of script 0;
evaluation of meta-command failed
number of transactions actually processed: 0/1
```
The workaround is coalesce() with a sentinel the query cannot return,
so IS NULL is unusable on anything \gset produced.
The patch checks PQgetisnull() and assigns the null value, exactly as
\set varname NULL does. An empty string returned by the server still
aborts the client when used in an expression.
A NULL-valued variable is still bound as the string "NULL" in the
extended and prepared query modes. That is the existing behavior of
\set varname NULL, so the patch only documents it.
Scripts that captured a NULL and interpolated it textually used to get
an empty string, usually a syntax error, but inside quotes it silently
inserted ''. That silent change is why I think this is master only,
though an unrecoverable client abort is an argument for backpatching.
The patch is attached. Thoughts?
--
Shinya Kato
NTT OSS Center
Good catch!
At Tue, 18 Aug 2026 14:07:02 +0900, Shinya Kato <shinya11.kato@gmail.com> wrote in
The patch is attached. Thoughts?
I think the code changes look good. Since pgbench variables can hold
NULL values, I think it is correct to handle SQL NULL this way. On the
other hand, I think the tests could be reduced somewhat. For example,
I don't think the tests for \aset and sorting are particularly
necessary.
I think this change can also be seen as making \gset and \aset behave
as one would naturally expect from the existing documentation: an SQL
NULL result is stored as a pgbench NULL value. If so, I wonder whether
additional documentation is needed at all. I think documentation of
the existing behavior would be better handled in a separate patch from
this change.
The patch checks PQgetisnull() and assigns the null value, exactly as
\set varname NULL does. An empty string returned by the server still
aborts the client when used in an expression.
This happens with any string that cannot be converted to a value type
supported by pgbench expressions. So I think it would be possible to
replace an empty string returned by PQgetvalue() with "null", but if
we are going to check for NULL anyway, I think it is better to assign
NULL directly.
Regards,
--
Kyotaro Horiguchi
NTT Open Source Software Center
Thank you for looking into this.
On Tue, Aug 18, 2026 at 4:46 PM Kyotaro Horiguchi
<horikyota.ntt@gmail.com> wrote:
On the
other hand, I think the tests could be reduced somewhat. For example,
I don't think the tests for \aset and sorting are particularly
necessary.
Removed them in v2. \aset goes through the same storing loop as \gset,
so the \gset cases already cover it.
I think this change can also be seen as making \gset and \aset behave
as one would naturally expect from the existing documentation: an SQL
NULL result is stored as a pgbench NULL value. If so, I wonder whether
additional documentation is needed at all. I think documentation of
the existing behavior would be better handled in a separate patch from
this change.
Agreed. 0001 no longer touches the documentation and 0002 has it. The
behaviors it describes apply to every variable regardless of how it
was set, so the paragraph goes next to the description of the
variable-substitution facility rather than under an individual meta
command.
This happens with any string that cannot be converted to a value type
supported by pgbench expressions. So I think it would be possible to
replace an empty string returned by PQgetvalue() with "null", but if
we are going to check for NULL anyway, I think it is better to assign
NULL directly.
Agreed, and thanks for confirming.
I created a CF entry: https://commitfest.postgresql.org/patch/7163/
--
Shinya Kato
NTT OSS Center
Attachments:
t253460_3v2-0001-Make-pgbench-gset-and-aset-store-SQL-NULL-as-the-.patchapplication/octet-stream; name=v2-0001-Make-pgbench-gset-and-aset-store-SQL-NULL-as-the-.patchDownload+83-5
v2-0002-doc-Describe-how-a-NULL-valued-pgbench-variable-i.patchapplication/octet-stream; name=v2-0002-doc-Describe-how-a-NULL-valued-pgbench-variable-i.patchDownload+10-1
Hi,
I tested v2 on master (86f7c82cf10). It builds cleanly and the pgbench
TAP tests pass. Four tests in that file fail on my machine, but they
fail without the patch too.
I confirmed the bug. Neither --continue-on-error nor --max-tries helps.
The old behavior was broken for text substitution as well:
"SELECT 1 WHERE :nv IS NULL;" becomes "WHERE IS NULL" and fails. So I
do not think anyone relies on it.
One doc gap: 0001 changes what \gset does with a NULL column, but the \gset
docs say nothing about NULL. One sentence is enough, so no v3 is
needed. Master only, no back-patch.
Please commit 0002 together with 0001. Patch 0001 says a captured NULL
can be tested with IS NULL, but that test cannot tell a real NULL from
a column holding the text 'NULL'. Only 0002 explains this.
As CommitFest manager I am moving
https://commitfest.postgresql.org/patch/7163/ to Ready for Committer,
Thanks,
Shihao
On Tue, Sep 08, 2026 at 10:05:31PM -0400, shihao zhong wrote:
One doc gap: 0001 changes what \gset does with a NULL column, but the \gset
docs say nothing about NULL. One sentence is enough, so no v3 is
needed. Master only, no back-patch.
Bug is a term mentioned on this thread, but I don't see a need for a
backpatch based on the lack of complaints. There is also a secondary
reason regarding compatibility, even if somewhat accidental with the
handling of NULL values. HEAD seems fine enough.
Please commit 0002 together with 0001. Patch 0001 says a captured NULL
can be tested with IS NULL, but that test cannot tell a real NULL from
a column holding the text 'NULL'. Only 0002 explains this.
Not sure if this is worth bothering in the docs, but I'll think more
about this point.
Similarly to Horiguchi-san's comments, I think that the tests could be
trimmed even more. I see no need for at least:
- "gset NULL interpolates as SQL NULL"
- "gset empty string stays empty"
- "gset NULL is bound as the string NULL in extended and prepared
modes"
These just act as cross-checks of the pgbench runs, that provide some
coverage due to the executions working.
At the end I would just keep the "pgbench gset command with NULL" bit,
which should be enough to cover readCommandResponse(), no? No need to
be fancy, efficient is fine and saves runtime cycles.
--
Michael
On Wed, Sep 09, 2026 at 02:23:35PM +0900, Michael Paquier wrote:
Similarly to Horiguchi-san's comments, I think that the tests could be
trimmed even more. I see no need for at least:
- "gset NULL interpolates as SQL NULL"
- "gset empty string stays empty"
- "gset NULL is bound as the string NULL in extended and prepared
modes"
Yeah, I'm still unconvinced by the value brought here in terms of
cross-checking an actual NULL and a NULL but written as a string. The
first test proposed is also good enough to check that we generate a
NULL variable through setNullValue()->putVariableValue(). So kept
only the first test, applied the result.
At the end I would just keep the "pgbench gset command with NULL" bit,
which should be enough to cover readCommandResponse(), no? No need to
be fancy, efficient is fine and saves runtime cycles.
Ditto about the doc suggestion. If somebody is excited enough about
this part, please feel free..
--
Michael
On Thu, Sep 10, 2026 at 1:16 PM Michael Paquier <michael@paquier.xyz> wrote:
The
first test proposed is also good enough to check that we generate a
NULL variable through setNullValue()->putVariableValue(). So kept
only the first test, applied the result.
You're right, the first case is enough. Thanks for committing it!
--
Shinya Kato
NTT OSS Center