Re: contrib/ intarray, ltree, intagg broken(?) by array changes

Joe Conway and I have just committed some changes in the internal
representation of Postgres arrays: an element-type-OID field is added to
the array header, and alignment calculations are now done the same way
as in ordinary tuple storage, instead of taking shortcuts. I believe
that these changes need to be reflected into the intarray, ltree, and
intagg contrib modules.

intarray and ltree both seem to be mapping their own declarations onto
arrays using largely-similar code. But while intarray fails its
regression test, I find ltree still passes. So I'm confused about what
that code is really doing and don't want to touch it.

I tried to fix intagg, but since there is no regression test for it
I'm unsure whether it's okay.

Could you folks take a look at CVS tip and see what changes are needed,
if any?

In the longer run, it might be possible to improve these routines to be
array-type-polymorphic using the new features. But with the 7.3 beta
date nearly upon us, I'd counsel first making the existing functionality
work again...

regards, tom lane

Can someone address the intagg issue here, or is the code OK?

---------------------------------------------------------------------------

Tom Lane wrote:
> Joe Conway and I have just committed some changes in the internal
> representation of Postgres arrays: an element-type-OID field is added to
> the array header, and alignment calculations are now done the same way
> as in ordinary tuple storage, instead of taking shortcuts. I believe
> that these changes need to be reflected into the intarray, ltree, and
> intagg contrib modules.
>
> intarray and ltree both seem to be mapping their own declarations onto
> arrays using largely-similar code. But while intarray fails its
> regression test, I find ltree still passes. So I'm confused about what
> that code is really doing and don't want to touch it.
>
> I tried to fix intagg, but since there is no regression test for it
> I'm unsure whether it's okay.
>
> Could you folks take a look at CVS tip and see what changes are needed,
> if any?
>
> In the longer run, it might be possible to improve these routines to be
> array-type-polymorphic using the new features. But with the 7.3 beta
> date nearly upon us, I'd counsel first making the existing functionality
> work again...
>
> regards, tom lane
>
> ---------------------------(end of broadcast)---------------------------
> TIP 3: if posting/reading through Usenet, please send an appropriate
> subscribe-nomail command to majordomo(at)postgresql(dot)org so that your
> message can get through to the mailing list cleanly
>

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073

This built and worked on my system.
famous last words, huh?

Bruce Momjian wrote:
>
> Can someone address the intagg issue here, or is the code OK?
>
> ---------------------------------------------------------------------------
>
> Tom Lane wrote:
> > Joe Conway and I have just committed some changes in the internal
> > representation of Postgres arrays: an element-type-OID field is added to
> > the array header, and alignment calculations are now done the same way
> > as in ordinary tuple storage, instead of taking shortcuts. I believe
> > that these changes need to be reflected into the intarray, ltree, and
> > intagg contrib modules.
> >
> > intarray and ltree both seem to be mapping their own declarations onto
> > arrays using largely-similar code. But while intarray fails its
> > regression test, I find ltree still passes. So I'm confused about what
> > that code is really doing and don't want to touch it.
> >
> > I tried to fix intagg, but since there is no regression test for it
> > I'm unsure whether it's okay.
> >
> > Could you folks take a look at CVS tip and see what changes are needed,
> > if any?
> >
> > In the longer run, it might be possible to improve these routines to be
> > array-type-polymorphic using the new features. But with the 7.3 beta
> > date nearly upon us, I'd counsel first making the existing functionality
> > work again...
> >
> > regards, tom lane
> >
> > ---------------------------(end of broadcast)---------------------------
> > TIP 3: if posting/reading through Usenet, please send an appropriate
> > subscribe-nomail command to majordomo(at)postgresql(dot)org so that your
> > message can get through to the mailing list cleanly
> >
>
> --
> Bruce Momjian | http://candle.pha.pa.us
> pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
> + If your life is a hard drive, | 13 Roberts Road
> + Christ can be your backup. | Newtown Square, Pennsylvania 19073

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

