Cleaning up the INET/CIDR mess - Mailing list pgsql-hackers
From | Tom Lane |
---|---|
Subject | Cleaning up the INET/CIDR mess |
Date | |
Msg-id | 20481.1138126997@sss.pgh.pa.us Whole thread Raw |
Responses |
Re: Cleaning up the INET/CIDR mess
Re: Cleaning up the INET/CIDR mess Re: Cleaning up the INET/CIDR mess |
List | pgsql-hackers |
We've had previous discussions about how the distinction between INET and CIDR isn't very well thought out, for instance http://archives.postgresql.org/pgsql-hackers/2005-01/msg01021.php http://archives.postgresql.org/pgsql-hackers/2006-01/msg00233.php The basic problem is that the code is schizophrenic about whether these types are "the same" or not. The fact that we have implicit binary (no function) coercions in both directions makes them effectively the same, so why are there two different type names in the catalogs? On the other hand, if they should be different (and they definitely have different I/O behavior), this coercion behavior is wrong. Also, if they are different types, it's redundant to have a flag inside the data structure saying which type a particular value is. After some consideration I've come to the conclusion that we really do want them to be separate types: the I/O behavior is settled (after quite some long discussions) and we don't want to change it, so we can't merge them into one type. That leads to the following proposals: Remove the internal is_cidr flag; it's a waste of space. (It doesn't actually cost anything today, because of alignment considerations, but it would cost 2 bytes if we implement the proposed 2-byte-length-word variant datum format.) Even more to the point, the presence of the flag has encouraged the sort of sloppy thinking and coding that got us into this mess. Whether it's an INET or a CIDR should be totally determined by the SQL type system. Without the flag, it's okay for cidr-to-inet to be a binary-compatible (no function) conversion. However, inet-to-cidr has to either zero out bits to the right of the netmask, or error out if any are set. Joachim Wieland posted a patch that makes the coercion function just silently zero out any such bits. That's OK with me, but does anyone want to argue for an error? (If we do make the coercion function raise error, then we'd probably need to provide a separate function that supports the bit-zeroing conversion.) Currently, both directions of cast are implicit, but that is a bad idea. I propose keeping cidr-to-inet as implicit but making inet-to-cidr an assignment cast. This fits with the fact that inet can represent all values of cidr but not vice versa (compare int4 and int8). Given the implicit binary-compatible coercion, it's OK to have just a single function taking inet for any case where the function truly doesn't care if it's looking at inet or cidr input. For the cases where the code currently pays attention to is_cidr, we'd have to make two separate functions. AFAICT that's only abbrev(inet) and text(inet) among the current functions. Also, set_masklen(inet,integer) would have to come in two flavors since the output type should be the same as the input. The relationship of cidr and inet would be a little bit like the relation between varchar and text. For instance, varchar doesn't have any comparison operators of its own, but piggybacks on text's comparison operators, relying on the implicit cast from varchar to text to make this transparent to users. One other point is what to do with the binary I/O functions (send/receive) for inet and cidr. I think that we should continue to send the is_cidr flag byte for backwards-compatibility reasons. On receive, we could either ignore that byte entirely, or insist that it match the expected datatype. I'm inclined to ignore the byte but am willing to listen to arguments to raise an error instead. Comments? regards, tom lane
pgsql-hackers by date: