Re: Cleaning up the INET/CIDR mess

From: Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: Cleaning up the INET/CIDR mess
Date: 2006-01-25 04:22:21
Message-ID: 200601250422.k0P4ML808767@candle.pha.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers


This is exactly what I had in mind:

split the types
zero out the bits going to cidr
no change going to inet
make functions take inet, which as not cast change

---------------------------------------------------------------------------

Tom Lane wrote:
> 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
>
> ---------------------------(end of broadcast)---------------------------
> TIP 2: Don't 'kill -9' the postmaster
>

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Philip Warner 2006-01-25 12:33:45 Suggestions for post-mortem...
Previous Message Jim C. Nasby 2006-01-25 02:54:27 Re: Weird pg_dumpall bug?