http://207.106.42.251/cgi-bin/pgpatches

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

---------------------------------------------------------------------------

mlw wrote:
> This built and worked on my system.
> famous last words, huh?
>
>
> Bruce Momjian wrote:
> >
> > Can someone address the intagg issue here, or is the code OK?
> >
> > ---------------------------------------------------------------------------
> >
> > Tom Lane wrote:
> > > Joe Conway and I have just committed some changes in the internal
> > > representation of Postgres arrays: an element-type-OID field is added to
> > > the array header, and alignment calculations are now done the same way
> > > as in ordinary tuple storage, instead of taking shortcuts. I believe
> > > that these changes need to be reflected into the intarray, ltree, and
> > > intagg contrib modules.
> > >
> > > intarray and ltree both seem to be mapping their own declarations onto
> > > arrays using largely-similar code. But while intarray fails its
> > > regression test, I find ltree still passes. So I'm confused about what
> > > that code is really doing and don't want to touch it.
> > >
> > > I tried to fix intagg, but since there is no regression test for it
> > > I'm unsure whether it's okay.
> > >
> > > Could you folks take a look at CVS tip and see what changes are needed,
> > > if any?
> > >
> > > In the longer run, it might be possible to improve these routines to be
> > > array-type-polymorphic using the new features. But with the 7.3 beta
> > > date nearly upon us, I'd counsel first making the existing functionality
> > > work again...
> > >
> > > regards, tom lane
> > >
> > > ---------------------------(end of broadcast)---------------------------
> > > TIP 3: if posting/reading through Usenet, please send an appropriate
> > > subscribe-nomail command to majordomo(at)postgresql(dot)org so that your
> > > message can get through to the mailing list cleanly
> > >
> >
> > --
> > Bruce Momjian | http://candle.pha.pa.us
> > pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
> > + If your life is a hard drive, | 13 Roberts Road
> > + Christ can be your backup. | Newtown Square, Pennsylvania 19073

> ? int_aggregate.sql
> ? intagg.patch
> ? intagg_test.sql
> Index: int_aggregate.c
> ===================================================================
> RCS file: /projects/cvsroot/pgsql-server/contrib/intagg/int_aggregate.c,v
> retrieving revision 1.4
> diff -u -r1.4 int_aggregate.c
> --- int_aggregate.c 2002/08/30 00:28:40 1.4
> +++ int_aggregate.c 2002/08/30 15:22:03
> @@ -11,7 +11,8 @@
> * This file is the property of the Digital Music Network (DMN).
> * It is being made available to users of the PostgreSQL system
> * under the BSD license.
> - *
> + *
> + * NOTE: This module requires sizeof(void *) to be the same as sizeof(int)
> */
> #include "postgres.h"
>
> @@ -37,6 +38,9 @@
> #include "utils/lsyscache.h"
>
>
> +/* Uncomment this define if you are compiling for postgres 7.2.x */
> +/* #define PG_7_2 */
> +
> /* This is actually a postgres version of a one dimensional array */
>
> typedef struct
> @@ -96,7 +100,9 @@
> p->a.size = cb;
> p->a.ndim = 0;
> p->a.flags = 0;
> +#ifndef PG_7_2
> p->a.elemtype = INT4OID;
> +#endif
> p->items = 0;
> p->lower= START_NUM;
> }
> @@ -149,7 +155,9 @@
> pnew->a.size = cb;
> pnew->a.ndim=1;
> pnew->a.flags = 0;
> +#ifndef PG_7_2
> pnew->a.elemtype = INT4OID;
> +#endif
> pnew->lower = 0;
> }
> else
> Index: int_aggregate.sql.in
> ===================================================================
> RCS file: /projects/cvsroot/pgsql-server/contrib/intagg/int_aggregate.sql.in,v
> retrieving revision 1.1
> diff -u -r1.1 int_aggregate.sql.in
> --- int_aggregate.sql.in 2002/02/25 03:45:27 1.1
> +++ int_aggregate.sql.in 2002/08/30 15:22:03
> @@ -1,7 +1,7 @@
> -- Drop functions
> +drop aggregate int_array_aggregate(int4);
> drop function int_agg_state (int4, int4);
> drop function int_agg_final_array (int4);
> -drop aggregate int_array_aggregate(int4);
> drop function int_array_enum (int4[]);
>
>
> @@ -9,14 +9,14 @@
> -- Is called for each item in an aggregation
> create function int_agg_state (int4, int4)
> returns int4
> - as 'MODULE_FILENAME','int_agg_state'
> + as 'MODULE_PATHNAME','int_agg_state'
> language 'c';
>
> -- Internal function for the aggregate
> -- Is called at the end of the aggregation, and returns an array.
> create function int_agg_final_array (int4)
> returns int4[]
> - as 'MODULE_FILENAME','int_agg_final_array'
> + as 'MODULE_PATHNAME','int_agg_final_array'
> language 'c';
>
> -- The aggration funcion.
> @@ -35,6 +35,6 @@
> -- as a row.
> create function int_array_enum(int4[])
> returns setof integer
> - as 'MODULE_FILENAME','int_enum'
> + as 'MODULE_PATHNAME','int_enum'
> language 'c';
>

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073

