Re: pgbench - add \aset to store results of a combined query

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

In response to

Responses

Browse pgsql-hackers by date

  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