Thread: pgsql: Fix assorted inconsistencies in GIN opclass support function dec

Fix assorted inconsistencies in GIN opclass support function declarations.

GIN had some minor issues too, mostly using "internal" where something
else would be more appropriate.  I went with the same approach as in
9ff60273e35cad6e, namely preferring the opclass' indexed datatype for
arguments that receive an operator RHS value, even if that's not
necessarily what they really are.

Again, this is with an eye to having a uniform rule for ginvalidate()
to check support function signatures.

Branch
------
master

Details
-------
http://git.postgresql.org/pg/commitdiff/dbe2328959e12701fade6b500ad411271923d6e4

Modified Files
--------------
contrib/hstore/hstore--1.3.sql     |   12 ++++-----
contrib/intarray/intarray--1.1.sql |    8 +++---
contrib/tsearch2/tsearch2--1.0.sql |    4 +--
doc/src/sgml/gin.sgml              |   48 ++++++++++++++++++++----------------
src/include/catalog/catversion.h   |    2 +-
src/include/catalog/pg_proc.h      |   22 ++++++++---------
6 files changed, 51 insertions(+), 45 deletions(-)

Alexander Korotkov <a.korotkov@postgrespro.ru> writes:
> On Wed, Jan 20, 2016 at 6:32 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> contrib/hstore/hstore--1.3.sql     |   12 ++++-----
>> contrib/intarray/intarray--1.1.sql |    8 +++---
>> contrib/tsearch2/tsearch2--1.0.sql |    4 +--

> Hmm...  Is it correct to change function signatures without extension
> version bump?  pg_upgraded clusters would remain with old version of these
> functions.  Once we have instances with same extension version but with
> different signatures of its functions, there is no correct way to refer
> these functions in future.  I think we should do the version bump in this
> case.

It doesn't really matter, though, because there simply isn't any need to
refer to these functions from SQL.  They are only useful as opclass
support functions.

But this is likely moot anyway, because of the need to bump all the
contrib modules' versions in order to install parallel-safety labels on
their functions.  (I wonder why that isn't on the open-items list.)

            regards, tom lane

On Mon, May 2, 2016 at 5:09 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Alexander Korotkov <a.korotkov@postgrespro.ru> writes:
>> On Wed, Jan 20, 2016 at 6:32 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>> contrib/hstore/hstore--1.3.sql     |   12 ++++-----
>>> contrib/intarray/intarray--1.1.sql |    8 +++---
>>> contrib/tsearch2/tsearch2--1.0.sql |    4 +--
>
>> Hmm...  Is it correct to change function signatures without extension
>> version bump?  pg_upgraded clusters would remain with old version of these
>> functions.  Once we have instances with same extension version but with
>> different signatures of its functions, there is no correct way to refer
>> these functions in future.  I think we should do the version bump in this
>> case.
>
> It doesn't really matter, though, because there simply isn't any need to
> refer to these functions from SQL.  They are only useful as opclass
> support functions.
>
> But this is likely moot anyway, because of the need to bump all the
> contrib modules' versions in order to install parallel-safety labels on
> their functions.  (I wonder why that isn't on the open-items list.)

Because it was argued by Noah that this was 9.7 work.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On 2016-05-02 17:09:48 -0400, Tom Lane wrote:
> But this is likely moot anyway, because of the need to bump all the
> contrib modules' versions in order to install parallel-safety labels on
> their functions.  (I wonder why that isn't on the open-items list.)

I think the concensus when discussing that was that that'd essentially
be a "feature", and thus too late for 9.6.  I'm a bit doubtful about
that position, but I also don't have a strong opinion the other way round.

On 2016-05-02 17:12:41 -0400, Robert Haas wrote:
> On Mon, May 2, 2016 at 5:09 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> > Alexander Korotkov <a.korotkov@postgrespro.ru> writes:
> > But this is likely moot anyway, because of the need to bump all the
> > contrib modules' versions in order to install parallel-safety labels on
> > their functions.  (I wonder why that isn't on the open-items list.)
>
> Because it was argued by Noah that this was 9.7 work.

Thinking about this again, I think that's not a good approach: It'll
mean we've to do very similar testing in 9.7 again. I'd rather label as
much as possible in one release.

Alexander Korotkov <a.korotkov@postgrespro.ru> writes:
> On Tue, May 3, 2016 at 12:09 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> It doesn't really matter, though, because there simply isn't any need to
>> refer to these functions from SQL.

> What if we'll want to reuse some on these functions in new opclass?

What context do you imagine that happening in?  Surely no other extension
would attempt to re-use them.  If, say, hstore itself wanted to reuse
these functions, that would be in a new version of hstore--n.n.sql, so
there's no problem.

            regards, tom lane