Patch applied. Thanks.

---------------------------------------------------------------------------

mlw wrote:
> This built and worked on my system.
> famous last words, huh?
>
>
> Bruce Momjian wrote:
> >
> > Can someone address the intagg issue here, or is the code OK?
> >
> > ---------------------------------------------------------------------------
> >
> > Tom Lane wrote:
> > > Joe Conway and I have just committed some changes in the internal
> > > representation of Postgres arrays: an element-type-OID field is added to
> > > the array header, and alignment calculations are now done the same way
> > > as in ordinary tuple storage, instead of taking shortcuts. I believe
> > > that these changes need to be reflected into the intarray, ltree, and
> > > intagg contrib modules.
> > >
> > > intarray and ltree both seem to be mapping their own declarations onto
> > > arrays using largely-similar code. But while intarray fails its
> > > regression test, I find ltree still passes. So I'm confused about what
> > > that code is really doing and don't want to touch it.
> > >
> > > I tried to fix intagg, but since there is no regression test for it
> > > I'm unsure whether it's okay.
> > >
> > > Could you folks take a look at CVS tip and see what changes are needed,
> > > if any?
> > >
> > > In the longer run, it might be possible to improve these routines to be
> > > array-type-polymorphic using the new features. But with the 7.3 beta
> > > date nearly upon us, I'd counsel first making the existing functionality
> > > work again...
> > >
> > > regards, tom lane
> > >
> > > ---------------------------(end of broadcast)---------------------------
> > > TIP 3: if posting/reading through Usenet, please send an appropriate
> > > subscribe-nomail command to majordomo(at)postgresql(dot)org so that your
> > > message can get through to the mailing list cleanly
> > >
> >
> > --
> > Bruce Momjian | http://candle.pha.pa.us
> > pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
> > + If your life is a hard drive, | 13 Roberts Road
> > + Christ can be your backup. | Newtown Square, Pennsylvania 19073

