Quantcast

GiST support for inet datatypes

classic Classic list List threaded Threaded
49 messages Options
123
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: GiST support for inet datatypes

Emre Hasegeli-3
2014-02-19 16:52, Tom Lane <[hidden email]>:
> Not at all, AFAICS.  If it were okay to decide that some formerly-default
> opclass is no longer default, then having such a command would be better
> than manually manipulating pg_opclass.opcdefault --- but extension upgrade
> scripts could certainly do the latter if they had to.  The problem here
> is whether it's upgrade-safe to make such a change at all; having
> syntactic sugar doesn't make it safer.

It is not hard to support != operator on the new operator class. That would
make it functionally compatible with the ones on btree_gist. That way,
changing the default would not break pg_dump dumps, it would only upgrade
the index to the new one.

pg_upgrade dumps current objects in the extension. It fails to restore them,
if the new operator class exists as the default. I do not really understand
how pg_upgrade handle extension upgrades. I do not have a solution to this.

It would be sad if we cannot make the new operator class default because of
the btree_gist implementation which is not useful for inet data types. You
wrote on 2010-10-11 about it [1]:

> Well, actually the btree_gist implementation for inet is a completely
> broken piece of junk: it thinks that convert_network_to_scalar is 100%
> trustworthy and can be used as a substitute for the real comparison
> functions, which isn't even approximately true.  I'm not sure why
> Teodor implemented it like that instead of using the type's real
> comparison functions, but it's pretty much useless if you want the
> same sort order that the type itself defines.

[1] http://www.postgresql.org/message-id/8973.1286841006@...


--
Sent via pgsql-hackers mailing list ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: GiST support for inet datatypes

Tom Lane-2
Emre Hasegeli <[hidden email]> writes:
> [ cites bug #5705 ]

Hm.  I had forgotten how thoroughly broken btree_gist's inet and cidr
opclasses are.  There was discussion at the time of just ripping them
out despite the compatibility hit.  We didn't do it, but if we had
then life would be simpler now.

Perhaps it would be acceptable to drop the btree_gist implementation
and teach pg_upgrade to refuse to upgrade if the old database contains
any such indexes.  I'm not sure that solves the problem, though, because
I think pg_upgrade will still fail if the opclass is in the old database
even though unused (because you'll get the complaint about multiple
default opclasses anyway).  The only simple way to avoid that is to not
have btree_gist loaded at all in the old DB, and I doubt that's an
acceptable upgrade restriction.  It would require dropping indexes of
the other types supported by btree_gist, which would be a pretty hard
sell since those aren't broken.

Really what we'd need here is for btree_gist to be upgraded to a version
that doesn't define gist_inet_ops (or at least doesn't mark it as default)
*before* pg_upgrading to a server version containing the proposed new
implementation.  Not sure how workable that is.  Could we get away with
having pg_upgrade unset the default flag in the old database?
(Ick, but there are no very good alternatives here ...)

BTW, I'd not been paying any attention to this thread, but now that
I scan through it, I think the proposal to change the longstanding
names of the inet operators has zero chance of being accepted.
Consistency with the names chosen for range operators is a mighty
weak argument compared to backwards compatibility.

                        regards, tom lane


--
Sent via pgsql-hackers mailing list ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: GiST support for inet datatypes

Emre Hasegeli-3
2014-02-20 01:37, Tom Lane <[hidden email]>:

> Perhaps it would be acceptable to drop the btree_gist implementation
> and teach pg_upgrade to refuse to upgrade if the old database contains
> any such indexes.  I'm not sure that solves the problem, though, because
> I think pg_upgrade will still fail if the opclass is in the old database
> even though unused (because you'll get the complaint about multiple
> default opclasses anyway).  The only simple way to avoid that is to not
> have btree_gist loaded at all in the old DB, and I doubt that's an
> acceptable upgrade restriction.  It would require dropping indexes of
> the other types supported by btree_gist, which would be a pretty hard
> sell since those aren't broken.
>
> Really what we'd need here is for btree_gist to be upgraded to a version
> that doesn't define gist_inet_ops (or at least doesn't mark it as default)
> *before* pg_upgrading to a server version containing the proposed new
> implementation.  Not sure how workable that is.  Could we get away with
> having pg_upgrade unset the default flag in the old database?
> (Ick, but there are no very good alternatives here ...)

