|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|
|Views:||Raw Message | Whole Thread | Download mbox | Resend email|
> [...] 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?
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
> 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.
Attached an updated v5.
|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|