Re: [PATCH] inet << indexability - Mailing list pgsql-hackers

From Tom Lane
Subject Re: [PATCH] inet << indexability
Date
Msg-id 28012.992707829@sss.pgh.pa.us
Whole thread Raw
In response to [PATCH] inet << indexability  (Alex Pilosov <alex@pilosoft.com>)
Responses Re: [PATCH] inet << indexability  (Alex Pilosov <alex@pilosoft.com>)
List pgsql-hackers
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


pgsql-hackers by date:

Previous
From: Tom Lane
Date:
Subject: Re: postgres dies while doing vacuum analyze
Next
From: Peter Eisentraut
Date:
Subject: Re: [current] readline breakage