Thread: BUG #5705: btree_gist: Index on inet changes query result
The following bug has been logged online: Bug reference: 5705 Logged by: Andreas Karlsson Email address: andreas@proxel.se PostgreSQL version: 9.1 Operating system: Linux Description: btree_gist: Index on inet changes query result Details: Hi, I was looking at the code to see how one would improve indexing of the inet types and saw an inconsistency between the compressed format (gbt_inet_compress) and how network_cmp_internal works. The btree_gist module ignores the netmask. This means that while the operator thinks 1.255.255.200/8 is smaller than 1.0.0.0 the GiST index thinks the opposite. An example for how to reproduce the bug: -- Demostrate that I did not get the operator wrong. :) SELECT '1.255.255.200/8'::inet < '1.0.0.0'::inet; ?column? ---------- t (1 row) -- Create and populate table CREATE TABLE inet_test (a inet); INSERT INTO inet_test VALUES ('1.255.255.200/8'); -- Without index SELECT * FROM inet_test WHERE a < '1.0.0.0'::inet; a ----------------- 1.255.255.200/8 (1 row) EXPLAIN SELECT * FROM inet_test WHERE a < '1.0.0.0'::inet; QUERY PLAN ------------------------------------------------------------- Seq Scan on inet_test (cost=0.00..26.38 rows=437 width=32) Filter: (a < '1.0.0.0'::inet) (2 rows) -- With index CREATE INDEX inet_test_idx ON inet_test USING gist (a); SET enable_seqscan = false; SELECT * FROM inet_test WHERE a < '1.0.0.0'::inet; a --- (0 rows) EXPLAIN SELECT * FROM inet_test WHERE a < '1.0.0.0'::inet; QUERY PLAN ---------------------------------------------------------------------------- ---- Index Scan using inet_test_idx on inet_test (cost=0.00..8.27 rows=1 width=32) Index Cond: (a < '1.0.0.0'::inet) (2 rows) -- With btree index DROP INDEX inet_test_idx; CREATE INDEX inet_test_btree_idx ON inet_test USING btree (a); SELECT * FROM inet_test WHERE a < '1.0.0.0'::inet; a ----------------- 1.255.255.200/8 (1 row) EXPLAIN SELECT * FROM inet_test WHERE a < '1.0.0.0'::inet; QUERY PLAN ---------------------------------------------------------------------------- ---- Index Scan using inet_test_btree_idx on inet_test (cost=0.00..8.27 rows=1 width=32) Index Cond: (a < '1.0.0.0'::inet) (2 rows)
"Andreas Karlsson" <andreas@proxel.se> writes: > I was looking at the code to see how one would improve indexing of the inet > types and saw an inconsistency between the compressed format > (gbt_inet_compress) and how network_cmp_internal works. The btree_gist > module ignores the netmask. 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. regards, tom lane
On Mon, Oct 11, 2010 at 7:50 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > "Andreas Karlsson" <andreas@proxel.se> writes: >> I was looking at the code to see how one would improve indexing of the i= net >> types and saw an inconsistency between the compressed format >> (gbt_inet_compress) and how network_cmp_internal works. The btree_gist >> module ignores the netmask. > > 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. =A0I'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. Are you planning to fix this? --=20 Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Robert Haas <robertmhaas@gmail.com> writes: > On Mon, Oct 11, 2010 at 7:50 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: >> 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. > Are you planning to fix this? No. I don't understand why Teodor did it like that, so I'm not going to try to change it. I'd be willing to take responsibility for ripping out btree_gist's inet support altogether ... regards, tom lane
On Tue, 2010-10-19 at 18:22 -0400, Tom Lane wrote: > Robert Haas <robertmhaas@gmail.com> writes: > > On Mon, Oct 11, 2010 at 7:50 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > >> 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. > > > Are you planning to fix this? > > No. I don't understand why Teodor did it like that, so I'm not going > to try to change it. I'd be willing to take responsibility for ripping > out btree_gist's inet support altogether ... > > regards, tom lane That is the reason why I just reported it instead of trying to fix it myself first. Since I could not understand why it was done like that, I did not feel like fixing it. Best regards, Andreas Karlsson
Teodor, would you please comment on this bug after reading the entire thread which includes comments from other developers? http://archives.postgresql.org/pgsql-bugs/2010-10/msg00099.php Thanks. --------------------------------------------------------------------------- Andreas Karlsson wrote: > > The following bug has been logged online: > > Bug reference: 5705 > Logged by: Andreas Karlsson > Email address: andreas@proxel.se > PostgreSQL version: 9.1 > Operating system: Linux > Description: btree_gist: Index on inet changes query result > Details: > > Hi, > > I was looking at the code to see how one would improve indexing of the inet > types and saw an inconsistency between the compressed format > (gbt_inet_compress) and how network_cmp_internal works. The btree_gist > module ignores the netmask. > > This means that while the operator thinks 1.255.255.200/8 is smaller than > 1.0.0.0 the GiST index thinks the opposite. > > An example for how to reproduce the bug: > > -- Demostrate that I did not get the operator wrong. :) > SELECT '1.255.255.200/8'::inet < '1.0.0.0'::inet; > ?column? > ---------- > t > (1 row) > > -- Create and populate table > CREATE TABLE inet_test (a inet); > INSERT INTO inet_test VALUES ('1.255.255.200/8'); > > > -- Without index > SELECT * FROM inet_test WHERE a < '1.0.0.0'::inet; > a > ----------------- > 1.255.255.200/8 > (1 row) > > EXPLAIN SELECT * FROM inet_test WHERE a < '1.0.0.0'::inet; > QUERY PLAN > ------------------------------------------------------------- > Seq Scan on inet_test (cost=0.00..26.38 rows=437 width=32) > Filter: (a < '1.0.0.0'::inet) > (2 rows) > > -- With index > CREATE INDEX inet_test_idx ON inet_test USING gist (a); > SET enable_seqscan = false; > > SELECT * FROM inet_test WHERE a < '1.0.0.0'::inet; > a > --- > (0 rows) > > EXPLAIN SELECT * FROM inet_test WHERE a < '1.0.0.0'::inet; > QUERY PLAN > ---------------------------------------------------------------------------- > ---- > Index Scan using inet_test_idx on inet_test (cost=0.00..8.27 rows=1 > width=32) > Index Cond: (a < '1.0.0.0'::inet) > (2 rows) > > -- With btree index > DROP INDEX inet_test_idx; > CREATE INDEX inet_test_btree_idx ON inet_test USING btree (a); > SELECT * FROM inet_test WHERE a < '1.0.0.0'::inet; > a > ----------------- > 1.255.255.200/8 > (1 row) > > EXPLAIN SELECT * FROM inet_test WHERE a < '1.0.0.0'::inet; > QUERY PLAN > > ---------------------------------------------------------------------------- > ---- > Index Scan using inet_test_btree_idx on inet_test (cost=0.00..8.27 rows=1 > width=32) > Index Cond: (a < '1.0.0.0'::inet) > (2 rows) > > -- > Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org) > To make changes to your subscription: > http://www.postgresql.org/mailpref/pgsql-bugs -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. +
This is currently a TODO so at least we are tracking it. --------------------------------------------------------------------------- andreas wrote: > On Tue, 2010-10-19 at 18:22 -0400, Tom Lane wrote: > > Robert Haas <robertmhaas@gmail.com> writes: > > > On Mon, Oct 11, 2010 at 7:50 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > > >> 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. > > > > > Are you planning to fix this? > > > > No. I don't understand why Teodor did it like that, so I'm not going > > to try to change it. I'd be willing to take responsibility for ripping > > out btree_gist's inet support altogether ... > > > > regards, tom lane > > That is the reason why I just reported it instead of trying to fix it > myself first. Since I could not understand why it was done like that, I > did not feel like fixing it. > > Best regards, > Andreas Karlsson > > > > -- > Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org) > To make changes to your subscription: > http://www.postgresql.org/mailpref/pgsql-bugs -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. +