Re: Allow GiST opcalsses without compress\decompres functions

From: Andrew Borodin <borodin(at)octonica(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Allow GiST opcalsses without compress\decompres functions
Date: 2017-05-28 18:06:04
Message-ID: CAJEAwVH0ayF6tOyGZ3X9yFz7ZhS7hyaXhGLR-Yn=4gt9pJU+sQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Thanks, Tom!

2017-05-28 22:22 GMT+05:00 Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>:
> Andrew Borodin <borodin(at)octonica(dot)com> writes:
>> Maybe we should make compress\decompress functions optional?
>
> 1. You'll need to adjust the documentation (gist.sgml) not just the code.
Sure, I'll check out where compression is mentioned and update docs.

> 2. If compress/decompress are omitted, then we could support index-only
> scans all the time, that is a no-op fetch function would work. The
> patch should cover that interaction too.
I do not think so. Decompress have to get att to the state where
consistency function can understand it. In theory. I've checked like a
dozen of types and have not found places where fetch should differ
from decompress.

> 3. Personally I wouldn't bother with the separate compressed[] flags,
> just look at OidIsValid(giststate->compressFn[i].fn_oid).
That would save few bytes, but make compress\decompress code slightly
harder to read. Though some comments will help.

> 4. I don't think you thought very carefully about the combinations
> where only one of the two functions is supplied. I can definitely
> see that compress + no decompress could be sensible. Less sure
> about the other case, but why not allow it? We could just say that
> an omitted function is taken to represent a no-op transformation.
Well, I thought only about the exclusion of this situation(when only
one function is supplied). But, Indeed, I've seen many opclasses where
compress was doing a work (like simplifying the data) while decompress
is just NOP.
I'll think more about it.
decompress-only opclass seems like having zero sense in adjusting
tuple on internal page. We decompress att, update it, then store it
back. Then, once upon a time, decompress it again. Looks odd. But this
does not look like a place where opcalss developer can introduce new
bugs, so I do not see reasons to forbid having only decompress
function.

> Please add this to the next commitfest.

OK.
Thank you for your comments. I'll address them and put a patch to the
commitfest.

Best regards, Andrey Borodin.

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2017-05-28 18:15:28 Re: Allow GiST opcalsses without compress\decompres functions
Previous Message Tom Lane 2017-05-28 18:03:26 Re: PostgreSQL 10 changes in exclusion constraints - did something change? CASE WHEN behavior oddity