On Mon, Jan 23, 2006 at 11:30:58PM -0500, Tom Lane wrote:
> Bruce Momjian <[email protected]> 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
