Thread: inet/cidr type comparisons

inet/cidr type comparisons

From
Alex Pilosov
Date:
I noticed current wierd behaviour of a less/greater than comparisons of
things involving inet/cidr:

10.1.2.3/8 is considered to be less than 10.0.0.0/32

The current logic does the following:
a) compare the network part of each (this will be 10.0.0.0), identical in
this case.

b) compare the netmask length. If a value has shorter netmask, its
considered to be less than a value with longer netmask.

c) the host part is compared. 

To me, this makes no sense. I think b and c should be transposed, and
netmask comparison must be only used as a tiebreaker when the values are
the same otherwise (such as, when comparing 10.1.2.3/8 and 10.1.2.3/32).

For type cidr, same thing applies: currently, 10.1.2.0/24 is considered to
be less than 10.0.0.0/8. 

If someone can tell me a good reason why the comparisons are the way they
are now, I'd much appreciate it :)

-alex



Re: inet/cidr type comparisons

From
Tom Lane
Date:
Alex Pilosov <alex@pilosoft.com> writes:
> I noticed current wierd behaviour of a less/greater than comparisons of
> things involving inet/cidr:

> 10.1.2.3/8 is considered to be less than 10.0.0.0/32

And what's wrong with that?  Essentially this comes from the conclusion
that 10/8 is less than 10.0.0.0/32, which I have no problem with.

> To me, this makes no sense. I think b and c should be transposed, and
> netmask comparison must be only used as a tiebreaker when the values are
> the same otherwise (such as, when comparing 10.1.2.3/8 and 10.1.2.3/32).

That would break the rule that network part is major sort key and host
part is minor sort key, which I think is useful behavior.

> For type cidr, same thing applies: currently, 10.1.2.0/24 is considered to
> be less than 10.0.0.0/8. 

It is?

regression=# select '10.1.2.0/24'::cidr < '10.0.0.0/8'::cidr;?column?
----------f
(1 row)

        regards, tom lane


Re: inet/cidr type comparisons

From
Alex Pilosov
Date:
This behaviour makes harder to use index to optimize a << b

What I have right now is rewriting a <<= b to use index plan :
(a >= network(b)) && ( a <= broadcast(b) )

However, that breaks down, since (for example) 

if a=10.1.2.3/32 and b = 10.1.2.0/24, broadcast(b) will be 10.1.2.255/24,
but 10.1.2.255/24 is considered to be less than 10.1.2.3/32...

I can work around this, however, the concept just didn't make sense to me,
but I see that for some people it does, so I'll live with it :)

So what I'm going to do then is to make a function set_masklen(inet|cidr,
int4) which would take an existing address and return a new value with
changed masklen.

Also, I'd like to create casting functions from varchar to inet/cidr,
since they are missing. Functions I'm writing:

varchar_inet(varchar)
varchar_cidr(varchar)
varchar_inet(varchar, int4)
varchar_cidr(varchar, int4)

(the last two variants will take masklen as a separate argument)

Does this look good? Actually, what's more advisable for these functions,
doing conversions from varchar or doing it from text?

Apologies for asking so many questions, but I'd like a sanity check before
proceeding :)

Thanks
-alex


On Mon, 11 Jun 2001, Tom Lane wrote:

> Alex Pilosov <alex@pilosoft.com> writes:
> > I noticed current wierd behaviour of a less/greater than comparisons of
> > things involving inet/cidr:
> 
> > 10.1.2.3/8 is considered to be less than 10.0.0.0/32
> 
> And what's wrong with that?  Essentially this comes from the conclusion
> that 10/8 is less than 10.0.0.0/32, which I have no problem with.
> 
> > To me, this makes no sense. I think b and c should be transposed, and
> > netmask comparison must be only used as a tiebreaker when the values are
> > the same otherwise (such as, when comparing 10.1.2.3/8 and 10.1.2.3/32).
> 
> That would break the rule that network part is major sort key and host
> part is minor sort key, which I think is useful behavior.
> 
> > For type cidr, same thing applies: currently, 10.1.2.0/24 is considered to
> > be less than 10.0.0.0/8. 
> 
> It is?
> 
> regression=# select '10.1.2.0/24'::cidr < '10.0.0.0/8'::cidr;
>  ?column?
> ----------
>  f
> (1 row)
> 
> 
>             regards, tom lane
> 
> 



