Re: postgres - development of inet/cidr

Started by Nonameover 25 years ago5 messages
#1Noname
darcy@druid.net

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.
#2Sevo Stille
sevo@ip23.net
In reply to: Noname (#1)
Re: Re: postgres - development of inet/cidr

"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

#3Jakub Bartosz Bielecki
J.B.Bielecki@elka.pw.edu.pl
In reply to: Sevo Stille (#2)
Re: Re: postgres - development of inet/cidr

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'.

#4Sevo Stille
sevo@ip23.net
In reply to: Jakub Bartosz Bielecki (#3)
Re: Re: postgres - development of inet/cidr

Jakub Bartosz Bielecki wrote:

"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.

Yes, there is. CIDR is defined as the network 10.0.0.1 & /27, while INET
is defined as host 10.0.0.1 within network 10.0.0.1 & /27. You can do
almost every network and host calculation both in CIDR and INET, but
you need implicit knowledge for it. Two columns are necessary to define
a host and its network in CIDR, and a network cannot be specified
without a host using INET - except for ugly in-band hacks like using
10.0.0.0/27 for the network which would prevent you from specifying a
base address.

Above example is exactly
equivalent to:
select '10.0.0.0/27'::inet << '10.0.0.2/27'::inet; -- FALSE

Nope. If the right hand side is automatically propagated to a network,
it is true. If not, the above IMHO should better raise an error, as a
host can never contain a host.

but:
select '10.0.0.0/27'::inet <<= '10.0.0.2/27'::inet; -- TRUE

Well, you might argue that a host could contain-or-equal a host, but as
only the equals part could ever be true, that is a redundant operator
without any meaning beyond equals, and accordingly it should not be
valid for that case.

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'.

Yes, but that does not make it any truer. CIDR 10.0.0.0/27 is
definitively not 10.0.0.0 but [10.0.0.0 .. 10.0.0.31]. A CIDR address is
never synonymous to a plain host address. You'll see the problem if you
try to calculate the inverse - any zeroed CIDR address in the entire
range from 10.0/8 to 10.0.0.0/32 would mask to 10.0.0.0. Accordingly,
there is no simple answer to a "host bigger/smaller than network"
question. For many applications, it may be useful to define that to mean
that the host is smaller than the network bottom address respectively
bigger than the top address, but any of the other possible views would
be perfectly legal as well.

Sevo

--
sevo@ip23.net

#5Jakub Bartosz Bielecki
J.B.Bielecki@elka.pw.edu.pl
In reply to: Sevo Stille (#4)
Re: Re: postgres - development of inet/cidr

On Wed, 5 Jul 2000, Sevo Stille wrote:

"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.

Yes, there is. CIDR is defined as the network 10.0.0.1 & /27, while INET
is defined as host 10.0.0.1 within network 10.0.0.1 & /27. You can do
almost every network and host calculation both in CIDR and INET, but
you need implicit knowledge for it.

I was talking about *current* implementation of INET/CIDR (which IMHO
is very ill).
There is INET for users that want simply to store IP's and don't care
about all the technical jargon.
There is CIDR for advanced users who want to store network data.

Currently these 2 types are handled by 1 implementation, moreover despite
INET netmask and CIDR prefix-length are something completely different,
both are stored in the same field of inet structure (yuck).

At the moment it works fine. But that's only a hack.
I guess the purpose was to prevent duplication of code... Blah...

select '10.0.0.0/27'::inet << '10.0.0.2/27'::inet; -- FALSE

Nope. If the right hand side is automatically propagated to a network,
it is true. If not, the above IMHO should better raise an error, as a
host can never contain a host.

select '10.0.0.0/27'::inet <<= '10.0.0.2/27'::inet; -- TRUE

Well, you might argue that a host could contain-or-equal a host, but as
only the equals part could ever be true, that is a redundant operator
without any meaning beyond equals, and accordingly it should not be
valid for that case.

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

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

Yes, but that does not make it any truer. CIDR 10.0.0.0/27 is
definitively not 10.0.0.0 but [10.0.0.0 .. 10.0.0.31].

Same as above... You are perfectly right.

Everything works until user starts messing with _both_ INET and CIDR
at the same time.

The possible solution is:
- inhibit cidr-to-inet cast (and maybe also inet-to-cidr, because
it would throw away netmask),
- CIDR operators: > = < << >>
- INET operators: > = < (and why not & | if it would be useful???)
functions: cidr network(inet); // '10.0.0.0/27'
text host(inet); // '10.0.0.1'
int masklen(inet); // 27
- write an usable manual.

Comments?
I *might* work on it if I find some spare time. But it's unlikely :(