Upgrading btree_gist on the old installation would be almost impossible
for the majority of the users who use package managers, in my opinion.
I cannot think of a better solution than your suggestion. I can try to
prepare a patch to execute the following query on pg_upgrade before
dumping the old database, if that is the final decision.

UPDATE pg_opclass SET opcdefault = false
WHERE opcname IN ('gist_inet_ops', 'gist_cidr_ops');

> BTW, I'd not been paying any attention to this thread, but now that
> I scan through it, I think the proposal to change the longstanding
> names of the inet operators has zero chance of being accepted.
> Consistency with the names chosen for range operators is a mighty
> weak argument compared to backwards compatibility.

That is why I prepared it as a separate patch on top of the others. It is
not only consistency with the range types: <@ and @> symbols used for
containment everywhere except the inet data types, particularly on
the geometric types, arrays; cube, hstore, intaray, ltree extensions.
The patch does not just change the operator names, it leaves the old ones,
adds new operators with GiST support and changes the documentation, like
your commit ba920e1c9182eac55d5f1327ab0d29b721154277 back in 2006. I could
not find why did you leave the inet operators unchanged on that commit,
in the mailing list archives [1]. GiST support will be a promotion for
users to switch to the new operators, if we make this change with it.

This change will also indirectly deprecate the undocumented non-transparent
btree index support that works sometimes for some of the subnet inclusion
operators [2].

[1] http://www.postgresql.org/message-id/14277.1157304939@...

[2] http://www.postgresql.org/message-id/389.1042992616@...


--
Sent via pgsql-hackers mailing list ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: GiST support for inet datatypes

Andreas Karlsson
In reply to this post by Emre Hasegeli-3
On 02/06/2014 06:14 PM, Emre Hasegeli wrote:
> Third versions of the patches attached. They are rebased to the HEAD. In
> this versions, the bitncommon function is changed. <sys/socket.h> included
> to network_gist.c to be able to compile it on FreeBSD. Geometric mean
> calculation for partial bucket match on the function
> inet_hist_inclusion_selectivity
> reverted back. It was something I changed without enough testing on
> the second revision of the patch. This version uses the maximum divider
> calculated from the boundaries of the bucket, like the first version. It is
> simpler and more reliable.

Thanks for the updated patch.

About the discussions about upgrading PostgreSQL, extensions and
defaults I do not have any strong opinion. I think that this patch is
useful even if it does not end up the default, but it would be a pity
since the BTree GiST index is broken.

Note: The patches do not apply anymore due to changes to
src/backend/utils/adt/Makefile.

>> I am not convinced of your approach to calculating the selectivity from the
>> histogram. The thing I have the problem with is the clever trickery involved
>> with how you handle different operator types. I prefer the clearer code of
>> the range types with how calc_hist_selectivity_scalar is used. Is there any
>> reason for why that approach would not work here or result in worse code?
>
> Currently we do not have histogram of the lower and upper bounds as
> the range types. Current histogram can be used nicely as the lower bound,
> but not the upper bound because the comparison is first on the common bits
> of the network part, then on the length of the network part. For example,
> 10.0/16 is defined as greater than 10/8.
>
> Using the histogram as the lower bounds of the networks is not enough to
> calculate selectivity for any of these operators. Using it also as the upper
> bounds is still not enough for the inclusion operators. The lengths of
> the network parts should taken into consideration in a way and it is
> what this patch does. Using separate histograms for the lower bounds,
> the upper bounds and the lengths of the network parts can solve all of these
> problems, but it is a lot of work.

I see, thanks for the explanation. But I am still not very fond of how
that code is written since I find it hard to verify the correctness of
it, but have no better suggestions.

>> I see from the tests that you still are missing selectivity functions for
>> operators, what is your plan for this?
>
> This was because the join selectivity estimation functions. I set
> the geo_selfuncs for the missing ones. All tests pass with them. I want
> to develop the join selectivity function too, but not for this commit fest.

