Thread

Inet sorting patch

Ryan Mooney <ryanm@mhpcc.edu> — 2000-03-01T21:00:42Z

If PostgreSQL failed to compile on your computer or you found a bug that
is likely to be specific to one platform then please fill out this form
and e-mail it to pgsql-ports@postgresql.org.

To report any other bug, fill out the form below and e-mail it to
pgsql-bugs@postgresql.org.

If you not only found the problem but solved it and generated a patch
then e-mail it to pgsql-patches@postgresql.org instead.  Please use the
command "diff -c" to generate the patch.

You may also enter a bug report at http://www.postgresql.org/ instead of
e-mail-ing this form.

============================================================================
                        POSTGRESQL BUG REPORT TEMPLATE
============================================================================


Your name		:	Ryan Mooney
Your email address	:	ryanm@mhpcc.edu


System Configuration
---------------------
  Architecture (example: Intel Pentium)  	: Sparc4m

  Operating System (example: Linux 2.0.26 ELF) 	: OpenBSD 2.6 sparc

  PostgreSQL version (example: PostgreSQL-6.5.3): PostgreSQL-6.5.3 

  Compiler used (example:  gcc 2.8.0)		: 2.95.1


Please enter a FULL description of your problem:
------------------------------------------------

Sorting for the inet data type randomly returns the wrong result
when you have networks with the same prefix, but different netmasks.
This is due to the fact that occassionally there is random (uninitialized?) 
data in the extra bits.

ie:

  10.0/10 == 00001010.00100000.00100000.00011000
  10.0/11 == 00001010.00000000.00000000.00000000

The v4bitncmp() function was only taking one bit length argument so
it would determine that the networks were different, even though
they really aren't (and the netmask test wouldn't be used).


Please describe a way to repeat the problem.   Please try to provide a
concise reproducible example, if at all possible: 
----------------------------------------------------------------------

This error is pseudo random because it relies on the "proper" random
data being inserted into the table.  I could make the problem reproduce
itself as follows:

create table foo (
  network inet,
  stuff char);

insert into foo values ('10.0/8', 'A');
insert into foo values ('10.0/9', 'A');
insert into foo values ('10.0/10', 'A');
insert into foo values ('10.0/11', 'A');

-- This pretty much works most of the time...
select * from foo order by network desc;

-- Do a bunch of "stuff" to the table to try & get it to
-- update those data blocks.
update foo set stuff = 'R' where network = '10.0/11';
update foo set stuff = 'U' where network = '10.0/10';
update foo set stuff = 'U' where network = '10.0/11';
update foo set stuff = 'A' where network = '10.0/10';
update foo set stuff = 'R' where network = '10.0/11';

-- If your unlucky it will return networks in the wrong order here.
select * from foo order by network desc;


If you know how this problem might be fixed, list the solution below:
---------------------------------------------------------------------

Arguably the correct way to solve this would be to fully initialize
the data values prior to inserting them, however it is likely that
anyone using the inet/cidr data types is having this problem so
they would have to unload/reload all thier tables for that to work
which would suck.  So I wrote a patch to network.c to make v4bitncmp()
take both bit counts and only use the shorter one.  As far as I can
tell this solves all cases, but I might be wrong, either way it doesn't
seem to break anything.


*** network.c	Tue Feb 29 15:17:22 2000
--- network.orig.c	Mon Feb 28 16:54:49 2000
***************
*** 18,24 ****
  #include "postgres.h"
  #include "utils/builtins.h"
  
