What to do about the broken btree_gist for inet/cidr? - Mailing list pgsql-hackers

From Andreas Karlsson
Subject What to do about the broken btree_gist for inet/cidr?
Date
Msg-id 7891efc1-8378-2cf2-617b-4143848ec895@proxel.se
Whole thread Raw
Responses Re: What to do about the broken btree_gist for inet/cidr?  (Tom Lane <tgl@sss.pgh.pa.us>)
List pgsql-hackers
Hi,

As discussed in this thread 
(https://www.postgresql.org/message-id/201010112055.o9BKtZf7011251@wwwmaster.postgresql.org) 
btree_gist is broken for the inet and cidr types. The reason is that it 
uses convert_network_to_scalar() to make a lossy GiST index, but 
convert_network_to_scalar() throws away the netmask which is necessary 
to preserve the correct sort order of the keys.

This has been broken for as long as we have had btree_gist indexes over 
inet. And personally I am not a fan of PostgreSQL shipping with known 
broken features. But it is also not obvious to me what the best way to 
fix it is.

To refresh my memory I quickly hacked together a proof of concept patch 
which adds a couple of test cases to reproduce the bug plus uses 
basically the same implementation of the btree_gist as text and numeric, 
which changes to index keys from 64 bit floats to a variable sized 
bytea. This means that the indexes are no longer lossy.

Some questions/thoughts:

Is it even worth fixing btree_gist for inet when we already have 
inet_ops which can do more things and is not broken. We could just 
deprecate and then rip out gist_inet_ops?

If we are going to fix it I cannot see any reasonably sized fix which 
does not also corrupt all existing indexes on inet, even those which do 
not contain any net masks (those which contain netmasks are already 
broken anyway). Do we have some way to handle this kind of breakage nicely?

I see two ways to fix the inet indexes, either do as in my PoC patch and 
make the indexes no longer lossy. This will make it more similar to how 
btree_gist works for other data types but requires us to change the 
storage type of the index keys, but since all indexes will break anyway 
that might not be an issue.

The other way is to implement correct lossy indexes, using something 
similar to what network_abbrev_convert() does.

Does anyone have any thoughts on this?

Andreas

Attachment

pgsql-hackers by date:

Previous
From: Tom Lane
Date:
Subject: Re: Improving spin-lock implementation on ARM.
Next
From: Tomas Vondra
Date:
Subject: Re: [PoC] Non-volatile WAL buffer