Thread: Re: postgres - development of inet/cidr

Re: postgres - development of inet/cidr

From
darcy@druid.net (D'Arcy J.M. Cain)
Date:
Thus spake Jakub Bartosz Bielecki
> I'm not really into PostgreSQL development, first of all.
> However once I have needed to use INET type
> in my database and (sadly) I noticed 2 very serious bugs:
> 
> 1. all functions which return text (host, network etc) should
>     return pascal-like ASCII string not C-like ASCIIZ
>    example:
>     select host('10.0.0.1')='10.0.0.1';  -- this returns false!
> 
> 2. inet comparison routines (and thus operators) work in a strange way 
>    if I set netmask length to a non-default value (other than 32).
>    example:
>     select '10.0.0.1/27'::inet='10.0.0.2/27'::inet;  -- returns true
>    I guess that this behaviour is different from described in manual.
>    And if it's right, then in surce code is 1 function which behaves
>    in an opposite way, messing the whole thing up...
> 
> I noticed these in Pg 6.5.3, however when I checked apropriate source file
> in Pg 7.0, to my suprise it was not fixed...
> 
> OK. Now my questions:
> - am I right?
> - are you working on it?
> - if not, should I fix it myself and send a patch to developers?
>   (I really dont feel like duplicating someones work)

I am working on different things right now so go ahead but do discuss your
proposed changes on hackers first (pgsql-hackers@PostgreSQL.org) as there
was quite a lot of discussion at the time about how it should work.


-- 
D'Arcy J.M. Cain <darcy@{druid|vex}.net>   |  Democracy is three wolves
http://www.druid.net/darcy/                |  and a sheep voting on
+1 416 425 1212     (DoD#0082)    (eNTP)   |  what's for dinner.


Re: Re: postgres - development of inet/cidr

From
Sevo Stille
Date:
"D'Arcy J.M. Cain" wrote:
> 
> Thus spake Jakub Bartosz Bielecki
> > 1. all functions which return text (host, network etc) should
> >       return pascal-like ASCII string not C-like ASCIIZ
> >    example:
> >       select host('10.0.0.1')='10.0.0.1';  -- this returns false!

Which it should not. In a text-to-text comparison, both have to be
equal.

> > 2. inet comparison routines (and thus operators) work in a strange way
> >    if I set netmask length to a non-default value (other than 32).
> >    example:
> >       select '10.0.0.1/27'::inet='10.0.0.2/27'::inet;  -- returns true
> >    I guess that this behaviour is different from described in manual.

This would be proper behaviour for the cidr datatype, which describes a
network. "select '10.0.0.1/27'::cidr='10.0.0.2/27'::cidr;" has to return
true, as both define the same network, the mask putting the 1 vs. 2
outside the comparison scope. 

On inet, I consider the above broken - going by the documentation,
having a netmask on a inet datatype does not define a network address
but rather supplies additional information on the cidr network the host
as specified by the address is in. Accordingly, it should only truncate
if the comparison casts to cidr. 

The big question is whether comparisons that only work on a cidr data
type (contains/contained) or have a cidr type on one side can safely
cast the inet type to cidr implicitly. For: 

"select '10.0.0.1/27'::inet = '10.0.0.2/27'::inet;"  FALSE  
"select '10.0.0.1/27'::cidr = '10.0.0.2/27'::cidr;"  TRUE
"select '10.0.0.1/27'::cidr = '10.0.0.2/27'::inet;"  FALSE  
"select '10.0.0.1/27'::cidr >> '10.0.0.2/27'::inet;" TRUE  
"select '10.0.0.1/27'::cidr << '10.0.0.2/27'::inet;" ERROR 

it looks sane, IMHO. But we need to reach an agreement on the proper
behaviour on greater/smaller comparisons. Should:

"select '10.0.0.1/27'::inet > '10.0.0.2/27'::cidr;"  

be true or false? Casting to cidr prior to comparison would make it
equivalent to "select '10.0.0.0/27'::cidr > '10.0.0.0/27'::cidr;", which
is false, both networks being equal. But we have at least three possible
comparisons of host to network - besides comparing net to net, the host
might be compared to the the base or top address of the network. That
situation can only be resolved doing explicit casts, so throwing an
error in these cases is IMHO indicated.

Sevo

-- 
sevo@ip23.net


Re: Re: postgres - development of inet/cidr

From
Jakub Bartosz Bielecki
Date:

On Mon, 3 Jul 2000, Sevo Stille wrote:
> 
> This would be proper behaviour for the cidr datatype, which describes a
> network. "select '10.0.0.1/27'::cidr='10.0.0.2/27'::cidr;" has to return
> true, as both define the same network, the mask putting the 1 vs. 2
> outside the comparison scope. 
> 
> On inet, I consider the above broken - going by the documentation,
> having a netmask on a inet datatype does not define a network address
> but rather supplies additional information on the cidr network the host
> as specified by the address is in. Accordingly, it should only truncate
> if the comparison casts to cidr. 

OK. After some inspection in list's archives I found the following
statement (http://www.postgresql.org/mhonarc/pgsql-hackers/1998-07):
> It does not work that way.  /24 is
> not a shorthand for specifying a netmask -- in CIDR, it's a "prefix
> length".
> That means "192.7.34.21/24" is either (a) a syntax error or
> (b) equivilent to "192.7.34/24".

Everybody seemed to agree with the above opinion at that time.

This is obviously _not_ the way that CIDR is handled at this moment.
"select '1.2.3.4/24'" returns "1.2.3/24" only because the _output_ routine
silently cuts host bits. Input routine stores it exactly as '1.2.3.4/24'.

Since IMHO it's wrong I prepared a patch (I'm sending it to pgsql-patch).
It fixes the CIDR input routine to zero host bits (ie beyond-prefix bits).
Please note that I didn't change the INET input routine.

Eventually I had to change a bit comparison functions.
To this moment they worked in a CIDR way (didn't compare host bits at all)
although they were used by both INET and CIDR.
Since CIDR is zero-padded now, whole 32 bits are compared by > = <
operators.
Subnet operators <<, >> are still the same, don't compare host bits.

> The big question is whether comparisons that only work on a cidr data
> type (contains/contained) or have a cidr type on one side can safely
> cast the inet type to cidr implicitly. For: 
> "select '10.0.0.1/27'::inet = '10.0.0.2/27'::inet;"  FALSE
> "select '10.0.0.1/27'::cidr = '10.0.0.2/27'::cidr;"  TRUE
> "select '10.0.0.1/27'::cidr = '10.0.0.2/27'::inet;"  FALSE
> "select '10.0.0.1/27'::cidr >> '10.0.0.2/27'::inet;" TRUE
OK.
> "select '10.0.0.1/27'::cidr << '10.0.0.2/27'::inet;" ERROR

Currently it's not an error... There is no way (and no reason) to
distinguish between INET and CIDR. Above example is exactly
equivalent to:select '10.0.0.0/27'::inet << '10.0.0.2/27'::inet; -- FALSE
but:select '10.0.0.0/27'::inet <<= '10.0.0.2/27'::inet; -- TRUE

> But we need to reach an agreement on the proper
> behaviour on greater/smaller comparisons. Should:
> 
> "select '10.0.0.1/27'::inet > '10.0.0.2/27'::cidr;"  
> 
> be true or false? Casting to cidr prior to comparison would make it
> equivalent to "select '10.0.0.0/27'::cidr > '10.0.0.0/27'::cidr;", which
> is false, both networks being equal. 

It should be (and is!) true... Since second argument is
really '10.0.0.0/27'.