Postgres Pro
Downloads
Home   >   mailing lists

Thread: GiST patches for 7.2 (please apply)

GiST patches for 7.2 (please apply)

From
Oleg Bartunov
Date:
Tom,

Teodor finally made patches to current CVS, please review and apply them asap
to be in sync with development (last time it was kind of problem)

gistpatch.gz

1. gistpatch.gz -  core GiST changes, all gist contrib modules were fixed reflecting  all changes. Added linear-time
splitalgorithm for R-tree GiST opclass  All regression tests passed.
 

2. btree_gist.tar.gz - Btree implementation for GiST with  support of int4 and timestamps. Should go into contrib

We're online right now and waiting for your reply
Regards,    Oleg
_____________________________________________________________
Oleg Bartunov, sci.researcher, hostmaster of AstroNet,
Sternberg Astronomical Institute, Moscow University (Russia)
Internet: oleg@sai.msu.su, http://www.sai.msu.su/~megera/
phone: +007(095)939-16-83, +007(095)939-23-83

Re: GiST patches for 7.2 (please apply)

From
Bruce Momjian
Date:
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.

> Tom,
> 
> Teodor finally made patches to current CVS, please review and apply them asap
> to be in sync with development (last time it was kind of problem)
> 
> gistpatch.gz
> 
> 1. gistpatch.gz -  core GiST changes, all gist contrib modules were fixed reflecting
>    all changes. Added linear-time split algorithm for R-tree GiST opclass
>    All regression tests passed.
> 
> 2. btree_gist.tar.gz - Btree implementation for GiST with
>    support of int4 and timestamps. Should go into contrib
> 
> We're online right now and waiting for your reply
> 
>     Regards,
>         Oleg
> _____________________________________________________________
> Oleg Bartunov, sci.researcher, hostmaster of AstroNet,
> Sternberg Astronomical Institute, Moscow University (Russia)
> Internet: oleg@sai.msu.su, http://www.sai.msu.su/~megera/
> phone: +007(095)939-16-83, +007(095)939-23-83

Content-Description: 

[ Attachment, skipping... ]

Content-Description: 

[ Attachment, skipping... ]

> 
> ---------------------------(end of broadcast)---------------------------
> TIP 2: you can get off all lists at once with the unregister command
>     (send "unregister YourEmailAddressHere" to majordomo@postgresql.org)

--  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,
Pennsylvania19026

Re: GiST patches for 7.2 (please apply)

From
Tom Lane
Date:
Looking at the revised version of initGISTstate, I am thinking that
I missed a step in updating the main indexing code for the new
pg_opclass definition.  It seems to me that if opckeytype is not 0,
that datatype ought to be used to declare the index column type from
the beginning.  Then initGISTstate wouldn't need to go through all
these pushups to develop a correct tuple descriptor for the index.

Any objections?
        regards, tom lane

Re: GiST patches for 7.2 (please apply)

From
Oleg Bartunov
Date:
OK. You may apply  patches to contrib modules (from the last patch)
and new btree module. Teodor will sent patch to core  tomorrow.
Oleg
On Wed, 22 Aug 2001, Tom Lane wrote:

> Oleg Bartunov <oleg@sai.msu.su> writes:
> > if you look into previous patch we sent (patch_72_systbl.gz)
> > you could find patch for ./src/backend/catalog/index.c which
> > is exact implementation of what 'seems to you' :-)
>
> So you did.  I'm not sure why I thought that was a bad idea when I
> was reviewing the patch.  Today it's obviously the right thing ;-)
>
> > Teodor thought you don't like this idea and he moved functionality to
> > initGISTstate. But he'd prefer his original implementation.
>
> Roger, I'll stick it in.
>
>             regards, tom lane
>
Regards,    Oleg
_____________________________________________________________
Oleg Bartunov, sci.researcher, hostmaster of AstroNet,
Sternberg Astronomical Institute, Moscow University (Russia)
Internet: oleg@sai.msu.su, http://www.sai.msu.su/~megera/
phone: +007(095)939-16-83, +007(095)939-23-83

Re: GiST patches for 7.2 (please apply)

From
Tom Lane
Date:
Oleg Bartunov <oleg@sai.msu.su> writes:
> if you look into previous patch we sent (patch_72_systbl.gz)
> you could find patch for ./src/backend/catalog/index.c which
> is exact implementation of what 'seems to you' :-)

So you did.  I'm not sure why I thought that was a bad idea when I
was reviewing the patch.  Today it's obviously the right thing ;-)

> Teodor thought you don't like this idea and he moved functionality to
> initGISTstate. But he'd prefer his original implementation.

Roger, I'll stick it in.
        regards, tom lane

Re: GiST patches for 7.2 (please apply)

From
Oleg Bartunov
Date:
On Wed, 22 Aug 2001, Tom Lane wrote:

> Looking at the revised version of initGISTstate, I am thinking that
> I missed a step in updating the main indexing code for the new
> pg_opclass definition.  It seems to me that if opckeytype is not 0,
> that datatype ought to be used to declare the index column type from
> the beginning.  Then initGISTstate wouldn't need to go through all
> these pushups to develop a correct tuple descriptor for the index.
>
> Any objections?
>

Tom,

if you look into previous patch we sent (patch_72_systbl.gz)
you could find patch for ./src/backend/catalog/index.c which
is exact implementation of what 'seems to you' :-)
Teodor thought you don't like this idea and he moved functionality to
initGISTstate. But he'd prefer his original implementation.
So, if you  have no objection, we could prepare NEW patch to
current CVS with old implementation.


>             regards, tom lane
>
Regards,    Oleg
_____________________________________________________________
Oleg Bartunov, sci.researcher, hostmaster of AstroNet,
Sternberg Astronomical Institute, Moscow University (Russia)
Internet: oleg@sai.msu.su, http://www.sai.msu.su/~megera/
phone: +007(095)939-16-83, +007(095)939-23-83