All tests pass now. Excellent!

Do you think the new index is useful even if you use the basic
geo_selfuncs? Or should we wait with committing the patches until all
selfuncs are implemented?

--
Andreas Karlsson


--
Sent via pgsql-hackers mailing list ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: GiST support for inet datatypes

Bruce Momjian
In reply to this post by Tom Lane-2
On Wed, Feb 19, 2014 at 06:37:42PM -0500, Tom Lane wrote:

> Emre Hasegeli <[hidden email]> writes:
> > [ cites bug #5705 ]
>
> Hm.  I had forgotten how thoroughly broken btree_gist's inet and cidr
> opclasses are.  There was discussion at the time of just ripping them
> out despite the compatibility hit.  We didn't do it, but if we had
> then life would be simpler now.
>
> Perhaps it would be acceptable to drop the btree_gist implementation
> and teach pg_upgrade to refuse to upgrade if the old database contains
> any such indexes.  I'm not sure that solves the problem, though, because
> I think pg_upgrade will still fail if the opclass is in the old database
> even though unused (because you'll get the complaint about multiple
> default opclasses anyway).  The only simple way to avoid that is to not
> have btree_gist loaded at all in the old DB, and I doubt that's an
> acceptable upgrade restriction.  It would require dropping indexes of
> the other types supported by btree_gist, which would be a pretty hard
> sell since those aren't broken.
>
> Really what we'd need here is for btree_gist to be upgraded to a version
> that doesn't define gist_inet_ops (or at least doesn't mark it as default)
> *before* pg_upgrading to a server version containing the proposed new
> implementation.  Not sure how workable that is.  Could we get away with
> having pg_upgrade unset the default flag in the old database?
> (Ick, but there are no very good alternatives here ...)

pg_upgrade has _never_ modified the old cluster, and I am hesitant to do
that now.  Can we make the changes in the new cluster, or in pg_dump
when in binary upgrade mode?

--
  Bruce Momjian  <[hidden email]>        http://momjian.us
  EnterpriseDB                             http://enterprisedb.com

  + Everyone has their own god. +


--
Sent via pgsql-hackers mailing list ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: GiST support for inet datatypes

Emre Hasegeli-3
In reply to this post by Andreas Karlsson
2014-02-24 02:44, Andreas Karlsson <[hidden email]>:

> Note: The patches do not apply anymore due to changes to
> src/backend/utils/adt/Makefile.

I will fix it on the next version.

> I see, thanks for the explanation. But I am still not very fond of how that
> code is written since I find it hard to verify the correctness of it, but
> have no better suggestions.

We can split the selectivity estimation patch from the GiST support, add
it to the next commit fest for more review. In the mean time, I can
improve it with join selectivity estimation. Testing with different datasets
would help the most to reveal how true the algorithm is.

> Do you think the new index is useful even if you use the basic geo_selfuncs?
> Or should we wait with committing the patches until all selfuncs are
> implemented?

My motivation was to be able to use the cidr datatype with exclusion
constraints. I think the index would also be useful without proper
selectivity estimation functions. Range types also use geo_selfuncs for
join selectivity estimation.


--
Sent via pgsql-hackers mailing list ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: GiST support for inet datatypes

Emre Hasegeli-3
In reply to this post by Bruce Momjian
2014-02-24 17:55, Bruce Momjian <[hidden email]>:

> pg_upgrade has _never_ modified the old cluster, and I am hesitant to do
> that now.  Can we make the changes in the new cluster, or in pg_dump
> when in binary upgrade mode?

It can be possible to update the new operator class in the new cluster
as not default, before restore. After the restore, pg_upgrade can upgrade
the btree_gist extension and reset the operator class as the default.
pg_upgrade suggests to re-initdb the new cluster if it fails, anyway. Do
you think it is a better solution? I think it will be more complicated
to do in pg_dump when in binary upgrade mode.


--
Sent via pgsql-hackers mailing list ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: GiST support for inet datatypes

Florian Pflug-2
On Feb27, 2014, at 11:39 , Emre Hasegeli <[hidden email]> wrote:

