Re: pgbench - allow backslash-continuations in custom scripts

From: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
To: Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr>
Cc: Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp>, PostgreSQL Developers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: pgbench - allow backslash-continuations in custom scripts
Date: 2015-12-22 07:17:07
Message-ID: CAB7nPqRX_-VymeEH-3ChoPrQLgKh=EGgQ2GUtZ53ccO9uLGmxA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Wed, Oct 14, 2015 at 5:49 PM, Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr> wrote:
>
> Hello,
>
> Here is a review, sorry for the delay...
>
>> This is done as the additional fourth patch, not merged into
>> previous ones, to show what's changed in the manner of command
>> storing.
>> [...]
>>>
>>> - SQL multi-statement.
>>>
>>> SELECT 1; SELECT 2;
>
>
> I think this is really "SELECT 1\; SELECT 2;"
>
> I join a test script I used.
>
>
> The purpose of this 4 parts patch is to reuse psql scanner from pgbench
> so that commands are cleanly separated by ";", including managing dollar
> quoting, having \ continuations in backslash-commands, having
> multi-statement commands...
>
> This review is about 4 part v4 of the patch. The patches apply and compile
> cleanly.
>
> I think that the features are worthwhile. I would have prefer more limited
> changes to get them, but my earlier attempt was rejected, and the scanner
> sharing with psql results in reasonably limited changes, so I would go for
> it.

Regarding that:
+#if !defined OUTSIDE_PSQL
+#include "variables.h"
+#else
+typedef int * VariableSpace;
+#endif

And that:
+/* Provide dummy macros when no use of psql variables */
+#if defined OUTSIDE_PSQL
+#define GetVariable(space,name) NULL
+#define standard_strings() true
+#define psql_error(fmt,...) do { \
+ fprintf(stderr, "psql_error is called. abort.\n");\
+ exit(1);\
+} while(0)
+#endif
That's ugly... Wouldn't it be better with something say in src/common
which is frontend-only? We could start with a set of routines allowing
commands to be parsed. That gives us more room for future improvement.

+ # fix up pg_xlogdump once it's been set up
+ # files symlinked on Unix are copied on windows
+ my $pgbench = AddSimpleFrontend('pgbench');
+ $pgbench->AddDefine('FRONTEND');
+ $pgbench->AddDefine('OUTSIDE_PSQL');
+ $pgbench->AddFile('src/bin/psql/psqlscan.l');
+ $pgbench->AddIncludeDir('src/bin/psql');
This is a simple copy-paste, with an incorrect comment at least
(haven't tested compilation with MSVC, I suspect that this is going to
fail still the flags are correctly set).

This patch is waiting for input from its author for quite some time
now, and the structure of this patch needs a rework. Are folks on this
thread fine if it is returned with feedback?
--
Michael

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2015-12-22 07:17:59 Re: pglogical_output - a general purpose logical decoding output plugin
Previous Message Michael Paquier 2015-12-22 07:04:06 Re: Commit fest status for 2015-11