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.paqu...@gmail.com>
wrote in <CAB7nPqRX_-VymeEH-3ChoPrQLgKh=eggq2gutz53cco9ulg...@mail.gmail.com>
> On Wed, Oct 14, 2015 at 5:49 PM, Fabien COELHO <coe...@cri.ensmp.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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
