From: | Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr> |
---|---|
To: | Michael Paquier <michael(at)paquier(dot)xyz> |
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-26 21:35:03 |
Message-ID: | alpine.DEB.2.21.2003262103530.22611@pseudo |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Bonjour Michaël,
> [...] Still sounds strange to me to invent a new variable to this
> structure if it is possible to track the exact same thing with an
> existing part of a Command, or it would make sense to split Command into
> two different structures with an extra structure used after the parsing
> for clarity?
Hmmm.
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.
> Well, it still looks cleaner to me to just assign the meta field
> properly within ParseScript(), and you get the same result. And it is
> also possible to use "meta" to do more sanity checks around META_GSET
> for some code paths. So I'd actually find the addition of a new
> argument using a meta command within readCommandResponse() cleaner.
I tried to do that.
> - * varprefix SQL commands terminated with \gset have this set
> + * varprefix SQL commands terminated with \gset or \aset have this set
> Nit from v4: varprefix can be used for \gset and \aset, and the
> comment was not updated.
It is now updated.
> + /* 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.).
> - } while (res);
> + } while (res != NULL);
> Useless diff.
Yep.
Attached an updated v5.
--
Fabien.
Attachment | Content-Type | Size |
---|---|---|
pgbench-aset-5.patch | text/x-diff | 9.9 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Alvaro Herrera | 2020-03-26 21:56:36 | Re: [Patch] pg_rewind: options to use restore_command from recovery.conf or command line |
Previous Message | Alexey Kondratov | 2020-03-26 21:24:19 | Re: [Patch] pg_rewind: options to use restore_command from recovery.conf or command line |