On Mon, Jan 23, 2006 at 11:30:58PM -0500, Tom Lane wrote:
> Bruce Momjian <pgman@candle.pha.pa.us> writes:
> > Patch for testing attached.

> This is an utterly bad idea, because it not only doesn't address the
> problem (ie, confusion about whether inet and cidr are distinct types
> or not), but it masks mistakes in that realm by hiding data on output.
> It'll be almost impossible to debug situations where x is different
> from y but they display the same.

FWIW, I append the patch I've done a few weeks ago. It adds an inettocidr
cast function.
I updated it to comply to Bruce's recent "ip_type" -> "ip_is_cidr" change.

Joachim

-- 
Joachim Wieland                                              [EMAIL PROTECTED]
C/ Usandizaga 12 1°B                                           ICQ: 37225940
20002 Donostia / San Sebastian (Spain)                     GPG key available
diff -cr cvs/pgsql/src/backend/utils/adt/network.c 
cvs.build/pgsql/src/backend/utils/adt/network.c
*** cvs/pgsql/src/backend/utils/adt/network.c   2006-01-23 23:52:36.000000000 
+0100
--- cvs.build/pgsql/src/backend/utils/adt/network.c     2006-01-24 
09:53:37.000000000 +0100
***************
*** 325,330 ****
--- 325,387 ----
        PG_RETURN_INET_P(dst);
  }
  
+ Datum
+ inettocidr(PG_FUNCTION_ARGS)
+ {
+       inet       *src = PG_GETARG_INET_P(0);
+       inet       *dst;
+       int                     byte;
+       int                     nbits;
+       int                     maxbits;
+       int                     maxbytes;
+       unsigned char mask;
+ 
+       /* make sure any unused bits are zeroed */
+       dst = (inet *) palloc0(VARHDRSZ + sizeof(inet_struct));
+ 
+       if (ip_family(src) == PGSQL_AF_INET)
+       {
+               maxbits = 32;
+               maxbytes = 4;
+       }
+       else
+       {
+               maxbits = 128;
+               maxbytes = 16;
+       }
+       Assert(ip_bits(dst) <= maxbits);
+ 
+       /* copy over */
+       ip_family(dst) = ip_family(src);
+       ip_bits(dst) = ip_bits(src);
+       ip_is_cidr(dst) = 1;  /* 1 represents CIDR */
+       memcpy(ip_addr(dst), ip_addr(src), maxbytes);
+ 
+       /* zero out the bits that are covered by the netmask */
+       if (ip_bits(dst) < maxbits)
+       {
+               byte = ip_bits(dst) / 8;
+               nbits = ip_bits(dst) % 8;
+               /* reset the first byte, this might be a partial byte */
+               mask = 0xff;
+               if (ip_bits(dst) != 0)
+               {
+                       mask >>= nbits;
+                       ip_addr(dst)[byte] &= ~mask;
+                       byte++;
+               }
+ 
+               /* from now on we reset only complete bytes */
+               while (byte < maxbytes)
+               {
+                       ip_addr(dst)[byte] = 0;
+                       byte++;
+               }
+       }
+ 
+       PG_RETURN_INET_P(dst);
+ }
+ 
  /*
   *    Basic comparison function for sorting and inet/cidr comparisons.
   *
diff -cr cvs/pgsql/src/include/catalog/pg_cast.h 
cvs.build/pgsql/src/include/catalog/pg_cast.h
*** cvs/pgsql/src/include/catalog/pg_cast.h     2005-10-21 17:45:06.000000000 
+0200
--- cvs.build/pgsql/src/include/catalog/pg_cast.h       2006-01-24 
09:38:15.000000000 +0100
***************
*** 249,255 ****
   * INET category
   */
  DATA(insert (  650    869    0 i ));
! DATA(insert (  869    650    0 i ));
  
  /*
   * BitString category
--- 249,255 ----
   * INET category
   */
  DATA(insert (  650    869    0 i ));
