Re: [HACKERS] Allow GiST opcalsses without compress\decompres functions
> 22 окт. 2017 г., в 21:21, Tom Lane написал(а): > > Andrey Borodin writes: >> I was looking for a way to correctly drop compress\decompress functions from >> opclasses. > > Making a new opclass seems like a pretty grotty answer; it won't > help existing installations. Unfortunately in cube's case that's the only option: cube allows 100-dimensional cubes, while 94-dimensional cubes could be toasted (according to code). > I think what you need is to undo opclasscmds.c's decision that the > dependencies should be INTERNAL. I tried this: Thanks Tom, that's exactly what I need. I'll push patches with this to November commitfest. Best regards, Andrey Borodin. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Allow GiST opcalsses without compress\decompres functions
Andrey Borodin writes: > I was looking for a way to correctly drop compress\decompress functions from > opclasses. Making a new opclass seems like a pretty grotty answer; it won't help existing installations. I think what you need is to undo opclasscmds.c's decision that the dependencies should be INTERNAL. I tried this: regression=# ALTER OPERATOR FAMILY gist_seg_ops USING gist drop function 3 (seg); ERROR: cannot drop function 3 (seg, seg) of operator family gist_seg_ops for access method gist: gseg_compress(internal) because operator class gist_seg_ops for access method gist requires it regression=# update pg_depend set deptype = 'a' where classid = 'pg_amproc'::regclass and objid = (select objid from pg_depend where classid = 'pg_amproc'::regclass and refclassid = 'pg_proc'::regclass and refobjid = 'gseg_compress(internal)'::regprocedure) and refclassid = 'pg_opclass'::regclass and deptype = 'i'; UPDATE 1 regression=# ALTER OPERATOR FAMILY gist_seg_ops USING gist drop function 3 (seg); ALTER OPERATOR FAMILY For safety, that update requires a bunch more pg_catalog schema-qualification than I bothered with, but you get the idea. I'm not sure if needing this hack indicates that we need more ALTER OPERATOR CLASS/FAMILY syntax to provide a less hacky way of solving the problem. The idea of removing core entries of an opclass has never come up before, and I'm not sure it would ever come up again. If we come across another use-case maybe then would be the time to fix it more cleanly. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Allow GiST opcalsses without compress\decompres functions
Hi, Tom! > 20 сент. 2017 г., в 8:38, Tom Lane написал(а): > > Andrey Borodin writes: >> [ 0001-Allow-uncompressed-GiST-4.patch ] > > Pushed, with a bit more work on the documentation and some minor > cosmetic changes. > > I did not like the fact that the new code paths added by the patch > were untested, so I went ahead and removed the no-longer-needed > no-op functions in the core GiST opclasses. There's still room > to improve the contrib opclasses, but that's much more tedious > (you need to write an extension update script) so I didn't feel > like messing with those now. I was looking for a way to correctly drop compress\decompress functions from opclasses. There are two cases: when the functions do something unnecessary (like TOASTing) and when they are just no-ops. First case occurs, for example, in cube. Please see patch attached. There is no DROP DEFAULT and I'm doing UPDATE pg_opclass SET opcdefault = FALSE WHERE opcname = 'gist_cube_ops'; I'm not sure it is correct way, but I haven't found any other workaround. Then I just create new default opclass without compress\decompress. Second case, for example in seg. I have tried alter operator family gist_seg_ops using gist drop function 3 (seg); It does not work: dependency between opclass and method is DEPENDENCY_INTERNAL and command is refused. Should I create new opclass as with cube or, may be, I can delete functions from system catalog? Best regards, Andrey Borodin. 0001-Create-cube-opclass-without-compress-decompress.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Allow GiST opcalsses without compress\decompres functions
On Wed, Sep 20, 2017 at 5:02 PM, Tom Lane wrote: > Alexander Korotkov writes: > > On Wed, Sep 20, 2017 at 4:25 PM, Tom Lane wrote: > >> Yes. We don't allow out-of-line values, but we do allow compressed and > >> short-header values. > > > BTW, this comment looks still invalid for me... > > >> #define SIGLENINT 4 /* >122 => key will *toast*, so very slow!!! */ > > I haven't done the math to see if 122 is exactly the right value, > but at some point index_form_tuple would decide that the tuple was > uncomfortably big and try to compress it. So the comment is right > in general terms. > If comment means toast as compression, not out-of-line storage then it's true. However, it would be good to rephrase this comment to clarify that. -- Alexander Korotkov Postgres Professional: http://www.postgrespro.com The Russian Postgres Company
Re: [HACKERS] Allow GiST opcalsses without compress\decompres functions
Alexander Korotkov writes: > On Wed, Sep 20, 2017 at 4:25 PM, Tom Lane wrote: >> Yes. We don't allow out-of-line values, but we do allow compressed and >> short-header values. > BTW, this comment looks still invalid for me... >> #define SIGLENINT 4 /* >122 => key will *toast*, so very slow!!! */ I haven't done the math to see if 122 is exactly the right value, but at some point index_form_tuple would decide that the tuple was uncomfortably big and try to compress it. So the comment is right in general terms. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Allow GiST opcalsses without compress\decompres functions
On Wed, Sep 20, 2017 at 4:25 PM, Tom Lane wrote: > Andrey Borodin writes: > > You mentioned that probably there cannot be TOASTed values in the index. > > I need to settle this question in more deterministic way. > > Can you point where to look at the code or who to ask: > > Can there be TOASTed values in index tuples? > > Yes. We don't allow out-of-line values, but we do allow compressed and > short-header values. > > If you don't believe me, look at index_form_tuple(). > OK. So GiST opclass should either implement decompress method with detoasting or detoast every input key in all other methods. BTW, this comment looks still invalid for me... > #define SIGLENINT 4 /* >122 => key will *toast*, so very slow!!! */ -- Alexander Korotkov Postgres Professional: http://www.postgrespro.com The Russian Postgres Company
Re: [HACKERS] Allow GiST opcalsses without compress\decompres functions
Andrey Borodin writes: > You mentioned that probably there cannot be TOASTed values in the index. > I need to settle this question in more deterministic way. > Can you point where to look at the code or who to ask: > Can there be TOASTed values in index tuples? Yes. We don't allow out-of-line values, but we do allow compressed and short-header values. If you don't believe me, look at index_form_tuple(). regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Allow GiST opcalsses without compress\decompres functions
On Wed, Sep 20, 2017 at 12:43 PM, Andrey Borodin wrote: > Hello Tom! Thanks for committing the patch! > > > 20 сент. 2017 г., в 8:38, Tom Lane написал(а): > > > > Andrey Borodin writes: > >> [ 0001-Allow-uncompressed-GiST-4.patch ] > > > > ... There's still room > > to improve the contrib opclasses > > I have one important question regarding compress\decompres functions. > > You mentioned that probably there cannot be TOASTed values in the index. > I need to settle this question in more deterministic way. > Can you point where to look at the code or who to ask: > Can there be TOASTed values in index tuples? > > If answer is NO, we can get rid of much more useless code. > And get rid of misleading comments too. For instance, this comment in hstore_gist.c seems misleading for me. #define SIGLENINT 4 /* >122 => key will *toast*, so very slow!!! */ -- Alexander Korotkov Postgres Professional: http://www.postgrespro.com The Russian Postgres Company
Re: [HACKERS] Allow GiST opcalsses without compress\decompres functions
Hello Tom! Thanks for committing the patch! > 20 сент. 2017 г., в 8:38, Tom Lane написал(а): > > Andrey Borodin writes: >> [ 0001-Allow-uncompressed-GiST-4.patch ] > > ... There's still room > to improve the contrib opclasses I have one important question regarding compress\decompres functions. You mentioned that probably there cannot be TOASTed values in the index. I need to settle this question in more deterministic way. Can you point where to look at the code or who to ask: Can there be TOASTed values in index tuples? If answer is NO, we can get rid of much more useless code. Best regards, Andrey Borodin. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Allow GiST opcalsses without compress\decompres functions
Andrey Borodin writes: > [ 0001-Allow-uncompressed-GiST-4.patch ] Pushed, with a bit more work on the documentation and some minor cosmetic changes. I did not like the fact that the new code paths added by the patch were untested, so I went ahead and removed the no-longer-needed no-op functions in the core GiST opclasses. There's still room to improve the contrib opclasses, but that's much more tedious (you need to write an extension update script) so I didn't feel like messing with those now. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Allow GiST opcalsses without compress\decompres functions
On Fri, Sep 15, 2017 at 3:36 PM, Andrey Borodin wrote: > > 14 сент. 2017 г., в 18:42, Alexander Korotkov > написал(а): > > 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. > If the patch will be accepted, I'll either do this myself for commitfest > at November or charge some students from Yandex Data School to do this > (they earn some points in algorithms course for contributing to OSS). > Great! I'm looking forward see their patches on commitfest! -- Alexander Korotkov Postgres Professional: http://www.postgrespro.com The Russian Postgres Company
Re: [HACKERS] Allow GiST opcalsses without compress\decompres functions
Dmitry and Alexander, thank you for looking into the patch! > 14 сент. 2017 г., в 18:42, Alexander Korotkov > написал(а): > 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. If the patch will be accepted, I'll either do this myself for commitfest at November or charge some students from Yandex Data School to do this (they earn some points in algorithms course for contributing to OSS). > BTW, this change in the patch look suspicious for me > > We are typically evade changing formatting in fragments of codes not directly > touched by the patch. Thanks for spotting this out. Here's fixed version. Best regards, Andrey Borodin. 0001-Allow-uncompressed-GiST-4.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Allow GiST opcalsses without compress\decompres functions
On Thu, Sep 14, 2017 at 2:20 PM, Dmitriy Sarafannikov < dsarafanni...@yandex.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 > > FUNCTION1 g_cube_consistent (internal, cube, > smallint, oid, internal), > FUNCTION2 g_cube_union (internal, internal), > - FUNCTION3 g_cube_compress (internal), > - FUNCTION4 g_cube_decompress (internal), > FUNCTION5 g_cube_penalty (internal, internal, > internal), > FUNCTION6 g_cube_picksplit (internal, internal), > FUNCTION7 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
Re: [HACKERS] Allow GiST opcalsses without compress\decompres functions
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 FUNCTION1 g_cube_consistent (internal, cube, smallint, oid, internal), FUNCTION2 g_cube_union (internal, internal), - FUNCTION3 g_cube_compress (internal), - FUNCTION4 g_cube_decompress (internal), FUNCTION5 g_cube_penalty (internal, internal, internal), FUNCTION6 g_cube_picksplit (internal, internal), FUNCTION7 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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Allow GiST opcalsses without compress\decompres functions
> 11 сент. 2017 г., в 12:57, Dmitriy Sarafannikov > написал(а): > Hi Andrew! Thanks for the patch, but patch > 0001-allow-uncompressed-Gist-2.patch no longer applies on current master > branch. > Please could you rebase it? Sure, see attachment. Thanks for looking into the patch! Best regards, Andrey Borodin. 0001-Allow-uncompressed-GiST-3.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Allow GiST opcalsses without compress\decompres functions
The following review has been posted through the commitfest application: make installcheck-world: not tested Implements feature: not tested Spec compliant: not tested Documentation:not tested Hi Andrew! Thanks for the patch, but patch 0001-allow-uncompressed-Gist-2.patch no longer applies on current master branch. Please could you rebase it? Some info from git apply command: Checking patch doc/src/sgml/gist.sgml... Checking patch src/backend/access/gist/gist.c... Checking patch src/backend/access/gist/gistget.c... Checking patch src/backend/access/gist/gistutil.c... error: while searching for: GISTENTRY *dep; gistentryinit(*e, k, r, pg, o, l); dep = (GISTENTRY *) DatumGetPointer(FunctionCall1Coll(&giststate->decompressFn[nkey], giststate->supportCollation[nkey], error: patch failed: src/backend/access/gist/gistutil.c:550 error: while searching for: gistentryinit(centry, attdata[i], r, NULL, (OffsetNumber) 0, isleaf); cep = (GISTENTRY *) DatumGetPointer(FunctionCall1Coll(&giststate->compressFn[i], giststate->supportCollation[i], PointerGetDatum(¢ry))); compatt[i] = cep->key; } } error: patch failed: src/backend/access/gist/gistutil.c:585 Hunk #3 succeeded at 648 (offset -7 lines). Hunk #4 succeeded at 924 (offset -7 lines). Hunk #5 succeeded at 944 (offset -7 lines). Checking patch src/backend/access/gist/gistvalidate.c... Applied patch doc/src/sgml/gist.sgml cleanly. Applied patch src/backend/access/gist/gist.c cleanly. Applied patch src/backend/access/gist/gistget.c cleanly. Applying patch src/backend/access/gist/gistutil.c with 2 rejects... Rejected hunk #1. Rejected hunk #2. Hunk #3 applied cleanly. Hunk #4 applied cleanly. Hunk #5 applied cleanly. Applied patch src/backend/access/gist/gistvalidate.c cleanly. The new status of this patch is: Waiting on Author -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Allow GiST opcalsses without compress\decompres functions
2017-05-29 0:00 GMT+05:00 Tom Lane : > But the opclass's consistent function might expect to be always given an > untoasted input, in which case you'd need the decompress function to fix > that up. In cube data is detoasted at least 4 times before going to g_cube_internal_consistent(), including once in g_cube_consistent() But I'll address that in another patch. Here's new version of the patch, with: 1. Corrected docs 2. Any combination of dompress\decompress\fetch is allowed 3. (Existence of fetch) or (absence of compress) leads to IOS The last point seems counterintuitive, but I do not see anything wrong. Best regards, Andrey Borodin, Octonica. 0001-Allow-uncompressed-GiST-2.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Allow GiST opcalsses without compress\decompres functions
Andrew Borodin writes: > I'm not expert in toasting, cube's compress does nothing while cube > decomress does detoasting. Is this for reason? The original input datum could be toasted --- at least to the extent of being compressed in-line or having short header; I do not think we allow out-of-line storage in an index. This is OK for storage within the index, and it would also be OK for an IOS to return the value in that form. But the opclass's consistent function might expect to be always given an untoasted input, in which case you'd need the decompress function to fix that up. Mind you, I'm not saying that this would represent good design; datatype-specific functions that expect somebody else to have detoasted their input seem pretty fragile. But it might be how cube's GIST support works. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Allow GiST opcalsses without compress\decompres functions
28 мая 2017 г. 11:15 PM пользователь "Tom Lane" написал: Andrew Borodin writes: > 2017-05-28 22:22 GMT+05:00 Tom Lane : >> 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. But if compress is omitted, then we know the in-index representation is the same as the original. Therefore the fetch function would always be a no-op and we can implement IOS whether or not the opclass designer bothered to supply one. regards, tom lane True. If there is no code to "hash" original value, it is not hashed, it's whole... I'm not expert in toasting, cube's compress does nothing while cube decomress does detoasting. Is this for reason? Best regards, Andrey Borodin.
Re: [HACKERS] Allow GiST opcalsses without compress\decompres functions
Andrew Borodin writes: > 2017-05-28 22:22 GMT+05:00 Tom Lane : >> 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. But if compress is omitted, then we know the in-index representation is the same as the original. Therefore the fetch function would always be a no-op and we can implement IOS whether or not the opclass designer bothered to supply one. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Allow GiST opcalsses without compress\decompres functions
Thanks, Tom! 2017-05-28 22:22 GMT+05:00 Tom Lane : > Andrew Borodin 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. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Allow GiST opcalsses without compress\decompres functions
Andrew Borodin writes: > Maybe we should make compress\decompress functions optional? 1. You'll need to adjust the documentation (gist.sgml) not just the code. 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. 3. Personally I wouldn't bother with the separate compressed[] flags, just look at OidIsValid(giststate->compressFn[i].fn_oid). 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. Please add this to the next commitfest. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Allow GiST opcalsses without compress\decompres functions
Hi, hackers! Currently, GiST stores each attribute in a compressed form. That is, each time attribute is written it's calling compress function, and when the attribute is accessed the decompress functions is called. Some types can't get any advantage out of this technique since the context of one value is not enough for seeding effective compression algorithm. And we have some of the compress functions like this Datum gist_box_compress(PG_FUNCTION_ARGS) { PG_RETURN_POINTER(PG_GETARG_POINTER(0)); } https://github.com/postgres/postgres/blob/master/src/backend/access/gist/gistproc.c#L195 https://github.com/postgres/postgres/blob/master/src/backend/utils/adt/rangetypes_gist.c#L221 https://github.com/postgres/postgres/blob/master/contrib/seg/seg.c#L255 https://github.com/postgres/postgres/blob/master/contrib/cube/cube.c#L384 Maybe we should make compress\decompress functions optional? Also, this brings some bit of performance. For the attached test I observe 6% faster GiST build and 4% faster scans. Not a big deal, but something. How do you think? In some cases, there are strange things in the code of compress\decompress. E.g. cube's decompress function is detoasting entry twice, to be very sure :) Best regards, Andrey Borodin, Octonica. CREATE EXTENSION IF NOT EXISTS CUBE ; DROP TABLE IF EXISTS TESTTABLE ; DROP OPERATOR CLASS IF EXISTS gist_cube_ops_nocompress USING GIST; CREATE OPERATOR CLASS gist_cube_ops_nocompress FOR TYPE CUBE USING GIST AS OPERATOR3 && , OPERATOR6 = , OPERATOR7 @> , OPERATOR8 <@ , OPERATOR13 @ , OPERATOR14 ~ , FUNCTION1 g_cube_consistent (INTERNAL, CUBE, SMALLINT, OID, INTERNAL), FUNCTION2 g_cube_union (INTERNAL, INTERNAL), --we do not have functions 3 and for (compress and decompress) FUNCTION5 g_cube_penalty (INTERNAL, INTERNAL, INTERNAL), FUNCTION6 g_cube_picksplit (INTERNAL, INTERNAL), FUNCTION7 g_cube_same (CUBE, CUBE, INTERNAL), FUNCTION8 g_cube_distance (INTERNAL, CUBE, SMALLINT, OID, INTERNAL), FUNCTION9 g_cube_decompress (INTERNAL);--fetch function, not for compression BEGIN; SELECT SETSEED(0.5); CREATE TABLE TESTTABLE AS SELECT CUBE(RANDOM()) C FROM GENERATE_SERIES(1,50) I; \timing --testing time to create compressed index CREATE INDEX COMPRESSED ON TESTTABLE USING GIST(C gist_cube_ops); --testing time to create uncompressed index CREATE INDEX UNCOMPRESSED ON TESTTABLE USING GIST(C gist_cube_ops_nocompress); \timing set enable_bitmapscan = false; UPDATE PG_INDEX SET INDISVALID = FALSE WHERE INDEXRELID = 'UNCOMPRESSED'::REGCLASS; UPDATE PG_INDEX SET INDISVALID = TRUE WHERE INDEXRELID = 'COMPRESSED'::REGCLASS; EXPLAIN ANALYZE select (SELECT COUNT(*) FROM TESTTABLE WHERE C <@ CUBE(-b,1)) from generate_series(1,50) b; EXPLAIN ANALYZE select (SELECT COUNT(*) FROM TESTTABLE WHERE C <@ CUBE(-b,1)) from generate_series(1,50) b; UPDATE PG_INDEX SET INDISVALID = TRUE WHERE INDEXRELID = 'UNCOMPRESSED'::REGCLASS; UPDATE PG_INDEX SET INDISVALID = FALSE WHERE INDEXRELID = 'COMPRESSED'::REGCLASS; EXPLAIN ANALYZE select (SELECT COUNT(*) FROM TESTTABLE WHERE C <@ CUBE(-b,1)) from generate_series(1,50) b; EXPLAIN ANALYZE select (SELECT COUNT(*) FROM TESTTABLE WHERE C <@ CUBE(-b,1)) from generate_series(1,50) b; COMMIT; 0001-Allow-uncompressed-GiST.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers