Hello, thank you for reviewing this.

> 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 tried to split it into patches with some meaningful (I thought)
steps, but I'll arrange them if it is not easy to read.

> 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'm sorry, but I didn't understood the 'submission notes' exactly
means. Is it precise descriptions in source comments? or commit
message of git-commit?

> 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.

Thank you. I'll send the rearranged patch sooner.


Kyotaro Horiguchi
NTT Open Source Software Center

Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:

Reply via email to