On Thu, Feb 18, 2016 at 6:54 AM, Kyotaro HORIGUCHI <
horiguchi.kyot...@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? 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;
in fact, they say that's psqlscan.h. psqlscan_slash.h has the same
problem; perhaps moving things there could be another preliminary 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?
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