> 2014-02-24 17:55, Bruce Momjian <[hidden email]>:
>
>> pg_upgrade has _never_ modified the old cluster, and I am hesitant to do
>> that now.  Can we make the changes in the new cluster, or in pg_dump
>> when in binary upgrade mode?
>
> It can be possible to update the new operator class in the new cluster
> as not default, before restore. After the restore, pg_upgrade can upgrade
> the btree_gist extension and reset the operator class as the default.
> pg_upgrade suggests to re-initdb the new cluster if it fails, anyway. Do
> you think it is a better solution? I think it will be more complicated
> to do in pg_dump when in binary upgrade mode.

Maybe I'm missing something, but isn't the gist of the problem here that
pg_dump won't explicitly state the operator class if it's the default?

If so, can't we just make pg_dump always spell out the operator class
explicitly? Then changing the default will never change the meaning of
database dumps, so upgraded clusters will simply continue to use the
old (now non-default) opclass, no?

best regards,
Florian Pflug



--
Sent via pgsql-hackers mailing list ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: GiST support for inet datatypes

Tom Lane-2
Florian Pflug <[hidden email]> writes:
> Maybe I'm missing something, but isn't the gist of the problem here that
> pg_dump won't explicitly state the operator class if it's the default?

That's not a bug, it's a feature, for much the same reasons that pg_dump
tries to minimize explicit schema-qualification.

At least, it's a feature for ordinary dumps.  I'm not sure whether it
might be a good idea for binary_upgrade dumps.  That would depend on
our policy about whether a new opclass has to have a new (and necessarily
weird) name.  If we try to make the new opclass still have the nicest
name then it won't help at all for pg_dump to dump the old name.

                        regards, tom lane


--
Sent via pgsql-hackers mailing list ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: GiST support for inet datatypes

Florian Pflug-2
On Feb27, 2014, at 17:56 , Tom Lane <[hidden email]> wrote:
> Florian Pflug <[hidden email]> writes:
>> Maybe I'm missing something, but isn't the gist of the problem here that
>> pg_dump won't explicitly state the operator class if it's the default?
>
> That's not a bug, it's a feature, for much the same reasons that pg_dump
> tries to minimize explicit schema-qualification.

I fail to see the value in this for opclasses. It's certainly nice for
schema qualifications, because dumping one schema and restoring into a
different schema is probably quite common. But how often does anyone dump
a database and restore it into a database with a different default opclass
for some type?

Or is the idea just to keep the dump as readable as possible?

> At least, it's a feature for ordinary dumps.  I'm not sure whether it
> might be a good idea for binary_upgrade dumps.  That would depend on
> our policy about whether a new opclass has to have a new (and necessarily
> weird) name.  If we try to make the new opclass still have the nicest
> name then it won't help at all for pg_dump to dump the old name.

Well, given the choice between not ever being able to change the default
opclass of a type, and not being able to re-use an old opclass' name,
I'd pick the latter. Especially because for default opclasses, users don't
usually have to know the name anyway.

best regards,
Florian Pflug



--
Sent via pgsql-hackers mailing list ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: GiST support for inet datatypes

Tom Lane-2
Florian Pflug <[hidden email]> writes:
> On Feb27, 2014, at 17:56 , Tom Lane <[hidden email]> wrote:
>> That's not a bug, it's a feature, for much the same reasons that pg_dump
>> tries to minimize explicit schema-qualification.

> I fail to see the value in this for opclasses. It's certainly nice for
> schema qualifications, because dumping one schema and restoring into a
> different schema is probably quite common.

The value in it is roughly the same as the reason we don't include a
version number when dumping CREATE EXTENSION.  If you had a default
opclass in the source database, you probably want a default opclass
in the destination, even if that's not bitwise the same as what you
had before.  The implication is that you want best practice for the
current PG version.

Another reason for not doing it is that unique-constraint syntax doesn't
even support it.  Constraints *have to* use the default opclass.

> But how often does anyone dump
> a database and restore it into a database with a different default opclass
> for some type?

Indeed.  The root of the problem here is that we've never really thought
about changing a type's default opclass before.  I don't think that a
two-line change in pg_dump fixes all the issues this will bring up.

