Horton

Thread

Commits

  1. Allow no-op GiST support functions to be omitted.

  2. Remove no-op GiST support functions in the core GiST opclasses.

  1. Allow GiST opcalsses without compress\decompres functions

    Andrew Borodin <borodin@octonica.com> — 2017-05-25T11:31:04Z

    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.

  2. Re: Allow GiST opcalsses without compress\decompres functions

    Tom Lane <tgl@sss.pgh.pa.us> — 2017-05-28T17:22:27Z

    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.

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

  3. Re: Allow GiST opcalsses without compress\decompres functions

    Andrew Borodin <borodin@octonica.com> — 2017-05-28T18:06:04Z

    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.

  4. Re: Allow GiST opcalsses without compress\decompres functions

    Tom Lane <tgl@sss.pgh.pa.us> — 2017-05-28T18:15:28Z

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

  5. Re: Allow GiST opcalsses without compress\decompres functions

    Andrew Borodin <borodin@octonica.com> — 2017-05-28T18:51:16Z

    28 мая 2017 г. 11:15 PM пользователь "Tom Lane" <tgl@sss.pgh.pa.us> написал:

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

  6. Re: Allow GiST opcalsses without compress\decompres functions

    Tom Lane <tgl@sss.pgh.pa.us> — 2017-05-28T19:00:47Z

    Andrew Borodin <borodin@octonica.com> 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

  7. Re: Allow GiST opcalsses without compress\decompres functions

    Andrew Borodin <borodin@octonica.com> — 2017-05-29T09:53:37Z

    2017-05-29 0:00 GMT+05:00 Tom Lane <tgl@sss.pgh.pa.us>:
> 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.

  8. Re: Allow GiST opcalsses without compress\decompres functions

    Dmitriy Sarafannikov <dsarafannikov@yandex.ru> — 2017-09-11T07:57:43Z

    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(&centry)));
			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

  9. Re: Allow GiST opcalsses without compress\decompres functions

    x4mmm@yandex-team.ru — 2017-09-11T10:25:15Z

    > 11 сент. 2017 г., в 12:57, Dmitriy Sarafannikov <dsarafannikov@yandex.ru> написал(а):
> 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.

  10. Re: Allow GiST opcalsses without compress\decompres functions

    Dmitriy Sarafannikov <dsarafannikov@yandex.ru> — 2017-09-14T11:20:14Z

    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

  11. Re: Allow GiST opcalsses without compress\decompres functions

    Alexander Korotkov <a.korotkov@postgrespro.ru> — 2017-09-14T13:42:38Z

    On Thu, Sep 14, 2017 at 2:20 PM, Dmitriy Sarafannikov <
dsarafannikov@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
>
>         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

  12. Re: Allow GiST opcalsses without compress\decompres functions

    x4mmm@yandex-team.ru — 2017-09-15T12:36:03Z

    Dmitry and Alexander, thank you for looking into the patch!

> 14 сент. 2017 г., в 18:42, Alexander Korotkov <a.korotkov@postgrespro.ru> написал(а):
> 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.

  13. Re: Allow GiST opcalsses without compress\decompres functions

    Alexander Korotkov <a.korotkov@postgrespro.ru> — 2017-09-15T12:53:09Z

    On Fri, Sep 15, 2017 at 3:36 PM, Andrey Borodin <x4mmm@yandex-team.ru>
wrote:

> > 14 сент. 2017 г., в 18:42, Alexander Korotkov <a.korotkov@postgrespro.ru>
> написал(а):
> > 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

  14. Re: Allow GiST opcalsses without compress\decompres functions

    Tom Lane <tgl@sss.pgh.pa.us> — 2017-09-20T03:38:30Z

    Andrey Borodin <x4mmm@yandex-team.ru> 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

  15. Re: Allow GiST opcalsses without compress\decompres functions

    x4mmm@yandex-team.ru — 2017-09-20T09:43:18Z

    Hello Tom! Thanks for committing the patch!

> 20 сент. 2017 г., в 8:38, Tom Lane <tgl@sss.pgh.pa.us> написал(а):
> 
> Andrey Borodin <x4mmm@yandex-team.ru> 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.

  16. Re: Allow GiST opcalsses without compress\decompres functions

    Alexander Korotkov <a.korotkov@postgrespro.ru> — 2017-09-20T09:58:57Z

    On Wed, Sep 20, 2017 at 12:43 PM, Andrey Borodin <x4mmm@yandex-team.ru>
wrote:

> Hello Tom! Thanks for committing the patch!
>
> > 20 сент. 2017 г., в 8:38, Tom Lane <tgl@sss.pgh.pa.us> написал(а):
> >
> > Andrey Borodin <x4mmm@yandex-team.ru> 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

  17. Re: Allow GiST opcalsses without compress\decompres functions

    Tom Lane <tgl@sss.pgh.pa.us> — 2017-09-20T13:25:12Z

    Andrey Borodin <x4mmm@yandex-team.ru> 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

  18. Re: Allow GiST opcalsses without compress\decompres functions

    Alexander Korotkov <a.korotkov@postgrespro.ru> — 2017-09-20T13:31:26Z

    On Wed, Sep 20, 2017 at 4:25 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

> Andrey Borodin <x4mmm@yandex-team.ru> 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

  19. Re: Allow GiST opcalsses without compress\decompres functions

    Tom Lane <tgl@sss.pgh.pa.us> — 2017-09-20T14:02:35Z

    Alexander Korotkov <a.korotkov@postgrespro.ru> writes:
> On Wed, Sep 20, 2017 at 4:25 PM, Tom Lane <tgl@sss.pgh.pa.us> 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

  20. Re: Allow GiST opcalsses without compress\decompres functions

    Alexander Korotkov <a.korotkov@postgrespro.ru> — 2017-09-20T14:09:38Z

    On Wed, Sep 20, 2017 at 5:02 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

> Alexander Korotkov <a.korotkov@postgrespro.ru> writes:
> > On Wed, Sep 20, 2017 at 4:25 PM, Tom Lane <tgl@sss.pgh.pa.us> 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

  21. Re: Allow GiST opcalsses without compress\decompres functions

    x4mmm@yandex-team.ru — 2017-10-22T11:48:20Z

    Hi, Tom!
> 20 сент. 2017 г., в 8:38, Tom Lane <tgl@sss.pgh.pa.us> написал(а):
> 
> Andrey Borodin <x4mmm@yandex-team.ru> 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.

  22. Re: Allow GiST opcalsses without compress\decompres functions

    Tom Lane <tgl@sss.pgh.pa.us> — 2017-10-22T16:21:44Z

    Andrey Borodin <x4mmm@yandex-team.ru> 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

  23. Re: Allow GiST opcalsses without compress\decompres functions

    x4mmm@yandex-team.ru — 2017-10-23T08:13:56Z

    > 22 окт. 2017 г., в 21:21, Tom Lane <tgl@sss.pgh.pa.us> написал(а):
> 
> Andrey Borodin <x4mmm@yandex-team.ru> 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.