On Thu, Jan 7, 2016 at 3:36 AM, Kyotaro HORIGUCHI <horiguchi.kyot...@lab.ntt.co.jp> wrote: > - 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.
I've looked at this patch a few times now but find it rather hard to verify. I am wondering if you would be willing to separate 0001 into subpatches. For example, maybe there could be one or two patches that ONLY move code around and then one or more patches that make the changes to that code. Right now, for example, psql_scan_setup() is getting three additional arguments, but it's also being moved to a different file. Perhaps those two things could be done one at a time. I also think this patch could really benefit from a detailed set of submission notes that specifically lay out why each change was made and why. For instance, I see that psqlscan.l used yyless() while psqlscanbody.l uses a new my_yyless() you've defined. There is probably a great reason for that and I'm sure if I stare at this for long enough I can figure out what that reason is, but it would be better if you had a list of bullet points explaining what was changed and why. I would really like to see this patch committed; my problem is that I don't have enough braincells to be sure that it's correct in the present form. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
