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
- * When we pop the stack all the way, we resume reading from the outer
- * 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

Reply via email to