Re: TABLESAMPLE patch

Поиск
Список
Период
Сортировка
От Jeff Janes
Тема Re: TABLESAMPLE patch
Дата
Msg-id CAMkU=1wL4byNeDZieYyiwQ3q68G=oNPiYgmQf0Q8b4fZgGyTQw@mail.gmail.com
обсуждение исходный текст
Ответ на Re: TABLESAMPLE patch  (Petr Jelinek <petr@2ndquadrant.com>)
Ответы Re: TABLESAMPLE patch  (Michael Paquier <michael.paquier@gmail.com>)
Список pgsql-hackers
On Mon, Apr 6, 2015 at 11:02 AM, Petr Jelinek <petr@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 to REPEATABLE.

"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 BERNOULLI).

Needs a Cat version bump.

Cheers,

Jeff

В списке pgsql-hackers по дате отправления:

Предыдущее
От: David Steele
Дата:
Сообщение: Re: deparsing utility commands
Следующее
От: Andres Freund
Дата:
Сообщение: Re: "rejected" vs "returned with feedback" in new CF app