On Mon, Apr 6, 2015 at 11:02 AM, Petr Jelinek <p...@2ndquadrant.com> wrote:

> On 06/04/15 14:30, Petr Jelinek wrote:
>> On 06/04/15 11:02, Simon Riggs wrote:
>>  Are we ready for a final detailed review and commit?
>> I plan to send v12 in the evening with some additional changes that came
>> up from Amit's comments + some improvements to error reporting. I think
>> it will be ready then.
> Ok so here it is.
> Changes vs v11:
> - changed input parameter list to expr_list
> - improved error reporting, particularly when input parameters are wrong
> - fixed SELECT docs to show correct syntax and mention that there can be
> more sampling methods
> - added name of the sampling method to the explain output - I don't like
> the code much there as it has to look into RTE but on the other hand I
> don't want to create new scan node just so it can hold the name of the
> sampling method for explain
> - made views containing TABLESAMPLE clause not autoupdatable
> - added PageIsAllVisible() check before trying to check for tuple
> visibility
> - some typo/white space fixes

Compiler warnings:
explain.c: In function 'ExplainNode':
explain.c:861: warning: 'sname' may be used uninitialized in this function

Docs spellings:

"PostgreSQL distrribution"  extra r.

"The optional parameter REPEATABLE acceps"   accepts.  But I don't know
that 'accepts' is the right word.  It makes the seed value sound optional

"each block having same chance"  should have "the" before "same".

"Both of those sampling methods currently...".  I think it should be
"these" not "those", as this sentence is immediately after their
introduction, not at a distance.

"...tuple contents and decides if to return in, or zero if none"  Something
here is confusing. "return it", not "return in"?

Other comments:

Do we want tab completions for psql?  (I will never remember how to spell

Needs a Cat version bump.



Reply via email to