Re: GiST patches for 7.2 (please apply)

From
Tom Lane
Date:
Oleg Bartunov <oleg@sai.msu.su> writes:
> Teodor finally made patches to current CVS, please review and apply them asap
> to be in sync with development (last time it was kind of problem)

Checked and committed.  Note I did not commit your change to the cube
regression test:

*** ./contrib/cube/expected/cube.out.orig    Wed Aug 22 16:04:42 2001
--- ./contrib/cube/expected/cube.out    Wed Aug 22 16:26:25 2001
***************
*** 130,142 **** SELECT '1e700'::cube AS cube;  cube   -------
!  (inf) (1 row)  SELECT '-1e700'::cube AS cube;   cube   --------
!  (-inf) (1 row)  SELECT '1e-700'::cube AS cube;
--- 130,142 ---- SELECT '1e700'::cube AS cube;  cube   -------
!  (Inf) (1 row)  SELECT '-1e700'::cube AS cube;   cube   --------
!  (-Inf) (1 row)  SELECT '1e-700'::cube AS cube;

since on my machine "inf" appears to be the correct result.  Is this a
platform dependency, or just a lack of synchronization somewhere else?
        regards, tom lane

Re: GiST patches for 7.2 (please apply)

From
Tom Lane
Date:
Oh, one other comment --- the rtree_gist code had a bunch of functions
declared like

GISTENTRY * gbox_compress(PG_FUNCTION_ARGS);
BOX *gbox_union(PG_FUNCTION_ARGS);
GIST_SPLITVEC * gbox_picksplit(PG_FUNCTION_ARGS);
bool gbox_consistent(PG_FUNCTION_ARGS);
float * gbox_penalty(PG_FUNCTION_ARGS);
bool * gbox_same(PG_FUNCTION_ARGS);

This is not portable.  The declaration of any V1-style fmgr-callable
function must be exactly

Datum foo(PG_FUNCTION_ARGS);

no more and no less.  You can't shortcut by assuming that pointers are
the same size as Datum, or that bool is the same size as Datum, or that
the generated machine code will be the same anyway.  (There are machines
that have different register conventions for returning pointers and
integers, even though they're the same size.)  If you're going to put up
with the notational cruft of the V1 calling convention for arguments,
don't blow the portability advantages by not doing it for results too.

I fixed this in rtree_gist.c, but did not look to see if similar
problems exist elsewhere.
        regards, tom lane

Re: GiST patches for 7.2 (please apply)

From
Teodor
Date:

Tom Lane wrote:
> 
> Oleg Bartunov <oleg@sai.msu.su> writes:
> > Teodor finally made patches to current CVS, please review and apply them asap
> > to be in sync with development (last time it was kind of problem)
> 
> Checked and committed.  Note I did not commit your change to the cube
> regression test:
> 
> *** ./contrib/cube/expected/cube.out.orig       Wed Aug 22 16:04:42 2001
> --- ./contrib/cube/expected/cube.out    Wed Aug 22 16:26:25 2001
> ***************
> *** 130,142 ****
>   SELECT '1e700'::cube AS cube;
>    cube
>   -------
> !  (inf)
>   (1 row)
> 
>   SELECT '-1e700'::cube AS cube;
>     cube
>   --------
> !  (-inf)
>   (1 row)
> 
>   SELECT '1e-700'::cube AS cube;
> --- 130,142 ----
>   SELECT '1e700'::cube AS cube;
>    cube
>   -------
> !  (Inf)
>   (1 row)
> 
>   SELECT '-1e700'::cube AS cube;
>     cube
>   --------
> !  (-Inf)
>   (1 row)
> 
>   SELECT '1e-700'::cube AS cube;
> 
> since on my machine "inf" appears to be the correct result.  Is this a
> platform dependency, or just a lack of synchronization somewhere else?
> 
>                         regards, tom lane

On my box FreeBSD4.3 it looks as 'Inf'. Very similar that this is
platform dependency.

Re: GiST patches for 7.2 (please apply)

From
Tom Lane
Date:
Teodor <teodor@stack.net> writes:
> Tom Lane wrote:
>> ... on my machine "inf" appears to be the correct result.  Is this a
>> platform dependency, or just a lack of synchronization somewhere else?

> On my box FreeBSD4.3 it looks as 'Inf'. Very similar that this is
> platform dependency.

I'm inclined to just remove that part of the "cube" regression test,
then.  It's not telling us anything very important about the behavior
of the cube datatype, so I think it's not worth dealing with a platform
dependency.  Objections anyone?
        regards, tom lane

Re: GiST patches for 7.2 (please apply)

From
Oleg Bartunov
Date:
you're right. nothing special.
Oleg
On Wed, 22 Aug 2001, Tom Lane wrote:

> Teodor <teodor@stack.net> writes:
> > Tom Lane wrote:
> >> ... on my machine "inf" appears to be the correct result.  Is this a
> >> platform dependency, or just a lack of synchronization somewhere else?
>
> > On my box FreeBSD4.3 it looks as 'Inf'. Very similar that this is
> > platform dependency.
>
> I'm inclined to just remove that part of the "cube" regression test,
> then.  It's not telling us anything very important about the behavior
> of the cube datatype, so I think it's not worth dealing with a platform
> dependency.  Objections anyone?
>
>             regards, tom lane
>
Regards,    Oleg
_____________________________________________________________
Oleg Bartunov, sci.researcher, hostmaster of AstroNet,
Sternberg Astronomical Institute, Moscow University (Russia)
Internet: oleg@sai.msu.su, http://www.sai.msu.su/~megera/
phone: +007(095)939-16-83, +007(095)939-23-83