I remind you also that even if you had a 100% bulletproof argument for
changing the behavior now, it won't affect what's in existing dump files.
The only real wiggle room we have is to change the --binary-upgrade
behavior, since we can plausibly insist that people use a current pg_dump
while doing an upgrade.

                        regards, tom lane


--
Sent via pgsql-hackers mailing list ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: GiST support for inet datatypes

Greg Stark
On Thu, Feb 27, 2014 at 5:30 PM, Tom Lane <[hidden email]> wrote:
> Indeed.  The root of the problem here is that we've never really thought
> about changing a type's default opclass before.  I don't think that a
> two-line change in pg_dump fixes all the issues this will bring up.

I think we did actually do this once. When you replaced rtree with
gist as the default opclass for the rtree method. IIRC you did it by
making "rtree" a synonym for "gist" and since the opclass wasn't
specified the default gist opclass kicked in automatically.

--
greg


--
Sent via pgsql-hackers mailing list ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: GiST support for inet datatypes

Emre Hasegeli-3
In reply to this post by Florian Pflug-2
2014-02-27 18:15, Florian Pflug <[hidden email]>:
>> It can be possible to update the new operator class in the new cluster
>> as not default, before restore. After the restore, pg_upgrade can upgrade
>> the btree_gist extension and reset the operator class as the default.
>> pg_upgrade suggests to re-initdb the new cluster if it fails, anyway. Do
>> you think it is a better solution? I think it will be more complicated
>> to do in pg_dump when in binary upgrade mode.
>
> Maybe I'm missing something, but isn't the gist of the problem here that
> pg_dump won't explicitly state the operator class if it's the default?

No, the problem is pg_upgrade. We can even benefit from this behavior of
pg_dump, if we would like to remove the operator classes on btree_gist.
Because of that behavior; users, who would upgrade by dump and restore,
will upgrade to the better default operator class without noticing. I am
not sure not notifying is the best think to do, though.

The problem is that pg_dump --binary-upgrade dumps objects in the extension
on the old cluster, not just the CREATE EXTENSION statement. pg_upgrade
fails to restore them, if the new operator class already exists on the new
cluster as the default. It effects all users who use the extension, even
if they are not using the inet and cidr operator classes in it.


--
Sent via pgsql-hackers mailing list ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: GiST support for inet datatypes

Florian Pflug-2
On Feb28, 2014, at 15:45 , Emre Hasegeli <[hidden email]> wrote:
> The problem is that pg_dump --binary-upgrade dumps objects in the extension
> on the old cluster, not just the CREATE EXTENSION statement. pg_upgrade
> fails to restore them, if the new operator class already exists on the new
> cluster as the default. It effects all users who use the extension, even
> if they are not using the inet and cidr operator classes in it.

Hm, what if we put the new opclass into an extension of its own, say inet_gist,
instead of into core?