! DATA(insert (  869    650 1265 a ));
  
  /*
   * BitString category
diff -cr cvs/pgsql/src/include/catalog/pg_proc.h 
cvs.build/pgsql/src/include/catalog/pg_proc.h
*** cvs/pgsql/src/include/catalog/pg_proc.h     2006-01-20 13:52:12.000000000 
+0100
--- cvs.build/pgsql/src/include/catalog/pg_proc.h       2006-01-24 
09:38:15.000000000 +0100
***************
*** 2359,2364 ****
--- 2359,2366 ----
  DESCR("I/O");
  DATA(insert OID = 911 (  inet_out                     PGNSP PGUID 12 f f t f 
i 1 2275 "869" _null_ _null_ _null_      inet_out - _null_ ));
  DESCR("I/O");
+ DATA(insert OID = 1265 (  inettocidr                  PGNSP PGUID 12 f f t f 
i 1 650 "869" _null_ _null_ _null_       inettocidr - _null_ ));
+ DESCR("convert inet to cidr");
  
  /* for cidr type support */
  DATA(insert OID = 1267 (  cidr_in                     PGNSP PGUID 12 f f t f 
i 1 650 "2275" _null_ _null_ _null_      cidr_in - _null_ ));
diff -cr cvs/pgsql/src/include/utils/builtins.h 
cvs.build/pgsql/src/include/utils/builtins.h
*** cvs/pgsql/src/include/utils/builtins.h      2006-01-20 13:52:13.000000000 
+0100
--- cvs.build/pgsql/src/include/utils/builtins.h        2006-01-24 
09:38:15.000000000 +0100
***************
*** 696,701 ****
--- 696,702 ----
  extern Datum inet_out(PG_FUNCTION_ARGS);
  extern Datum inet_recv(PG_FUNCTION_ARGS);
  extern Datum inet_send(PG_FUNCTION_ARGS);
+ extern Datum inettocidr(PG_FUNCTION_ARGS);
  extern Datum cidr_in(PG_FUNCTION_ARGS);
  extern Datum cidr_out(PG_FUNCTION_ARGS);
  extern Datum cidr_recv(PG_FUNCTION_ARGS);
diff -cr cvs/pgsql/src/test/regress/expected/inet.out 
cvs.build/pgsql/src/test/regress/expected/inet.out
*** cvs/pgsql/src/test/regress/expected/inet.out        2004-10-08 
03:45:37.000000000 +0200
--- cvs.build/pgsql/src/test/regress/expected/inet.out  2006-01-24 
09:38:15.000000000 +0100
***************
*** 242,244 ****
--- 242,289 ----
  
  SET enable_seqscan TO on;
  DROP INDEX inet_idx1;
+ -- check conversion from inet to cidr (this should zero the unmasked bits)
+ -- no change expected
+ SELECT '192.168.255.255/32'::inet::cidr;
+         cidr        
+ --------------------
+  192.168.255.255/32
+ (1 row)
+ 
+ -- should give 192.168.240.0/20.
+ SELECT '192.168.255.255/20'::inet::cidr;
+        cidr       
+ ------------------
+  192.168.240.0/20
+ (1 row)
+ 
+ -- cast every entry from the table
+ SELECT i, i::cidr FROM INET_TBL;
+         i         |        i         
+ ------------------+------------------
+  192.168.1.226/24 | 192.168.1.0/24
+  192.168.1.226    | 192.168.1.226/32
+  192.168.1.0/24   | 192.168.1.0/24
+  192.168.1.0/25   | 192.168.1.0/25
+  192.168.1.255/24 | 192.168.1.0/24
+  192.168.1.255/25 | 192.168.1.128/25
+  10.1.2.3/8       | 10.0.0.0/8
+  10.1.2.3/8       | 10.0.0.0/8
+  10.1.2.3         | 10.1.2.3/32
+  10.1.2.3/24      | 10.1.2.0/24
+  10.1.2.3/16      | 10.1.0.0/16
+  10.1.2.3/8       | 10.0.0.0/8
+  11.1.2.3/8       | 11.0.0.0/8
+  9.1.2.3/8        | 9.0.0.0/8
+  10:23::f1/64     | 10:23::/64
+  10:23::ffff      | 10:23::ffff/128
+  ::4.3.2.1/24     | ::/24
+ (17 rows)
+ 
+ -- this lost the /32 once...
+ SELECT '192.168.1.1'::inet::cidr;
+       cidr      
+ ----------------
+  192.168.1.1/32
+ (1 row)
+ 
diff -cr cvs/pgsql/src/test/regress/expected/opr_sanity.out 
cvs.build/pgsql/src/test/regress/expected/opr_sanity.out
*** cvs/pgsql/src/test/regress/expected/opr_sanity.out  2005-11-07 
18:36:47.000000000 +0100
--- cvs.build/pgsql/src/test/regress/expected/opr_sanity.out    2006-01-24 
09:38:15.000000000 +0100
***************
*** 274,279 ****
--- 274,283 ----
  -- also binary compatible.  This is legal, but usually not intended.
  -- As of 7.4, this finds the casts from text and varchar to bpchar, because
  -- those are binary-compatible while the reverse way goes through rtrim().
