Re: pgbench - allow backslash-continuations in custom scripts - Mailing list pgsql-hackers

Thank you for the comment, but could you please tell me what kind
of criteria should I take to split this patch? The discussion
about splitting criteria is in the following reply (in the
sentence begins with "By the way").

At Wed, 16 Mar 2016 11:57:25 -0400, Robert Haas <robertmhaas@gmail.com> wrote in
<CA+Tgmobvu1aBRdRaKvqMVp0ifhQpgvnOEZa2Rg3AHfRWPE5-Tg@mail.gmail.com>
> On Thu, Feb 18, 2016 at 6:54 AM, Kyotaro HORIGUCHI <
> horiguchi.kyotaro@lab.ntt.co.jp> wrote:
> 
> > It is the SQL part of old psqlscan.l but the difference between
> > them is a bit bothersome to see. I attached the diff between them
> > as "psqlscanbody.l.diff" for convenience.
> >
> 
> This is a huge diff, and I don't see that you've explained the reason for
> all the changes.  For example:
> 
> -/*
> - * We use a stack of flex buffers to handle substitution of psql variables.
> - * Each stacked buffer contains the as-yet-unread text from one psql
> variable.
> - * When we pop the stack all the way, we resume reading from the outer
> buffer
> - * identified by scanbufhandle.
> - */
> -typedef struct StackElem
> -{
> -       YY_BUFFER_STATE buf;            /* flex input control structure */
> -       char       *bufstring;          /* data actually being scanned by
> flex *
> /
> -       char       *origstring;         /* copy of original data, if needed
> */
> -       char       *varname;            /* name of variable providing data,
> or N
> ULL */
> -       struct StackElem *next;
> -} StackElem;
> 
> Perhaps we could separate this part of the code motion into its own
> preliminary patch?

The "preliminary patch" seems to mean the same thing with the
first patch in the following message.

http://www.postgresql.org/message-id/20160107.173603.31865003.horiguchi.kyotaro@lab.ntt.co.jp

The commit log says as the following.

| Subject: [PATCH 1/5] Prepare for sharing psqlscan with pgbench.
| 
| Lexer is no longer compiled as a part of mainloop.c.  The slash
| command lexer is brought out from the command line lexer.  psql_scan
| no longer accesses directly to pset struct and VariableSpace. This
| change allows psqlscan to be used without these things.

The following two patches are the follwings.

| Subject: [PATCH 2/5] Change the access method to shell variables
| 
| Access to shell variables via a callback function so that the lexer no
| longer need to be aware of VariableSpace.

| Subject: [PATCH 3/5] Detach common.c from psqlscan
| 
| Call standard_strings() and psql_error() via callback functions so
| that psqlscan.l can live totally without common.c stuff. They are
| bundled up with get_variable() callback in one struct since now we
| have as many as four callback functions.

These patches are made so as to keep the compilable and workable
state of the source files. It might be a bit more readable if
unshackled from such restriction.

>  I see this went to psqlscan_int.h, but there's no
> obvious reason for that particular name, and the comments don't explain it;

I assumed that is a convention of naming by looking libpq-int.h
(though it doesn't use underscore, but hyphen). But the file
needs a comment like libpq-int.h. I'll rename it and add such
comment to it.

By the way, the patch set mentioned above gives such preliminary
steps separately. Should I make the next patch set based on the
older one which is separating the preliminary steps? Or in new
splitting criteria that is free from the compilable-workable
restriction is preferable?

> in fact, they say that's psqlscan.h.  psqlscan_slash.h has the same
> problem; perhaps moving things there could be another preliminary patch.

It is also included in the 0001 patch.

> -                                       yyless(0);
> +                                       my_yyless(0);
> 
> Why do we need to do this?  Is "my_" really the best prefix?  Is this
> another change that could be its own patch?

Oops! Sorry for the silly name. I was not able to think up a
proper name for it. Does psqlscan_yyless seems good?

regards,

-- 
Kyotaro Horiguchi
NTT Open Source Software Center