From: | Michael Paquier <michael(at)paquier(dot)xyz> |
---|---|
To: | Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr> |
Cc: | Ibrar Ahmed <ibrar(dot)ahmad(at)gmail(dot)com>, PostgreSQL Developers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: pgbench - add \aset to store results of a combined query |
Date: | 2020-03-30 06:30:58 |
Message-ID: | 20200330063058.GB43995@paquier.xyz |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Thu, Mar 26, 2020 at 10:35:03PM +0100, Fabien COELHO wrote:
> Your point is to store the gset/aset status into the meta field, even if the
> command type is SQL. This is not done for gset, which relies on the non-null
> prefix, and breaks the assumption that meta is set to something only when
> the command is a meta command. Why not. I updated the comment, so now meta
> is none/gset/aset when command type is sql, and I removed the aset field.
Yes, that's the point I was trying to make. Thanks for sending a new
version.
> - * meta The type of meta-command, or META_NONE if command is SQL
> + * meta The type of meta-command, if command is SQL META_NONE,
> + * META_GSET or META_ASET which dictate what to do with the
> + * SQL query result.
I did not quite get why you need to update this comment. The same
concepts as before apply.
>> + /* coldly skip empty result under \aset */
>> + if (ntuples <= 0)
>> + break;
>> Shouldn't this check after \aset? And it seems to me that this code
>> path is not taken, so a test would be nice.
>
> Added (I think, if I understood what you suggested.).
+ /* coldly skip empty result under \aset */
+ else if (meta == META_ASET && ntuples <= 0)
+ break;
Yes, that's what I meant. Now it happens that we don't have a
regression test to cover the path where we have no tuples. Could it
be possible to add one?
+ Assert((meta == META_NONE && varprefix == NULL) ||
+ ((meta == META_GSET || meta == META_ASET) && varprefix != NULL));
+
Good addition. That would blow up if meta is set to something else
than {META_NONE,META_GSET,META_ASET}, so anybody changing this code
path will need to question if he/she needs to do something here.
Except for the addition of a test case to skip empty results when
\aset is used, I think that we are pretty good here.
--
Michael
From | Date | Subject | |
---|---|---|---|
Next Message | Masahiko Sawada | 2020-03-30 06:46:34 | Re: pg_stat_statements issue with parallel maintenance (Was Re: WAL usage calculation patch) |
Previous Message | Amit Kapila | 2020-03-30 06:24:33 | Re: backup manifests |