! static int	v4bitncmp(unsigned int a1, unsigned int a2, int bits1, int bits2);
  
  /*
   *	Access macros.	Add IPV6 support.
--- 18,24 ----
  #include "postgres.h"
  #include "utils/builtins.h"
  
! static int	v4bitncmp(unsigned int a1, unsigned int a2, int bits);
  
  /*
   *	Access macros.	Add IPV6 support.
***************
*** 137,143 ****
  		return FALSE;
  	if ((ip_family(a1) == AF_INET) && (ip_family(a2) == AF_INET))
  	{
! 		int			order = v4bitncmp(ip_v4addr(a1), ip_v4addr(a2), ip_bits(a1), ip_bits(a2));
  
  		return ((order < 0) || ((order == 0) && (ip_bits(a1) < ip_bits(a2))));
  	}
--- 137,143 ----
  		return FALSE;
  	if ((ip_family(a1) == AF_INET) && (ip_family(a2) == AF_INET))
  	{
! 		int			order = v4bitncmp(ip_v4addr(a1), ip_v4addr(a2), ip_bits(a2));
  
  		return ((order < 0) || ((order == 0) && (ip_bits(a1) < ip_bits(a2))));
  	}
***************
*** 166,172 ****
  	if ((ip_family(a1) == AF_INET) && (ip_family(a2) == AF_INET))
  	{
  		return ((ip_bits(a1) == ip_bits(a2))
! 		 && (v4bitncmp(ip_v4addr(a1), ip_v4addr(a2), ip_bits(a1), ip_bits(a2)) == 0));
  	}
  	else
  	{
--- 166,172 ----
  	if ((ip_family(a1) == AF_INET) && (ip_family(a2) == AF_INET))
  	{
  		return ((ip_bits(a1) == ip_bits(a2))
! 		 && (v4bitncmp(ip_v4addr(a1), ip_v4addr(a2), ip_bits(a1)) == 0));
  	}
  	else
  	{
***************
*** 192,198 ****
  		return FALSE;
  	if ((ip_family(a1) == AF_INET) && (ip_family(a2) == AF_INET))
  	{
! 		int			order = v4bitncmp(ip_v4addr(a1), ip_v4addr(a2), ip_bits(a1), ip_bits(a2));
  
  		return ((order > 0) || ((order == 0) && (ip_bits(a1) > ip_bits(a2))));
  	}
--- 192,198 ----
  		return FALSE;
  	if ((ip_family(a1) == AF_INET) && (ip_family(a2) == AF_INET))
  	{
! 		int			order = v4bitncmp(ip_v4addr(a1), ip_v4addr(a2), ip_bits(a2));
  
  		return ((order > 0) || ((order == 0) && (ip_bits(a1) > ip_bits(a2))));
  	}
***************
*** 222,228 ****
  	if ((ip_family(a1) == AF_INET) && (ip_family(a2) == AF_INET))
  	{
  		return ((ip_bits(a1) > ip_bits(a2))
! 		 && (v4bitncmp(ip_v4addr(a1), ip_v4addr(a2), ip_bits(a1), ip_bits(a2)) == 0));
  	}
  	else
  	{
--- 222,228 ----
  	if ((ip_family(a1) == AF_INET) && (ip_family(a2) == AF_INET))
  	{
  		return ((ip_bits(a1) > ip_bits(a2))
! 		 && (v4bitncmp(ip_v4addr(a1), ip_v4addr(a2), ip_bits(a2)) == 0));
  	}
  	else
  	{
***************
*** 242,248 ****
  	if ((ip_family(a1) == AF_INET) && (ip_family(a2) == AF_INET))
  	{
  		return ((ip_bits(a1) >= ip_bits(a2))
! 		 && (v4bitncmp(ip_v4addr(a1), ip_v4addr(a2), ip_bits(a1), ip_bits(a2)) == 0));
  	}
  	else
  	{
--- 242,248 ----
  	if ((ip_family(a1) == AF_INET) && (ip_family(a2) == AF_INET))
  	{
  		return ((ip_bits(a1) >= ip_bits(a2))
! 		 && (v4bitncmp(ip_v4addr(a1), ip_v4addr(a2), ip_bits(a2)) == 0));
  	}
  	else
  	{
***************
*** 262,268 ****
  	if ((ip_family(a1) == AF_INET) && (ip_family(a2) == AF_INET))
  	{
  		return ((ip_bits(a1) < ip_bits(a2))
! 		 && (v4bitncmp(ip_v4addr(a1), ip_v4addr(a2), ip_bits(a1), ip_bits(a2)) == 0));
  	}
  	else
  	{
--- 262,268 ----
  	if ((ip_family(a1) == AF_INET) && (ip_family(a2) == AF_INET))
  	{
  		return ((ip_bits(a1) < ip_bits(a2))
! 		 && (v4bitncmp(ip_v4addr(a1), ip_v4addr(a2), ip_bits(a1)) == 0));
  	}
  	else
  	{
***************
*** 282,288 ****
  	if ((ip_family(a1) == AF_INET) && (ip_family(a2) == AF_INET))
  	{
  		return ((ip_bits(a1) <= ip_bits(a2))
! 		 && (v4bitncmp(ip_v4addr(a1), ip_v4addr(a2), ip_bits(a1), ip_bits(a2)) == 0));
  	}
  	else
  	{
--- 282,288 ----
  	if ((ip_family(a1) == AF_INET) && (ip_family(a2) == AF_INET))
  	{
  		return ((ip_bits(a1) <= ip_bits(a2))
! 		 && (v4bitncmp(ip_v4addr(a1), ip_v4addr(a2), ip_bits(a1)) == 0));
  	}
  	else
  	{
***************
*** 475,490 ****
   */
  
  static int
! v4bitncmp(unsigned int a1, unsigned int a2, int bits1, int bits2)
  {
  	unsigned long mask = 0;
! 	int			i, bits;
! 
! 	bits=(bits1 < bits2) ? bits1 : bits2;
  
  	for (i = 0; i < bits; i++)
  		mask = (mask >> 1) | 0x80000000;
- 
  	a1 = ntohl(a1);
  	a2 = ntohl(a2);
  	if ((a1 & mask) < (a2 & mask))
--- 475,487 ----
   */
  
  static int
! v4bitncmp(unsigned int a1, unsigned int a2, int bits)
  {
  	unsigned long mask = 0;
! 	int			i;
  
  	for (i = 0; i < bits; i++)
  		mask = (mask >> 1) | 0x80000000;
  	a1 = ntohl(a1);
  	a2 = ntohl(a2);
  	if ((a1 & mask) < (a2 & mask))