Re: inet/cidr type comparisons

From
Jim Mercer
Date:
On Mon, Jun 11, 2001 at 01:16:01PM -0400, Alex Pilosov wrote:
> Apologies for asking so many questions, but I'd like a sanity check before
> proceeding :)

while you are in there, can you cahnge the print functions so that they
are consistent?

the output should be 'x.x.x.x' or 'x.x.x.x/x'.

in some cases, i have seen select's which output 'x.x.x/24', which totally
defeats the purpose of classless ip addrs.

either the addrs are classless, or they are not.

the current print stuff seems to make special cases of classful addresses.

-- 
[ Jim Mercer        jim@reptiles.org         +1 416 410-5633 ]
[ Now with more and longer words for your reading enjoyment. ]


Re: inet/cidr type comparisons

From
Tom Lane
Date:
Alex Pilosov <alex@pilosoft.com> writes:
> What I have right now is rewriting a <<= b to use index plan :
> (a >= network(b)) && ( a <= broadcast(b) )
> However, that breaks down, since (for example) 
> if a=10.1.2.3/32 and b = 10.1.2.0/24, broadcast(b) will be 10.1.2.255/24,
> but 10.1.2.255/24 is considered to be less than 10.1.2.3/32...

That simply demonstrates that broadcast(b) is not the right function to
use to derive an indexscan bound.  You probably want to do this the same
way that textual indexscan bounds are derived, viz for b = '10.1.2.0/24'
a >= '10.1.2.0/24' AND a < '10.1.3.0/24'

In other words, increment the network part.  This is for the same
reasons that motivate the construction of indexscan limits for
"a LIKE 'abc%'" as "a >= 'abc' AND a < 'abd'".

While there may not be a user-visible function for next-network-part,
that hardly matters since the special-indexqual stuff isn't user-visible
either.


> So what I'm going to do then is to make a function set_masklen(inet|cidr,
> int4) which would take an existing address and return a new value with
> changed masklen.

There may or may not be any reason to export such a function; are there
other uses for such a thing?


> Also, I'd like to create casting functions from varchar to inet/cidr,
> since they are missing. Functions I'm writing:

Should be functions from text to inet/cidr, for consistency with the
rest of Postgres.

> varchar_inet(varchar, int4)
> varchar_cidr(varchar, int4)

> (the last two variants will take masklen as a separate argument)

And do what exactly?  What if the text string specifies masklen too?

Unless this is a very common scenario, seems it's sufficient to provide
text to inet/cidr.  The other can be done with the equivalent of
inet('10.1.2.3' || '/' || '32').
        regards, tom lane


Re: inet/cidr type comparisons

From
Alex Pilosov
Date:
On Mon, 11 Jun 2001, Tom Lane wrote:

> While there may not be a user-visible function for next-network-part,
> that hardly matters since the special-indexqual stuff isn't user-visible
> either.
Well, since I'm making an indexqual clause, I do need a valid pg_proc id
there. 

It can't be resolved during the planning (directfunctioncall) because I do
want queries of a << b (b isn't a constant) to be also using the same
mechanism. (so far it looks like special_index_* can cope with that OK)

> > So what I'm going to do then is to make a function set_masklen(inet|cidr,
> > int4) which would take an existing address and return a new value with
> > changed masklen.
> 
> There may or may not be any reason to export such a function; are there
> other uses for such a thing?

Yeah, same reason I want to have text-to-varchar-with-masklen, it really
sucks having to separate masklen and host, then aggregating them back
again...At any case, I think it has to be public due to above reasoning
(function id for planner)

> > Also, I'd like to create casting functions from varchar to inet/cidr,
> > since they are missing. Functions I'm writing:
> 
> Should be functions from text to inet/cidr, for consistency with the
> rest of Postgres.
OK

