Re: pgbench - allow backslash-continuations in custom scripts

Hello,

At Fri, 25 Dec 2015 14:18:24 +0900 (Tokyo Standard Time), Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp> wrote in <20151225(dot)141824(dot)75912397(dot)horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp>
> > > Not really, I meant to see if it would be possible to include this set
> > > of routines directly in libpqcommon (as part of OBJS_FRONTEND). This
> > > way any client applications could easily reuse it. If we found that
> > > what was in psql is now useful to pgbench, I have little doubt that
> > > some other folks would make good use of that. I honestly have not
> > > looked at the code to see if that's doable or not, but soft-linking
> > > directly in pgbench a file of psql will not help future maintenance
> > > for sure. This increases the complexity of the code.
>
> Thanks. I understand it and I agree to the last sentense. I don't
> want them to be exposed as generic features.
>
> Instaed, I'd like to separate backslash commands from psqlscan.l
> and use callbacks to access variables by ":name" syntax (it is a
> common syntax between pgbench and psql). Current psqlscan.l
> simply exits noticing of leading backslash of backslash commands
> to the caller so it would be separated without heavy
> surgery. This can put away the ugliness of VariableSpace
> overriding.

Done. Lexer for backslash commands is moved out of SQL lexer as a
standalone lexer. Finally the SQL lexer left behind no longer is
aware of VariableSpace and simply used from pgbench by linking
the object file in psql directory. Addition to that, PGconn
required by escape_variable() so the core of the escape feature
is moved out into callback function side.

> standard_strings() checks standard_comforming_strins on the
> session. This is necessarily called on parsing so it can be moved
> out into PsqlScanState.

Such behavior is not compatible with the current behavior. So I
made starndard_strings() a callback, too.

> psql_error() redirects messages according to queryFout and adds
> input file information. They are heavily dependent to psql. So
> I'd like to make them a callback in PsqlScanState and do fprintf
> as the default behavior (=NULL).

Done.

> These measures will get rid of the ugliness. What do you think
> about this?
>
> > Just my 0.02€:
> >
> > I understand that you suggest some kind of dynamic
> > parametrization... this is harder to do and potentially as fragile as
> > the link/ifdef option, with an undertermined set of callbacks to
> > provide... the generic version would be harder to debug, and this
> > approach would prevent changing lexer options. Basically I'm not sure
> > that doing all that for improving the handling of pgbench scripts is
> > worth the effort. I would go with the simpler option.
>
> Undetermined set of callbacks would do so but it seems to me a
> set of few known functions to deal with as shown above. The
> shared lexer deals only with SQL and a backslash at the top of a
> command.

Finally, PsqlScanState has four callback funcions and all pgbench
needs to do to use it is setting NULL to all of them and link the
object file in psql directory. No link switch/ifdef are necessary.

| const char get_variable(const char *, bool escape, bool as_ident, void (**free_fn)(void *));
| int enc_mblen(const char *);
| bool standard_strings(void);
| void error_out(const char *fmt, ...)

The attached patches are the following.

- 0001-Prepare-for-sharing-psqlscan-with-pgbench.patch

This diff looks a bit large but most of them is cut'n-paste
work and the substantial change is rather small.

This refactors psqlscan.l into two .l files. The additional
psqlscan_slash.l is a bit tricky in the point that recreating
scan_state on transition between psqlscan.l.

.c files generated from .c are no longer complied as a part of
mainloop.c. Thay have dedicated envelope .c files so that the
sql lexer can easily used outside psql.

- 0002-Change-the-access-method-to-shell-variables.patch

Detaches VariableSpace from psqlscan.

- 0003-Detach-common.c-from-psqlscan.patch

Detaches PGconn from psqlscan by throwing out the session
encoding stuff by ading two callbacks.

- 0004-pgbench-uses-common-frontend-SQL-parser.patch

This looks larger than what actually it does because an if
branch with a large block in process_commands() was removed.
'git diff -w' shows far small, substantial changes.

- 0005-Change-the-way-to-hold-command-list.patch

In the changes in 0004, SQL multistatement is passed as a
linked list to the caller, then the caller assigns them on an
array with the same data type. This patch changes the way to
hold commands entirely to linked list.

--
Kyotaro Horiguchi
NTT Open Source Software Center