From: | Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp> |
---|---|
To: | michael(dot)paquier(at)gmail(dot)com |
Cc: | coelho(at)cri(dot)ensmp(dot)fr, pgsql-hackers(at)postgresql(dot)org |
Subject: | Re: pgbench - allow backslash-continuations in custom scripts |
Date: | 2015-12-22 08:34:51 |
Message-ID: | 20151222.173451.229754367.horiguchi.kyotaro@lab.ntt.co.jp |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
I'm terribly sorry to have overlooked Febien's comment.
I'll rework on this considering Febien's previous comment and
Michael's this comment in the next CF.
At Tue, 22 Dec 2015 16:17:07 +0900, Michael Paquier <michael(dot)paquier(at)gmail(dot)com> wrote in <CAB7nPqRX_-VymeEH-3ChoPrQLgKh=EGgQ2GUtZ53ccO9uLGmxA(at)mail(dot)gmail(dot)com>
> 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;"
Mmm. It is differenct from my recognition. I'll confirm that.
> > I join a test script I used.
Thank you. I'll work on with it.
> > 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...
I have to admit that I think just the same.
> 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.
If I read you correctly, I should cut it out into a new file and
include it. Is it correct?
> + # 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).
Oops. Thank you for pointing out. It worked for me but, honestly
saying, I couldn't another clean way to do that but I'll
reconsider it.
> 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?
It's fine for me.
regards,
--
Kyotaro Horiguchi
NTT Open Source Software Center
From | Date | Subject | |
---|---|---|---|
Next Message | Pavel Stehule | 2015-12-22 08:59:07 | Re: plpgsql - DECLARE - cannot to use %TYPE or %ROWTYPE for composite types |
Previous Message | Viktor Leis | 2015-12-22 08:28:03 | Re: Experimental evaluation of PostgreSQL's query optimizer |