Thread: Re: [HACKERS] CIDR/INET improvements

Re: [HACKERS] CIDR/INET improvements

From
Bruce Momjian
Date:
I looked into this, and it seems the easiest solution is to just call
network() if a cidr-cast value is output and the value is actually an
inet value internally.

Patch for testing attached.  Passes regression tests.  By affecting only
the output you can internally cast back and forth and only output is
affected.  However, if you load in a dump that was interally inet but
was dumped out as cidr-cast, you lose the unmasked bits.

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

Tom Lane wrote:
> Joachim Wieland <joe@mcknight.de> writes:
> > Actually both types are not binary compatible, since they have a
> > type component that is either 0 or 1, depending on whether it is of type
> > INET or CIDR.
>
> The whole question of the relationship of those types really needs to be
> looked at more carefully.  We've got this schizophrenic idea that they
> sometimes are the same type and sometimes are not.  ISTM that either
> they are the same type (and having a bit within the data is reasonable)
> or they are distinct types (in which case the bit within the data should
> be redundant).  I'm not sure which is better.
>
> I think the reason why things are as they are right now is to avoid
> needing a pile of redundant-seeming pg_proc entries, eg you'd need
> both abbrev(inet) and abbrev(cidr) if you were taking a hard line
> about them being different types.
>
> You can *not* just throw in a cast that removes the bit without breaking
> many of those functions for the CIDR case.  For instance abbrev behaves
> differently depending on the state of the bit:
>
> regression=# select abbrev(cidr '10.1.0.0/16');
>  abbrev
> ---------
>  10.1/16
> (1 row)
>
> regression=# select abbrev(inet '10.1.0.0/16');
>    abbrev
> -------------
>  10.1.0.0/16
> (1 row)
>
>
> > What about functions to get/set a specific byte, for example:
>
> I would vote against adding any such thing in the absence of any strong
> demand for it.  I think functions that expose the underlying data just
> encourage people to write IPv4-only code.  If you can't define and use
> the function in a way that handles both IPv4 and IPv6, you probably
> shouldn't have it.
>
>             regards, tom lane
>
> ---------------------------(end of broadcast)---------------------------
> TIP 1: if posting/reading through Usenet, please send an appropriate
>        subscribe-nomail command to majordomo@postgresql.org so that your
>        message can get through to the mailing list cleanly
>

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073
Index: src/backend/utils/adt/network.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/utils/adt/network.c,v
retrieving revision 1.60
diff -c -c -r1.60 network.c
*** src/backend/utils/adt/network.c    23 Jan 2006 21:49:39 -0000    1.60
--- src/backend/utils/adt/network.c    24 Jan 2006 04:05:52 -0000
***************
*** 166,171 ****
--- 166,181 ----
  Datum
  cidr_out(PG_FUNCTION_ARGS)
  {
+     inet       *src = PG_GETARG_INET_P(0);
+
+     /* If this is an INET, zero any unmasked bits */
+     if (!ip_is_cidr(src))
+     {
+         Datum        src2;
+
+         src2 = DirectFunctionCall1(network_network, PG_GETARG_DATUM(0));
+         fcinfo->arg[0] = src2;
+     }
      return inet_out(fcinfo);
  }


Re: [HACKERS] CIDR/INET improvements

From
Tom Lane
Date:
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.

            regards, tom lane

Re: [HACKERS] CIDR/INET improvements

From
Joachim Wieland
Date:
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                                              joe@mcknight.de
C/ Usandizaga 12 1°B                                           ICQ: 37225940
20002 Donostia / San Sebastian (Spain)                     GPG key available

Attachment

Re: [HACKERS] CIDR/INET improvements

From
Tom Lane
Date:
Joachim Wieland <joe@mcknight.de> writes:
> FWIW, I append the patch I've done a few weeks ago. It adds an inettocidr
> cast function.

I think we need to take two steps back and look at the larger picture:
the INET/CIDR situation is conceptually a mess and it's going to take
more than a localized change to clean it up.

I have some ideas about this and will try to post a proposal on -hackers
later today.

            regards, tom lane

Re: [HACKERS] CIDR/INET improvements

From
Tom Lane
Date:
Joachim Wieland <joe@mcknight.de> writes:
> FWIW, I append the patch I've done a few weeks ago. It adds an inettocidr
> cast function.

I've incorporated this code into the INET cleanup patch just committed.
Thanks!

            regards, tom lane