> Unless this is a very common scenario, seems it's sufficient to provide
> text to inet/cidr.  The other can be done with the equivalent of
> 
>     inet('10.1.2.3' || '/' || '32').
Sounds ok, but I'd like to have network_set_masklen then to change masklen
without having to break up the value again...

Thanks again.

-alex




Re: inet/cidr type comparisons

From
Tom Lane
Date:
Jim Mercer <jim@reptiles.org> writes:
> while you are in there, can you cahnge the print functions so that they
> are consistent?

I believe they are consistent in 7.1; leastwise, you will have to make
a pretty good argument why we should change them again.  We had a very
long discussion that led up to the current solution.

There are formatting functions that provide various alternative displays
if you don't like the default one, btw.
        regards, tom lane


Re: inet/cidr type comparisons

From
Alex Pilosov
Date:
On Mon, 11 Jun 2001, Jim Mercer wrote:

> while you are in there, can you cahnge the print functions so that they
> are consistent?
> 
> the output should be 'x.x.x.x' or 'x.x.x.x/x'.
> 
> in some cases, i have seen select's which output 'x.x.x/24', which totally
> defeats the purpose of classless ip addrs.
7.1 will do it correctly, please upgrade:

users=# select '10/8'::inet; ?column?
------------10.0.0.0/8
(1 row)




Re: inet/cidr type comparisons

From
Tom Lane
Date:
Alex Pilosov <alex@pilosoft.com> writes:
> On Mon, 11 Jun 2001, Tom Lane wrote:
>> While there may not be a user-visible function for next-network-part,
>> that hardly matters since the special-indexqual stuff isn't user-visible
>> either.

> Well, since I'm making an indexqual clause, I do need a valid pg_proc id
> there. 

No, you need a constant there.

> It can't be resolved during the planning (directfunctioncall) because I do
> want queries of a << b (b isn't a constant) to be also using the same
> mechanism. (so far it looks like special_index_* can cope with that OK)

You're mistaken ... that's not supported currently.
        regards, tom lane


Re: inet/cidr type comparisons

From
Jim Mercer
Date:
On Mon, Jun 11, 2001 at 02:33:12PM -0400, Tom Lane wrote:
> Jim Mercer <jim@reptiles.org> writes:
> > while you are in there, can you cahnge the print functions so that they
> > are consistent?
> 
> I believe they are consistent in 7.1; leastwise, you will have to make
> a pretty good argument why we should change them again.  We had a very
> long discussion that led up to the current solution.

ah, it appears that this was done, thanx very much.

PostgreSQL 7.0.3 on i386-unknown-freebsdelf4.2, compiled by gcc 2.95.2
reptiles=> select '216.95.252/24'::cidr, '216.95.252/24'::inet;
---------------+-----------------216.95.252/24 | 216.95.252.0/24

PostgreSQL 7.1.2 on i386-unknown-freebsd4.3, compiled by GCC 2.95.3
amphibians=# select '216.95.252/24'::cidr, '216.95.252/24'::inet;
-----------------+-----------------216.95.252.0/24 | 216.95.252.0/24

-- 
[ Jim Mercer        jim@reptiles.org         +1 416 410-5633 ]
[ Now with more and longer words for your reading enjoyment. ]


Re: inet/cidr type comparisons

From
Alex Pilosov
Date:
On Mon, 11 Jun 2001, Tom Lane wrote:

> > It can't be resolved during the planning (directfunctioncall) because I do
> > want queries of a << b (b isn't a constant) to be also using the same
> > mechanism. (so far it looks like special_index_* can cope with that OK)
> 
> You're mistaken ... that's not supported currently.
Augh, you are right. Well, now I have three options

a) fix match_clause_to_indexkey to use something like special_index_* for
'special' operators that can use indices in a nested loop-join. 

b) add another access method to btree and muck with pg_amop (uuugh, bad
idea)

c) decide that I'm way deeper than I wanted to be already, and forget
about the idea.

Where I need a<<=b, I guess I can just use a>=network(b) and
a<next_network(b) directly. 

Tom, thanks for helping me out.

I'll still follow up with set_masklen and network_text funcs.

-alex