Couldn't we then remove the inet support from the latest version of btree_gist
(the one we'd ship with 9.4)? People who don't use the old inet opclass could
then simply upgrade the extension after running pg_upgrade to get rid of the
old, broken version. People who *do* use the old inet opclass would need to
drop their indices before doing that, then install the extension inet_gist,
and finally re-create their indices.

People who do nothing would continue to use the old inet opclass.

inet_gist's SQL script could check whether btree_gist has been upgrade, and
if not fail with an error like "btree_gist must be upgraded to at least version
x.y before inet_gist can be installed". That would avoid failing with a rather
cryptic error later.

best regards,
Florian Pflug



--
Sent via pgsql-hackers mailing list ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: GiST support for inet datatypes

Emre Hasegeli-3
2014-02-28 17:30, Florian Pflug <[hidden email]>:
> Hm, what if we put the new opclass into an extension of its own, say inet_gist,
> instead of into core?

It will work but I do not think it is better than adding it in core as
not default.


--
Sent via pgsql-hackers mailing list ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: GiST support for inet datatypes

Robert Haas
In reply to this post by Tom Lane-2
On Thu, Feb 27, 2014 at 12:30 PM, Tom Lane <[hidden email]> wrote:

> Florian Pflug <[hidden email]> writes:
>> On Feb27, 2014, at 17:56 , Tom Lane <[hidden email]> wrote:
>>> That's not a bug, it's a feature, for much the same reasons that pg_dump
>>> tries to minimize explicit schema-qualification.
>
>> I fail to see the value in this for opclasses. It's certainly nice for
>> schema qualifications, because dumping one schema and restoring into a
>> different schema is probably quite common.
>
> The value in it is roughly the same as the reason we don't include a
> version number when dumping CREATE EXTENSION.  If you had a default
> opclass in the source database, you probably want a default opclass
> in the destination, even if that's not bitwise the same as what you
> had before.  The implication is that you want best practice for the
> current PG version.

I don't think that argument holds a lot of water in this instance.
The whole reason for having multiple opclasses that each one can
implement different comparison behavior.  It's unlikely that you want
an upgrade to change the comparison behavior you had before; you'd be
sad if, for example, the dump/restore process failed to preserve your
existing collation settings.

But even if that were desirable in general, suppressing it for binary
upgrade dumps certainly seems more than sane.

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


--
Sent via pgsql-hackers mailing list ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: GiST support for inet datatypes

Tom Lane-2
Robert Haas <[hidden email]> writes:
> On Thu, Feb 27, 2014 at 12:30 PM, Tom Lane <[hidden email]> wrote:
>> The value in it is roughly the same as the reason we don't include a
>> version number when dumping CREATE EXTENSION.  If you had a default
>> opclass in the source database, you probably want a default opclass
>> in the destination, even if that's not bitwise the same as what you
>> had before.  The implication is that you want best practice for the
>> current PG version.

> I don't think that argument holds a lot of water in this instance.
> The whole reason for having multiple opclasses that each one can
> implement different comparison behavior.

Well, I doubt we'd accept a proposal to change the default opclass
of a datatype to something that had incompatible behavior --- but
we might well accept one to change it to something that had improved
behavior, such as more operators.

The first couple dozen lines in GetIndexOpClass() make for interesting
reading in this context.  That approach to cross-version compatibility
probably doesn't work in the pg_upgrade universe, of course; but what I'd
like to point out here is that those kluges wouldn't have been necessary
in the first place if pg_dump had had the suppress-default-opclasses
behavior at the time.  (No, it didn't always do that; cf commits
e5bbf1965 and 1929a90b6.)

                        regards, tom lane


--
Sent via pgsql-hackers mailing list ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: GiST support for inet datatypes

Emre Hasegeli-3
In reply to this post by Emre Hasegeli-3
Fourth version of the patch attached. It is rebased to the HEAD (8879fa0).
Operator name formatting patch rebased on top of it. I will put
the selectivity estimation patch to the next commit fest.

This version of the patch does not touch to the btree_gist extension,
does not set the operator class as the default. It adds support to
the not equals operator to make the operator class compatible with
the btree_gist extension.


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

inet-gist-v4.patch (60K) Download Attachment
inet-gist-v4-operator-rename.patch (27K) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: GiST support for inet datatypes

Andres Freund-3
Hi,

On 2014-03-08 23:40:31 +0200, Emre Hasegeli wrote:
> Fourth version of the patch attached. It is rebased to the HEAD (8879fa0).
> Operator name formatting patch rebased on top of it. I will put
> the selectivity estimation patch to the next commit fest.
>
> This version of the patch does not touch to the btree_gist extension,
> does not set the operator class as the default. It adds support to
> the not equals operator to make the operator class compatible with
> the btree_gist extension.

This patch looks like it can be applied much more realistically, but it
looks too late for 9.4. I suggest moving it to the next CF?

Greetings,

Andres Freund

--
 Andres Freund                   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services


--
Sent via pgsql-hackers mailing list ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: GiST support for inet datatypes

Andreas Karlsson
On 04/04/2014 04:01 PM, Andres Freund wrote:
> This patch looks like it can be applied much more realistically, but it
> looks too late for 9.4. I suggest moving it to the next CF?

If it does not change the default operator class I do not see anything
preventing it from being applied to 9.4, as long as the committers have
the time to look at this. My review is done and I think the first patch
is ok and useful by itself.

--
Andreas


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

123
Loading...