Re: Allow GiST opcalsses without compress\decompres functions

From: Alexander Korotkov <a(dot)korotkov(at)postgrespro(dot)ru>
To: Dmitriy Sarafannikov <dsarafannikov(at)yandex(dot)ru>
Cc: pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>, Andrey Borodin <x4mmm(at)yandex-team(dot)ru>
Subject: Re: Allow GiST opcalsses without compress\decompres functions
Date: 2017-09-14 13:42:38
Message-ID: CAPpHfduKArEeOkyN+C2EaRPGWrD+0JcJCSC1SqpCteqTMtU=QA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Thu, Sep 14, 2017 at 2:20 PM, Dmitriy Sarafannikov <
dsarafannikov(at)yandex(dot)ru> wrote:

> The following review has been posted through the commitfest application:
> make installcheck-world: tested, passed
> Implements feature: tested, passed
> Spec compliant: tested, passed
> Documentation: tested, passed
>
> This is simple and intuitive patch. Code looks pretty clear and well
> documented.
>
> I think it is good idea to decrease overhead on calling fake
> compressing/decomressing
> functions. This eliminates the need to implement the fetch function in
> such cases.
>
> I also tried to disable compress and decompress functions in contrib/cube:
> diff --git a/contrib/cube/cube--1.2.sql b/contrib/cube/cube--1.2.sql
> index dea2614888..26301b71d7 100644
> --- a/contrib/cube/cube--1.2.sql
> +++ b/contrib/cube/cube--1.2.sql
> @@ -370,8 +370,6 @@ CREATE OPERATOR CLASS gist_cube_ops
>
> FUNCTION 1 g_cube_consistent (internal, cube,
> smallint, oid, internal),
> FUNCTION 2 g_cube_union (internal, internal),
> - FUNCTION 3 g_cube_compress (internal),
> - FUNCTION 4 g_cube_decompress (internal),
> FUNCTION 5 g_cube_penalty (internal, internal,
> internal),
> FUNCTION 6 g_cube_picksplit (internal, internal),
> FUNCTION 7 g_cube_same (cube, cube, internal),
>
> And it is enables IndexOnlyScan, this is expected behaviour.
> + -- test explain
> + set enable_seqscan to 'off';
> + set enable_bitmapscan to 'off';
> + select count(*) from test_cube where c && '(3000,1000),(0,0)';
> + count
> + -------
> + 5
> + (1 row)
> +
> + explain analyze select c from test_cube where c && '(3000,1000),(0,0)';
> + QUERY PLAN
> + ------------------------------------------------------------
> --------------------------------------------------------------------
> + Index Only Scan using test_cube_ix on test_cube (cost=0.15..56.43
> rows=16 width=32) (actual time=0.015..0.018 rows=5 loops=1)
> + Index Cond: (c && '(3000, 1000),(0, 0)'::cube)
> + Heap Fetches: 5
> + Planning time: 0.038 ms
> + Execution time: 0.032 ms
> + (5 rows)
>
> The new status of this patch is: Ready for Committer

It would be good if someone would write patch for removing useless
compress/decompress methods from builtin and contrib GiST opclasses.
Especially when it gives benefit in IndexOnlyScan enabling.

BTW, this change in the patch look suspicious for me.

> @@ -913,7 +931,6 @@ gistproperty(Oid index_oid, int attno,
> ReleaseSysCache(tuple);
>
> /* Now look up the opclass family and input datatype. */
> -
> tuple = SearchSysCache1(CLAOID, ObjectIdGetDatum(opclass));
> if (!HeapTupleIsValid(tuple))
> {

We are typically evade changing formatting in fragments of codes not
directly touched by the patch.

------
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Craig Ringer 2017-09-14 14:00:54 Re: [PATCH] pageinspect function to decode infomasks
Previous Message Alvaro Herrera 2017-09-14 13:17:50 Re: psql: new help related to variables are not too readable