From: | Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp> |
---|---|
To: | Robert Haas <robertmhaas(at)gmail(dot)com> |
Cc: | Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr>, Michael Paquier <michael(dot)paquier(at)gmail(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: pgbench - allow backslash-continuations in custom scripts |
Date: | 2016-02-15 06:04:46 |
Message-ID: | 56C16A7E.3000202@lab.ntt.co.jp |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Hello, thank you for reviewing this.
> On Thu, Jan 7, 2016 at 3:36 AM, Kyotaro HORIGUCHI
> <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)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.
regards,
--
Kyotaro Horiguchi
NTT Open Source Software Center
From | Date | Subject | |
---|---|---|---|
Next Message | Craig Ringer | 2016-02-15 06:15:29 | Re: Refectoring of receivelog.c |
Previous Message | Michael Paquier | 2016-02-15 05:54:37 | Re: Support for N synchronous standby servers - take 2 |