Thanks, Tom!
2017-05-28 22:22 GMT+05:00 Tom Lane <tgl@sss.pgh.pa.us>:
> Andrew Borodin <borodin@octonica.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.
