Horton

Re: Review: ECPG FETCH readahead

Antonin Houska <antonin.houska@gmail.com>

View thread
From: Antonin Houska <antonin.houska@gmail.com>
To: Boszormenyi Zoltan <zboszor@pr.hu>, PG Hackers <pgsql-hackers@postgresql.org>
Cc: Michael Meskes <meskes@postgresql.org>, Noah Misch <noah@leadboat.com>, Peter Eisentraut <peter_e@gmx.net>, Robert Haas <robertmhaas@gmail.com>, Heikki Linnakangas <hlinnakangas@vmware.com>, Bruce Momjian <bruce@momjian.us>, Hans-Jürgen Schönig <hs@cybertec.at>
Date: 2014-04-23T22:06:48Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Fix some "translator:" comments mangled by pgindent

  2. Make sure float4in/float8in accept all standard spellings of "infinity".

[Now I'm only replying where my explanation seems useful. If you expect
anything else, please remind me.]

On 04/23/2014 06:41 PM, Boszormenyi Zoltan wrote:
> 
> All exported ECPG functions returns bool. IIRC the code generated by
> "EXEC SQL WHENEVER <something-else-than-CONTINUE>" makes use
> of the returned value.

ok

>>
>> 26.patch (the READAHEAD feature itself)
>> ---------------------------------------
>> Maybe just the arguments and return value of
>> the ecpglib functinons (ECPGopen, ECPGopen, ECPGfetch, ...) deserve a
>> bit more attention.
> 
> What do you mean exactly?

Basically the missing description of return type was most blatant, but
you explained it above. Now that I see some of the existing library
functions, the descriptions of parameters are neither too eloquent. So
ignore this remark.

>>
>> * sql-cursor-ra-fetch.stderr
>>
>> [NO_PID]: ecpg_execute on line 169: query: move forward all in
>> scroll_cur; with 0 parameter(s) on connection regress1
>> ...
>> [NO_PID]: ecpg_execute on line 169: query: move relative -3 in
>> scroll_cur; with 0 parameter(s) on
>>
>> As the first iteration is special anyway, wouldn't "move absolute -3" be
>> more efficient than the existing 2 commands?
> 
> The caching code tries to do the correct thing whichever direction
> the cursor is scanned. AFAIR this one explicitly tests invalidating
> the readahead window if you fall off it by using MOVE FORWARD ALL.

I have no doubt about correctness of the logic, just suspect that a
single MOVE command could do the action. Perhaps consider it my paranoia
and let committer judge if it's worth a change (especially if the
related amount of coding would seem inadequate).

>>
>> Other
>> -----
>>
>> Besides the individual parts I propose some typo fixes and
>> improvements in wording:
>>
>>
>> * doc/src/sgml/ecpg.sgml

In general, I'm not English native speaker, can be wrong in some cases.
Just pointed out what I thought is worth checking.

// Tony