Date: Sun, 8 Apr 2012 16:25:01 +0200
From: Michael Meskes <meskes@postgresql.org>
To: Noah Misch <noah@leadboat.com>
Cc: Michael Meskes <meskes@postgresql.org>,
	Boszormenyi Zoltan <zb@cybertec.at>, Robert Haas <robertmhaas@gmail.com>,
	PG Hackers <pgsql-hackers@postgresql.org>,
	Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>,
	Bruce Momjian <bruce@momjian.us>
Subject: Re: ECPG FETCH readahead
Message-ID: <20120408142501.GA21513@feivel.credativ.lan>
Mail-Followup-To: Noah Misch <noah@leadboat.com>,
	Michael Meskes <meskes@postgresql.org>,
	Boszormenyi Zoltan <zb@cybertec.at>,
	Robert Haas <robertmhaas@gmail.com>,
	PG Hackers <pgsql-hackers@postgresql.org>,
	Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>,
	Bruce Momjian <bruce@momjian.us>
References: <4F55A9AD.6050600@cybertec.at>
	<20120306110658.GC15988@tornado.leadboat.com>
	<CA+TgmoaBGLpW5V3nFES-McLbZyY_9nUvpnFLjT7zAYuVPu809A@mail.gmail.com>
	<4F6D9893.3030300@cybertec.at>
	<20120329004323.GA17329@tornado.leadboat.com>
	<4F74409C.3050904@cybertec.at>
	<20120329170341.GA4142@tornado.leadboat.com>
	<4F74E6A7.8040204@cybertec.at>
	<20120407112008.GA2286@feivel.credativ.lan>
	<20120407155042.GB11987@tornado.leadboat.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <20120407155042.GB11987@tornado.leadboat.com>
User-Agent: Mutt/1.5.21 (2010-09-15)
Content-Transfer-Encoding: quoted-printable

On Sat, Apr 07, 2012 at 11:50:42AM -0400, Noah Misch wrote:
> Both.  The second patch appeared after my first review, based on a comm=
ent in
> that review.  I looked at it during my re-review before marking the ove=
rall
> project Ready for Committer.

Thanks.

> I do call your attention to a question I raised in my second review: if=
 a
> program contains "DECLARE foo READAHEAD 5 CURSOR FOR ..." and the user =
runs
> the program with ECPGFETCHSZ=3D10 in the environment, should that curso=
r use a
> readahead window of 5 or of 10?  Original commentary:
> http://archives.postgresql.org/message-id/20120329004323.GA17329@tornad=
o.leadboat.com

I'd say it should be 5. I don't like an environment variable overwriting =
a
hard-coded setting. I think this is what you, Noah, thought, too, right? =
I'd
say let's change this. Is it possible to allow just READAHEAD without a n=
umber?
In that case I would accept the environment variable.

And some comments mostly directed at Zoltan:

ecpg --help says ...default 0 (disabled)..., but options less than 1 are =
not
accepted and the default setting of 1 has a comment "Disabled by default"=
. I
guess this needs to be adjusted.

Is there a reason why two new options for ecpg were invented? Normally ec=
pg
options define how the preprocessor works but not the resulting binary. W=
ell,
different preprocessor behaviour might result in different binary behavio=
ur of
course. The only option that only effects the resulting binary is "-r" fo=
r
"runtime". Again, this is not completely true as the option has to make i=
ts way
into the binary, but that's it. Now I wonder whether it would make more s=
ense
to add the two options as runtime options instead. The
--detect-cursor-resultset-size option should work there without a problem=
. I
haven't delved into the source code enough to find out if -R changes some=
thing
in the compiler stage.

The test case cursor-readahead.pgc has a comment saying "test automatic p=
repare
for all statements". Copy/Paste error?

I cannot find a test that tests the environment variable giving the fetch=
 size.
Could you please point me to that?

Michael=20
--=20
Michael Meskes
Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org)
Michael at BorussiaFan dot De, Meskes at (Debian|Postgresql) dot Org
Jabber: michael.meskes at googlemail dot com
VfL Borussia! For=C3=A7a Bar=C3=A7a! Go SF 49ers! Use Debian GNU/Linux, P=
ostgreSQL