> ? int_aggregate.sql
> ? intagg.patch
> ? intagg_test.sql
> Index: int_aggregate.c
> ===================================================================
> RCS file: /projects/cvsroot/pgsql-server/contrib/intagg/int_aggregate.c,v
> retrieving revision 1.4
> diff -u -r1.4 int_aggregate.c
> --- int_aggregate.c 2002/08/30 00:28:40 1.4
> +++ int_aggregate.c 2002/08/30 15:22:03
> @@ -11,7 +11,8 @@
> * This file is the property of the Digital Music Network (DMN).
> * It is being made available to users of the PostgreSQL system
> * under the BSD license.
> - *
> + *
> + * NOTE: This module requires sizeof(void *) to be the same as sizeof(int)
> */
> #include "postgres.h"
>
> @@ -37,6 +38,9 @@
> #include "utils/lsyscache.h"
>
>
> +/* Uncomment this define if you are compiling for postgres 7.2.x */
> +/* #define PG_7_2 */
> +
> /* This is actually a postgres version of a one dimensional array */
>
> typedef struct
> @@ -96,7 +100,9 @@
> p->a.size = cb;
> p->a.ndim = 0;
> p->a.flags = 0;
> +#ifndef PG_7_2
> p->a.elemtype = INT4OID;
> +#endif
> p->items = 0;
> p->lower= START_NUM;
> }
> @@ -149,7 +155,9 @@
> pnew->a.size = cb;
> pnew->a.ndim=1;
> pnew->a.flags = 0;
> +#ifndef PG_7_2
> pnew->a.elemtype = INT4OID;
> +#endif
> pnew->lower = 0;
> }
> else
> Index: int_aggregate.sql.in
> ===================================================================
> RCS file: /projects/cvsroot/pgsql-server/contrib/intagg/int_aggregate.sql.in,v
> retrieving revision 1.1
> diff -u -r1.1 int_aggregate.sql.in
> --- int_aggregate.sql.in 2002/02/25 03:45:27 1.1
> +++ int_aggregate.sql.in 2002/08/30 15:22:03
> @@ -1,7 +1,7 @@
> -- Drop functions
> +drop aggregate int_array_aggregate(int4);
> drop function int_agg_state (int4, int4);
> drop function int_agg_final_array (int4);
> -drop aggregate int_array_aggregate(int4);
> drop function int_array_enum (int4[]);
>
>
> @@ -9,14 +9,14 @@
> -- Is called for each item in an aggregation
> create function int_agg_state (int4, int4)
> returns int4
> - as 'MODULE_FILENAME','int_agg_state'
> + as 'MODULE_PATHNAME','int_agg_state'
> language 'c';
>
> -- Internal function for the aggregate
> -- Is called at the end of the aggregation, and returns an array.
> create function int_agg_final_array (int4)
> returns int4[]
> - as 'MODULE_FILENAME','int_agg_final_array'
> + as 'MODULE_PATHNAME','int_agg_final_array'
> language 'c';
>
> -- The aggration funcion.
> @@ -35,6 +35,6 @@
> -- as a row.
> create function int_array_enum(int4[])
> returns setof integer
> - as 'MODULE_FILENAME','int_enum'
> + as 'MODULE_PATHNAME','int_enum'
> language 'c';
>

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073

We'll be back to work next week and look into code.

Oleg
On Sun, 1 Sep 2002, Bruce Momjian wrote:

>
> Can someone address the intagg issue here, or is the code OK?
>
> ---------------------------------------------------------------------------
>
> Tom Lane wrote:
> > Joe Conway and I have just committed some changes in the internal
> > representation of Postgres arrays: an element-type-OID field is added to
> > the array header, and alignment calculations are now done the same way
> > as in ordinary tuple storage, instead of taking shortcuts. I believe
> > that these changes need to be reflected into the intarray, ltree, and
> > intagg contrib modules.
> >
> > intarray and ltree both seem to be mapping their own declarations onto
> > arrays using largely-similar code. But while intarray fails its
> > regression test, I find ltree still passes. So I'm confused about what
> > that code is really doing and don't want to touch it.
> >
> > I tried to fix intagg, but since there is no regression test for it
> > I'm unsure whether it's okay.
> >
> > Could you folks take a look at CVS tip and see what changes are needed,
> > if any?
> >
> > In the longer run, it might be possible to improve these routines to be
> > array-type-polymorphic using the new features. But with the 7.3 beta
> > date nearly upon us, I'd counsel first making the existing functionality
> > work again...
> >
> > regards, tom lane
> >
> > ---------------------------(end of broadcast)---------------------------
> > TIP 3: if posting/reading through Usenet, please send an appropriate
> > subscribe-nomail command to majordomo(at)postgresql(dot)org so that your
> > message can get through to the mailing list cleanly
> >
>
>

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