+ -- As of 8.2, this finds the cast from CIDR to INET. Both share the same
+ -- internal representation, but CIDR doesn't allow the unmasked bits to be
+ -- non-zero. That's why <INET>::CIDR will be cast by means of a function, the
+ -- other way round is regarded to be a legal binary compatible cast.
  SELECT *
  FROM pg_cast c
  WHERE c.castfunc = 0 AND
***************
*** 285,291 ****
  ------------+------------+----------+-------------
           25 |       1042 |        0 | i
         1043 |       1042 |        0 | i
! (2 rows)
  
  -- **************** pg_operator ****************
  -- Look for illegal values in pg_operator fields.
--- 289,296 ----
  ------------+------------+----------+-------------
           25 |       1042 |        0 | i
         1043 |       1042 |        0 | i
!         650 |        869 |        0 | i
! (3 rows)
  
  -- **************** pg_operator ****************
  -- Look for illegal values in pg_operator fields.
diff -cr cvs/pgsql/src/test/regress/sql/inet.sql 
cvs.build/pgsql/src/test/regress/sql/inet.sql
*** cvs/pgsql/src/test/regress/sql/inet.sql     2004-10-08 03:45:37.000000000 
+0200
--- cvs.build/pgsql/src/test/regress/sql/inet.sql       2006-01-24 
09:38:15.000000000 +0100
***************
*** 65,67 ****
--- 65,78 ----
  SET enable_seqscan TO on;
  DROP INDEX inet_idx1;
  
+ -- check conversion from inet to cidr (this should zero the unmasked bits)
+ -- no change expected
+ SELECT '192.168.255.255/32'::inet::cidr;
+ -- should give 192.168.240.0/20.
+ SELECT '192.168.255.255/20'::inet::cidr;
+ -- cast every entry from the table
+ SELECT i, i::cidr FROM INET_TBL;
+ 
+ -- this lost the /32 once...
+ SELECT '192.168.1.1'::inet::cidr;
+ 
diff -cr cvs/pgsql/src/test/regress/sql/opr_sanity.sql 
cvs.build/pgsql/src/test/regress/sql/opr_sanity.sql
*** cvs/pgsql/src/test/regress/sql/opr_sanity.sql       2005-07-01 
21:19:05.000000000 +0200
--- cvs.build/pgsql/src/test/regress/sql/opr_sanity.sql 2006-01-24 
09:38:15.000000000 +0100
***************
*** 223,228 ****
--- 223,233 ----
  -- As of 7.4, this finds the casts from text and varchar to bpchar, because
  -- those are binary-compatible while the reverse way goes through rtrim().
  
+ -- As of 8.2, this finds the cast from CIDR to INET. Both share the same
+ -- internal representation, but CIDR doesn't allow the unmasked bits to be
+ -- non-zero. That's why <INET>::CIDR will be cast by means of a function, the
+ -- other way round is regarded to be a legal binary compatible cast.
+ 
  SELECT *
  FROM pg_cast c
  WHERE c.castfunc = 0 AND
---------------------------(end of broadcast)---------------------------
TIP 4: Have you searched our list archives?

               http://archives.postgresql.org

Reply via email to