[PATCH] inet << indexability

Augh. Previous patch had some garbage changes in it. Sorry. This one is
clean...I promise, I'll get better at this.

-alex

On Fri, 15 Jun 2001, Alex Pilosov wrote:

Alex Pilosov <alex@pilosoft.com> writes:

Also, I have a question: I put in a regression test to check that the type
can be indexed, by doing 'explain select ...'. However, the expected
result may vary when the optimizer is tweaked.

Yes, I'd noted that already in looking at your prior version. I think
it's best not to do an EXPLAIN in the regress test, because I don't want
to have to touch the tests every time the cost functions are tweaked.
However, we can certainly check to make sure that the results of an
indexscan are what we expect. Is the table set up so that this is a
useful test case? For example, it'd be nice to check boundary
conditions (eg, try both << and <<= on a case where they should give
different results).

Do you have any thought of making network_scan_first and
network_scan_last user-visible functions? (Offhand I see no use for
them to a user, but maybe you do.) If not, I'd suggest not using the
fmgr call protocol for them, but just making them pass and return inet*,
or possibly Datum. No need for the extra notational cruft of
DirectFunctionCall.

Another minor stylistic gripe is that you should use bool/true/false
where appropriate, not int/1/0. Otherwise it looks pretty good.

Oh, one more thing: those dynloader/openbsd.h and psql/tab-complete.c
changes don't belong in this patch...

regards, tom lane

Alex Pilosov <alex@pilosoft.com> writes:

I didn't want to make them user-visible, however, the alternative, IMHO,
is worse, since these functions rely on network_broadcast and
network_network to do the work, calling sequence would be:
a) indxpath casts Datum to inet, passes to network_scan*
b) network_scan will create new Datum, pass it to network_broadcast
c) network_scan will extract inet from Datum returned
d) indxpath will then cast inet back to Datum :)
Which, I think, is pretty messy :)

Sure, but you could make them look like

Datum network_scan_first(Datum networkaddress)

without incurring any of that overhead. (Anyway, Datum <-> inet* is
only a cast.)

regards, tom lane

On Sat, 16 Jun 2001, Tom Lane wrote:

Alex Pilosov <alex@pilosoft.com> writes:

Also, I have a question: I put in a regression test to check that the type
can be indexed, by doing 'explain select ...'. However, the expected
result may vary when the optimizer is tweaked.

Yes, I'd noted that already in looking at your prior version. I think
it's best not to do an EXPLAIN in the regress test, because I don't want
to have to touch the tests every time the cost functions are tweaked.

I'll remove it with resubmitted patch.

However, we can certainly check to make sure that the results of an
indexscan are what we expect. Is the table set up so that this is a
useful test case? For example, it'd be nice to check boundary
conditions (eg, try both << and <<= on a case where they should give
different results).

I'll do that too.

Do you have any thought of making network_scan_first and
network_scan_last user-visible functions? (Offhand I see no use for
them to a user, but maybe you do.) If not, I'd suggest not using the
fmgr call protocol for them, but just making them pass and return inet*,
or possibly Datum. No need for the extra notational cruft of
DirectFunctionCall.

I didn't want to make them user-visible, however, the alternative, IMHO,
is worse, since these functions rely on network_broadcast and
network_network to do the work, calling sequence would be:
a) indxpath casts Datum to inet, passes to network_scan*
b) network_scan will create new Datum, pass it to network_broadcast
c) network_scan will extract inet from Datum returned
d) indxpath will then cast inet back to Datum :)

Which, I think, is pretty messy :)

Another minor stylistic gripe is that you should use bool/true/false
where appropriate, not int/1/0. Otherwise it looks pretty good.

I'll clean it up and resubmit.

Oh, one more thing: those dynloader/openbsd.h and psql/tab-complete.c
changes don't belong in this patch...

Sorry, my fault

-alex

On Sat, 16 Jun 2001, Tom Lane wrote:

Alex Pilosov <alex@pilosoft.com> writes:

I didn't want to make them user-visible, however, the alternative, IMHO,
is worse, since these functions rely on network_broadcast and
network_network to do the work, calling sequence would be:
a) indxpath casts Datum to inet, passes to network_scan*
b) network_scan will create new Datum, pass it to network_broadcast
c) network_scan will extract inet from Datum returned
d) indxpath will then cast inet back to Datum :)
Which, I think, is pretty messy :)

Sure, but you could make them look like

Datum network_scan_first(Datum networkaddress)

without incurring any of that overhead. (Anyway, Datum <-> inet* is
only a cast.)

Gotcha, I misunderstood you the first time.
Thanks

-alex

Your patch has been added to the PostgreSQL unapplied patches list at:

http://candle.pha.pa.us/cgi-bin/pgpatches

I will try to apply it within the next 48 hours.

Augh. Previous patch had some garbage changes in it. Sorry. This one is
clean...I promise, I'll get better at this.

-alex

On Fri, 15 Jun 2001, Alex Pilosov wrote:

This is second take at indexability of << operator for inet types.

Please take a look at it.

Also, I have a question: I put in a regression test to check that the type
can be indexed, by doing 'explain select ...'. However, the expected
result may vary when the optimizer is tweaked.

I am not sure if its a good idea to check for that, so feel free to not
commit the regression test part of this patch...If there's a better way to
check that the query will use the index in regression test, I'd like to
know too.

-alex

Content-Description:

[ Attachment, skipping... ]

---------------------------(end of broadcast)---------------------------
TIP 4: Don't 'kill -9' the postmaster

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Tom already merged [latest version of this patch] it in, so you can delete
this one from patch list.

Oh, OK. I thought he had done that but I couldn't find the commit
message in my mailbox.

Thanks
-alex

On Mon, 18 Jun 2001, Bruce Momjian wrote:

Your patch has been added to the PostgreSQL unapplied patches list at:

http://candle.pha.pa.us/cgi-bin/pgpatches

I will try to apply it within the next 48 hours.

Augh. Previous patch had some garbage changes in it. Sorry. This one is
clean...I promise, I'll get better at this.

-alex

On Fri, 15 Jun 2001, Alex Pilosov wrote:

This is second take at indexability of << operator for inet types.

Please take a look at it.

Also, I have a question: I put in a regression test to check that the type
can be indexed, by doing 'explain select ...'. However, the expected
result may vary when the optimizer is tweaked.

I am not sure if its a good idea to check for that, so feel free to not
commit the regression test part of this patch...If there's a better way to
check that the query will use the index in regression test, I'd like to
know too.

-alex

Content-Description:

[ Attachment, skipping... ]

---------------------------(end of broadcast)---------------------------
TIP 4: Don't 'kill -9' the postmaster

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Tom already merged [latest version of this patch] it in, so you can delete
this one from patch list.

Thanks
-alex

On Mon, 18 Jun 2001, Bruce Momjian wrote:

... best not to do an EXPLAIN in the regress test, because I don't want
to have to touch the tests every time the cost functions are tweaked...

At some point we really should have an "optimizer" regression test, so
y'all *do* have to touch some regression test when the cost functions
are tweaked. If it were isolated into a single test, with appropriate
comments to keep it easy to remember *why* a result should be a certain
way, then it should be manageable and make it easier to proactively
evaluate changes.

It likely would have have full coverage, but at least some over/under
test cases would help...

- Thomas