I have already received a patch that modifies the regression tests and
it seems things are working. However, it would be good for you to take
a look. Fortunately, that can be done anytime during beta. Thanks.

---------------------------------------------------------------------------

Oleg Bartunov wrote:
> We'll be back to work next week and look into code.
>
> Oleg
> On Sun, 1 Sep 2002, Bruce Momjian wrote:
>
> >
> > Can someone address the intagg issue here, or is the code OK?
> >
> > ---------------------------------------------------------------------------
> >
> > Tom Lane wrote:
> > > Joe Conway and I have just committed some changes in the internal
> > > representation of Postgres arrays: an element-type-OID field is added to
> > > the array header, and alignment calculations are now done the same way
> > > as in ordinary tuple storage, instead of taking shortcuts. I believe
> > > that these changes need to be reflected into the intarray, ltree, and
> > > intagg contrib modules.
> > >
> > > intarray and ltree both seem to be mapping their own declarations onto
> > > arrays using largely-similar code. But while intarray fails its
> > > regression test, I find ltree still passes. So I'm confused about what
> > > that code is really doing and don't want to touch it.
> > >
> > > I tried to fix intagg, but since there is no regression test for it
> > > I'm unsure whether it's okay.
> > >
> > > Could you folks take a look at CVS tip and see what changes are needed,
> > > if any?
> > >
> > > In the longer run, it might be possible to improve these routines to be
> > > array-type-polymorphic using the new features. But with the 7.3 beta
> > > date nearly upon us, I'd counsel first making the existing functionality
> > > work again...
> > >
> > > regards, tom lane
> > >
> > > ---------------------------(end of broadcast)---------------------------
> > > TIP 3: if posting/reading through Usenet, please send an appropriate
> > > subscribe-nomail command to majordomo(at)postgresql(dot)org so that your
> > > message can get through to the mailing list cleanly
> > >
> >
> >
>
> Regards,
> Oleg
> _____________________________________________________________
> Oleg Bartunov, sci.researcher, hostmaster of AstroNet,
> Sternberg Astronomical Institute, Moscow University (Russia)
> Internet: oleg(at)sai(dot)msu(dot)su, http://www.sai.msu.su/~megera/
> phone: +007(095)939-16-83, +007(095)939-23-83
>
>

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073

> intarray and ltree both seem to be mapping their own declarations onto
> arrays using largely-similar code. But while intarray fails its
> regression test, I find ltree still passes. So I'm confused about what
> that code is really doing and don't want to touch it.

Please, apply attached patch, it solves the problem.

--
Teodor Sigaev
teodor(at)stack(dot)net

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.

---------------------------------------------------------------------------

Teodor Sigaev wrote:
>
>
> > intarray and ltree both seem to be mapping their own declarations onto
> > arrays using largely-similar code. But while intarray fails its
> > regression test, I find ltree still passes. So I'm confused about what
> > that code is really doing and don't want to touch it.
>
> Please, apply attached patch, it solves the problem.
>
>
> --
> Teodor Sigaev
> teodor(at)stack(dot)net
>

[ application/gzip is not supported, skipping... ]

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073

Patch applied. Thanks.

---------------------------------------------------------------------------

Teodor Sigaev wrote:
>
>
> > intarray and ltree both seem to be mapping their own declarations onto
> > arrays using largely-similar code. But while intarray fails its
> > regression test, I find ltree still passes. So I'm confused about what
> > that code is really doing and don't want to touch it.
>
> Please, apply attached patch, it solves the problem.
>
>
> --
> Teodor Sigaev
> teodor(at)stack(dot)net
>

[ application/gzip is not supported, skipping... ]

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073