Re: TABLESAMPLE patch

On 06/01/15 14:22, Petr Jelinek wrote:
> On 06/01/15 08:51, Michael Paquier wrote:
>> On Tue, Dec 23, 2014 at 5:21 AM, Petr Jelinek <petr(at)2ndquadrant(dot)com>
>> wrote:
>>> Attached is v3 which besides the fixes mentioned above also includes
>>> changes
>>> discussed with Tomas (except the CREATE/DROP TABLESAMPLE METHOD),
>>> fixes for
>>> crash with FETCH FIRST and is rebased against current master.
>> This patch needs a rebase, there is a small conflict in
>> parallel_schedule.
>>
>
> Sigh, I really wish we had automation that checks this automatically for
> patches in CF.
>

Here is rebase against current master.

>> Structurally speaking, I think that the tsm methods should be added in
>> src/backend/utils and not src/backend/access which is more high-level
>> as tsm_bernoulli.c and tsm_system.c contain only a set of new
>
> I am not sure if I parsed this correctly, do you mean to say that only
> low-level access functions belong to src/backend/access? Makes sense.

Made this change.

>
>> procedure functions. Having a single header file tsm.h would be also a
>> good thing.
>

Moved into single tablesample.h file.

>
>> Regarding the naming, is "tsm" (table sample method) really appealing?
>> Wouldn't it be better to use simply tablesample_* for the file names
>> and the method names?
>>

I created the src/backend/tablesample and files are named just system.c
and bernoulli.c, but I kept tsm_ prefix for methods as they become too
long for my taste when prefixing with tablesample_.

>> This is a large patch... Wouldn't sampling.[c|h] extracted from
>> ANALYZE live better as a refactoring patch? This would limit a bit bug
>> occurrences on the main patch.
>>
>
> That's a good idea, I'll split it into patch series.
>

I've split the sampling.c/h into separate patch.

I also wrote basic CREATE/DROP TABLESAMPLE METHOD support, again as
separate patch in the attached patch-set. This also includes modules
test with simple custom tablesample method.

There are some very minor cleanups in the main tablesample code itself
but no functional changes.

--
Petr Jelinek http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services