Re: pgbench - allow backslash-continuations in custom scripts

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

In response to

Responses

Browse pgsql-hackers by date

  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