On Mon, Apr 6, 2015 at 11:32 PM, 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
>
New changes looks fine to me except for one typo.
+ The optional parameter <literal>REPEATABLE</literal> acceps any number
+ or expression producing a number and is used as random seed for
+ sampling.
typo
/acceps/accepts
So the patch for implementation of SYSTEM and BERNOULLI TABLESAMPLE
methods is "Ready For Committer". I could not get chance to review the
DDL patch (patch to implement user defined TABLESAMPLE methods).
Note to Committer - None of us is very clear on what should be the
implementation for Tablesample clause incase of UPDATE/DELETE
statement. I am of opinion that either we support to Update/Delete
based on Tablesample clause or prohibit it in all cases, however Peter
thinks it is okay to support for the same in FROM,USING clause of
Update, Delete respectively.