pgsql: Fix assorted inconsistencies in GIN opclass support function dec

Started by Tom Laneabout 10 years ago10 messageshackers
Jump to latest
#1Tom Lane
tgl@sss.pgh.pa.us

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(-)

--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers

#2Alexander Korotkov
aekorotkov@gmail.com
In reply to: Tom Lane (#1)
Re: pgsql: Fix assorted inconsistencies in GIN opclass support function dec

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.
The same for 9ff60273e35cad6e9d3a4adf59d5c2455afe9d9e.

------
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

#3Tom Lane
tgl@sss.pgh.pa.us
In reply to: Alexander Korotkov (#2)
Re: pgsql: Fix assorted inconsistencies in GIN opclass support function dec

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

--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers

#4Robert Haas
robertmhaas@gmail.com
In reply to: Tom Lane (#3)
Re: pgsql: Fix assorted inconsistencies in GIN opclass support function dec

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

--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers

#5Andres Freund
andres@anarazel.de
In reply to: Tom Lane (#3)
Re: pgsql: Fix assorted inconsistencies in GIN opclass support function dec

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.

--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers

#6Andres Freund
andres@anarazel.de
In reply to: Robert Haas (#4)
Re: pgsql: Fix assorted inconsistencies in GIN opclass support function dec

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.

--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers

#7Robert Haas
robertmhaas@gmail.com
In reply to: Andres Freund (#6)
Re: [COMMITTERS] pgsql: Fix assorted inconsistencies in GIN opclass support function dec

On Mon, May 2, 2016 at 5:15 PM, Andres Freund <andres@anarazel.de> wrote:

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.

Yeah, I somewhat agree, but there's also no patch for this, yet.

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#8Alexander Korotkov
aekorotkov@gmail.com
In reply to: Tom Lane (#3)
Re: pgsql: Fix assorted inconsistencies in GIN opclass support function dec

On Tue, May 3, 2016 at 12:09 AM, 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.

What if we'll want to reuse some on these functions in new opclass?
Assumption that we don't need to refer these functions from SQL seems
dangerous to me.
If pg_upgraded instances are OK to leave with old function definitions, why
do we bother about new instances?

------
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

#9Tom Lane
tgl@sss.pgh.pa.us
In reply to: Alexander Korotkov (#8)
Re: Re: pgsql: Fix assorted inconsistencies in GIN opclass support function dec

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

--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers

#10Alexander Korotkov
aekorotkov@gmail.com
In reply to: Tom Lane (#9)
Re: Re: pgsql: Fix assorted inconsistencies in GIN opclass support function dec

On Tue, May 3, 2016 at 4:32 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

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.

Let's consider particular example: gin_extract_hstore() function.

In PostgreSQL 9.6 hstore 1.3 it's gin_extract_hstore(hstore, internal)
RETURNS internal.
In PostgreSQL 9.5 hstore 1.3 it's gin_extract_hstore(internal, internal)
RETURNS internal.
After pg_upgrade 9.5 => 9.6 it will be still gin_extract_hstore(internal,
internal) RETURNS internal.

Thus, in 9.6 hstore 1.3 we can meet either

1. gin_extract_hstore(internal, internal) RETURNS internal
2. gin_extract_hstore(hstore, internal) RETURNS internal

Then, if we want to reuse this function in hstore 1.4, we will need to
write a migration script hstore--1.3--1.4.sql. How can we refer
gin_extract_hstore() function from this script?

------
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company