Thread: Re: [BUGS] BUG #3852: Could not create complex aggregate

"Sokolov Yura" <funny.falcon@gmail.com> writes:
> create or replace function add_group(grp anyarray, ad anyelement, size int4)
> returns anyarray
> language plpgsql
> ...

> create aggregate build_group(anyelement, int4) (
>   SFUNC= add_group,
>   STYPE = anyarray
> );

> ERROR:  argument declared "anyarray" is not an array but type anyarray

After chewing on this for awhile, it seems to me that pg_aggregate.c
is using enforce_generic_type_consistency() in a rather fundamentally
different way than it's being used anywhere else.  Everywhere else,
the "actual" argument types are expected to be real (not polymorphic)
types and enforce_generic_type_consistency() is expected to derive a
real result type.  But in pg_aggregate.c, the "actual" argument types
are the declared input and transition types of the aggregate function,
which could be polymorphic, and it is okay to hand back a polymorphic
result type if there's not enough information yet.

I think we could make enforce_generic_type_consistency() clearer by
adding an additional argument "bool allow_poly" which specifies
whether polymorphic "actual" argument and result types are allowed.
(Or maybe split it into two functions, although I think there'd be a lot
of code duplication.)  In this case it would allow ANYARRAY as the
"actual" argument type matching an ANYARRAY parameter, similarly for
ANYELEMENT, ANYENUM, etc, and it would hand back ANYARRAY or ANYELEMENT
if the result type couldn't be determined yet.  lookup_agg_function()
should always invoke enforce_generic_type_consistency(), with this
argument "true".

Although this problem really goes quite far back, I think it's probably
not interesting to back-patch further than 8.2, because AFAICS the
interesting cases involve aggregates with more than one argument.

Thoughts?
        regards, tom lane

Tom Lane wrote:
> "Sokolov Yura" <funny.falcon@gmail.com> writes:
>> create or replace function add_group(grp anyarray, ad anyelement, size int4)
>> returns anyarray
>> language plpgsql
>> ...
> 
>> create aggregate build_group(anyelement, int4) (
>>   SFUNC= add_group,
>>   STYPE = anyarray
>> );
> 
>> ERROR:  argument declared "anyarray" is not an array but type anyarray

> After chewing on this for awhile, it seems to me that pg_aggregate.c
> is using enforce_generic_type_consistency() in a rather fundamentally
> different way than it's being used anywhere else.
[snip]
> I think we could make enforce_generic_type_consistency() clearer by
> adding an additional argument "bool allow_poly" which specifies
> whether polymorphic "actual" argument and result types are allowed.
[snip]
> lookup_agg_function()
> should always invoke enforce_generic_type_consistency(), with this
> argument "true".

This sounds like a reasonable plan to me.

> Although this problem really goes quite far back, I think it's probably
> not interesting to back-patch further than 8.2, because AFAICS the
> interesting cases involve aggregates with more than one argument.

I agree, especially since this is the first time anyone has complained.

Did you want me to work on this? I could probably put some time into it 
this coming weekend.

Joe

Joe Conway <mail@joeconway.com> writes:
> Did you want me to work on this? I could probably put some time into it 
> this coming weekend.

I'll try to get to it before that --- if no serious bugs come up this
week, core is thinking of wrapping 8.3.0 at the end of the week, so
it'd be nice to have this dealt with sooner than that.
        regards, tom lane

Hi,

On Mon, 2008-01-07 at 17:12 -0500, Tom Lane wrote:
> if no serious bugs come up this week, core is thinking of wrapping
> 8.3.0 at the end of the week,

Please give the packagers and other people one more week break between
two releases. We all had a busy weekend.

Regards,
--
Devrim GÜNDÜZ , RHCE
PostgreSQL Replication, Consulting, Custom Development, 24x7 support
Managed Services, Shared and Dedicated Hosting
Co-Authors: plPHP, ODBCng - http://www.commandprompt.com/

Devrim G�ND�Z wrote:
> Hi,
> 
> On Mon, 2008-01-07 at 17:12 -0500, Tom Lane wrote:
> > if no serious bugs come up this week, core is thinking of wrapping
> > 8.3.0 at the end of the week,
> 
> Please give the packagers and other people one more week break between
> two releases. We all had a busy weekend.

We have thousands of people waiting for the 8.3 release.  It seems odd
to hold up 8.3 because the packagers are too busy.  Perhaps we need more
packagers or release the source and have the binary packages ready later.

--  Bruce Momjian  <bruce@momjian.us>        http://momjian.us EnterpriseDB
http://postgres.enterprisedb.com
 + If your life is a hard drive, Christ can be your backup. +

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On Mon, 7 Jan 2008 19:06:27 -0500 (EST)
Bruce Momjian <bruce@momjian.us> wrote:

> Devrim GÜNDÜZ wrote:
> > Hi,
> > 
> > On Mon, 2008-01-07 at 17:12 -0500, Tom Lane wrote:
> > > if no serious bugs come up this week, core is thinking of wrapping
> > > 8.3.0 at the end of the week,
> > 
> > Please give the packagers and other people one more week break
> > between two releases. We all had a busy weekend.
> 
> We have thousands of people waiting for the 8.3 release.  It seems odd
> to hold up 8.3 because the packagers are too busy.  Perhaps we need
> more packagers or release the source and have the binary packages
> ready later.

Perhaps we need to allow the release candidate to percolate and be
tested a bit before we rush to release. If I recall, it is release when
it is done not release because lots of people are waiting for it.

Sincerely,

Joshua D. Drake 


- -- 
The PostgreSQL Company: Since 1997, http://www.commandprompt.com/ 
Sales/Support: +1.503.667.4564   24x7/Emergency: +1.800.492.2240
Donate to the PostgreSQL Project: http://www.postgresql.org/about/donate
SELECT 'Training', 'Consulting' FROM vendor WHERE name = 'CMD'


-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)

iD8DBQFHgsEEATb/zqfZUUQRAkpVAJ9aThdQjyyCc9B7mv4hGDZbJygUpwCfcTLi
qGyVYutwo6cL8UwfWf38c0A=
=T8Pb
-----END PGP SIGNATURE-----

Hi,

On Mon, 2008-01-07 at 19:06 -0500, Bruce Momjian wrote:
> We have thousands of people waiting for the 8.3 release.

Really? How did you get this statistic?

...and IIRC, our motto is "Release when it is ready", not "release
because thousands ppl are waiting for it".

Regards,
--
Devrim GÜNDÜZ , RHCE
PostgreSQL Replication, Consulting, Custom Development, 24x7 support
Managed Services, Shared and Dedicated Hosting
Co-Authors: plPHP, ODBCng - http://www.commandprompt.com/

On Monday 07 January 2008 16:06:27 Bruce Momjian wrote:
> Devrim GÜNDÜZ wrote:
> > Hi,
> >
> > On Mon, 2008-01-07 at 17:12 -0500, Tom Lane wrote:
> > > if no serious bugs come up this week, core is thinking of wrapping
> > > 8.3.0 at the end of the week,
> >
> > Please give the packagers and other people one more week break between
> > two releases. We all had a busy weekend.
>
> We have thousands of people waiting for the 8.3 release.  It seems odd
> to hold up 8.3 because the packagers are too busy.  Perhaps we need more
> packagers or release the source and have the binary packages ready later.

Also to be fair I've seen no real announcement of RC1, probabbly because it's
been bundled in with "security releases" for the other branched.. can we
maybe do an RC1 announcement, let people actually test the RC before we push
a gold ?


--
Darcy Buskermolen
Command Prompt, Inc.
+1.503.667.4564 X 102
http://www.commandprompt.com/
PostgreSQL solutions since 1997

Hi,

On Mon, 2008-01-07 at 19:06 -0500, Bruce Momjian wrote:
> It seems odd to hold up 8.3 because the packagers are too busy.

... and read my e-mail again. I wrote:

"Please give the packagers and other people one more week break..."

I did not say "just the packagers". You give almost no time to people to
test the release itself -- we are already behind our schedule. One week
more would not hurt, IMHO.

Regards,
--
Devrim GÜNDÜZ , RHCE
PostgreSQL Replication, Consulting, Custom Development, 24x7 support
Managed Services, Shared and Dedicated Hosting
Co-Authors: plPHP, ODBCng - http://www.commandprompt.com/

Joshua D. Drake wrote:
> > > Hi,
> > > 
> > > On Mon, 2008-01-07 at 17:12 -0500, Tom Lane wrote:
> > > > if no serious bugs come up this week, core is thinking of wrapping
> > > > 8.3.0 at the end of the week,
> > > 
> > > Please give the packagers and other people one more week break
> > > between two releases. We all had a busy weekend.
> > 
> > We have thousands of people waiting for the 8.3 release.  It seems odd
> > to hold up 8.3 because the packagers are too busy.  Perhaps we need
> > more packagers or release the source and have the binary packages
> > ready later.
> 
> Perhaps we need to allow the release candidate to percolate and be
> tested a bit before we rush to release. If I recall, it is release when
> it is done not release because lots of people are waiting for it.

Right, but you are mixing source code not ready with packagers not
ready.  They are two different things.

I am not saying we are ready to release but if we determine the source
code is ready I would like to avoid more delay because the packagers
aren't ready.

--  Bruce Momjian  <bruce@momjian.us>        http://momjian.us EnterpriseDB
http://postgres.enterprisedb.com
 + If your life is a hard drive, Christ can be your backup. +

Devrim G�ND�Z wrote:
-- Start of PGP signed section.
> Hi,
> 
> On Mon, 2008-01-07 at 19:06 -0500, Bruce Momjian wrote:
> > It seems odd to hold up 8.3 because the packagers are too busy.
> 
> ... and read my e-mail again. I wrote:
> 
> "Please give the packagers and other people one more week break..."
> 
> I did not say "just the packagers". You give almost no time to people to
> test the release itself -- we are already behind our schedule. One week
> more would not hurt, IMHO.

If we need more time for testing, fine, but I am not in favor of
delaying to give packagers more "break" time.  I think I was clear in
that.

--  Bruce Momjian  <bruce@momjian.us>        http://momjian.us EnterpriseDB
http://postgres.enterprisedb.com
 + If your life is a hard drive, Christ can be your backup. +

Bruce Momjian <bruce@momjian.us> writes:
> I am not saying we are ready to release but if we determine the source
> code is ready I would like to avoid more delay because the packagers
> aren't ready.

It may be moot anyway.  Now that I've caught up on today's email,
this thread looks like we have some nasty GIST/GIN bugs:
http://archives.postgresql.org/pgsql-general/2008-01/msg00176.php
I don't think we can even consider releasing before we've got to
the bottom of those reports.
        regards, tom lane

On Mon, Jan 07, 2008 at 08:00:36PM -0500, Bruce Momjian wrote:
> Joshua D. Drake wrote:
> > > > Hi,
> > > > 
> > > > On Mon, 2008-01-07 at 17:12 -0500, Tom Lane wrote:
> > > > > if no serious bugs come up this week, core is thinking of wrapping
> > > > > 8.3.0 at the end of the week,
> > > > 
> > > > Please give the packagers and other people one more week break
> > > > between two releases. We all had a busy weekend.
> > > 
> > > We have thousands of people waiting for the 8.3 release.  It seems odd
> > > to hold up 8.3 because the packagers are too busy.  Perhaps we need
> > > more packagers or release the source and have the binary packages
> > > ready later.
> > 
> > Perhaps we need to allow the release candidate to percolate and be
> > tested a bit before we rush to release. If I recall, it is release when
> > it is done not release because lots of people are waiting for it.
> 
> Right, but you are mixing source code not ready with packagers not
> ready.  They are two different things.
> 
> I am not saying we are ready to release but if we determine the source
> code is ready I would like to avoid more delay because the packagers
> aren't ready.

(Yes, I read Toms message saying we are not ready, but these points apply
anyway)

I think you greatly underestimate how important the binary distributions
are. If they are not available at release time, we will get a *lot* of
questions about that. I know it's certainly true for win32 packaging, and
I beleive it is for the others as well.

More importantly, we haven't even announced RC1 yet. Which means that we're
now talking maybe 4 days of RC before we wrap release, which seems way too
little.

And in fact, just 4 days of warnings for the packagers for something as
big as this isn't enough *even if we hadn't just released the back
branches*. And we normally give them more warning.

Also note that the tentative release timeline says that the presskit should
be verified 6 days before release, something which hasn't even been
started. Translations of presskits shuold be finished 10 days before.
Embargoed pressreleases should go out 7 days before. etc.

Buttom line, I think Devrim has a very valid complaint. For more reasons
than he actually stated :-) That said, 2 or 3 weeks from now seems very
doable. But let's pick an actual date around taht and inform the packagers
and press contacts about it so they can prepare.

//Magnus

On Mon, Jan 07, 2008 at 04:24:13PM -0800, Darcy Buskermolen wrote:
> On Monday 07 January 2008 16:06:27 Bruce Momjian wrote:
> > Devrim GÜNDÜZ wrote:
> > > Hi,
> > >
> > > On Mon, 2008-01-07 at 17:12 -0500, Tom Lane wrote:
> > > > if no serious bugs come up this week, core is thinking of wrapping
> > > > 8.3.0 at the end of the week,
> > >
> > > Please give the packagers and other people one more week break between
> > > two releases. We all had a busy weekend.
> >
> > We have thousands of people waiting for the 8.3 release.  It seems odd
> > to hold up 8.3 because the packagers are too busy.  Perhaps we need more
> > packagers or release the source and have the binary packages ready later.
> 
> Also to be fair I've seen no real announcement of RC1, probabbly because it's 
> been bundled in with "security releases" for the other branched.. can we 
> maybe do an RC1 announcement, let people actually test the RC before we push 
> a gold ?

Yeah, when are we going to annonce RC1? We nede to update the beta pages on
the website at the same time.

//Magnus

On Tue, Jan 08, 2008 at 09:47:15AM +0000, Peter Childs wrote:
> On 08/01/2008, Darcy Buskermolen <darcyb@commandprompt.com> wrote:
> >
> > On Monday 07 January 2008 16:06:27 Bruce Momjian wrote:
> > > Devrim GÜNDÜZ wrote:
> > > > Hi,
> > > >
> > > > On Mon, 2008-01-07 at 17:12 -0500, Tom Lane wrote:
> > > > > if no serious bugs come up this week, core is thinking of wrapping
> > > > > 8.3.0 at the end of the week,
> > > >
> > > > Please give the packagers and other people one more week break between
> > > > two releases. We all had a busy weekend.
> > >
> > > We have thousands of people waiting for the 8.3 release.  It seems odd
> > > to hold up 8.3 because the packagers are too busy.  Perhaps we need more
> > > packagers or release the source and have the binary packages ready
> > later.
> >
> > Also to be fair I've seen no real announcement of RC1, probabbly because
> > it's
> > been bundled in with "security releases" for the other branched.. can we
> > maybe do an RC1 announcement, let people actually test the RC before we
> > push
> > a gold ?
> >
> >
> >
> Indeed the website still says we are on Beta 4. I did not even know RC1 was
> out until I saw this thread this morning.

RC1 isn't out. What's available on the ftp site is a preliminary version of
it, that should not be used.

//Magnus

Magnus Hagander <magnus@hagander.net> writes:
> On Tue, Jan 08, 2008 at 09:47:15AM +0000, Peter Childs wrote:
>> Indeed the website still says we are on Beta 4. I did not even know RC1 was
>> out until I saw this thread this morning.

> RC1 isn't out. What's available on the ftp site is a preliminary version of
> it, that should not be used.

Eh?  On what do you base that statement?  RC1 is what it is, there is
nothing "preliminary" about it; and anyone who is running an 8.3beta
installation that is at all security-exposed would be well advised to
get onto it ASAP.

We didn't include 8.3RC1 in the security announcement because Josh
wanted to make a separate announcement for it, but from every
perspective except the PR one, it's out.
        regards, tom lane

On Tue, Jan 08, 2008 at 11:23:13AM -0500, Tom Lane wrote:
> Magnus Hagander <magnus@hagander.net> writes:
> > On Tue, Jan 08, 2008 at 09:47:15AM +0000, Peter Childs wrote:
> >> Indeed the website still says we are on Beta 4. I did not even know RC1 was
> >> out until I saw this thread this morning.
> 
> > RC1 isn't out. What's available on the ftp site is a preliminary version of
> > it, that should not be used.
> 
> Eh?  On what do you base that statement?  RC1 is what it is, there is
> nothing "preliminary" about it; and anyone who is running an 8.3beta
> installation that is at all security-exposed would be well advised to
> get onto it ASAP.
> 
> We didn't include 8.3RC1 in the security announcement because Josh
> wanted to make a separate announcement for it, but from every
> perspective except the PR one, it's out.

There has been no annonucement whatsoever. Our web site stll claims beta4
is the current version. I was under the impression that this tarball, like
all others, are considered preliminary until announced one way or another.

And I was under the impression that we *were* going to announce RC1
sometime this week. And that we just didn't want to do it at the exact same
time as we did the backbranch releases. If not then we relly need to update
the website with this information. But we don't normally release RC or beta
(or any other, for that matter) releases without telling anybody, so IMHO
it seems like a bad idea...

//Magnus

Magnus Hagander <magnus@hagander.net> writes:
> On Tue, Jan 08, 2008 at 11:23:13AM -0500, Tom Lane wrote:
>> We didn't include 8.3RC1 in the security announcement because Josh
>> wanted to make a separate announcement for it, but from every
>> perspective except the PR one, it's out.

> There has been no annonucement whatsoever. Our web site stll claims beta4
> is the current version. I was under the impression that this tarball, like
> all others, are considered preliminary until announced one way or another.

Uh, no, that isn't the project policy.  If we were to find some fatal
problem in RC1 at this point, we'd spin an RC2, precisely because RC1
has been up on the servers for a couple days now and confusion would
inevitably result if we tried to redefine what RC1 was.

The lack of an announcement is not my bailiwick, but tarball-making
is.  Once a tarball appears in the public FTP directories, it's
official, and there's no reason to discourage people from using it.
        regards, tom lane

Tom Lane wrote:
> Magnus Hagander <magnus@hagander.net> writes:
>> On Tue, Jan 08, 2008 at 11:23:13AM -0500, Tom Lane wrote:
>>> We didn't include 8.3RC1 in the security announcement because Josh
>>> wanted to make a separate announcement for it, but from every
>>> perspective except the PR one, it's out.
> 
>> There has been no annonucement whatsoever. Our web site stll claims beta4
>> is the current version. I was under the impression that this tarball, like
>> all others, are considered preliminary until announced one way or another.
> 
> Uh, no, that isn't the project policy.  If we were to find some fatal
> problem in RC1 at this point, we'd spin an RC2, precisely because RC1
> has been up on the servers for a couple days now and confusion would
> inevitably result if we tried to redefine what RC1 was.
> 
> The lack of an announcement is not my bailiwick, but tarball-making
> is.  Once a tarball appears in the public FTP directories, it's
> official, and there's no reason to discourage people from using it.

Ok. That's not how I thought it was, and we have previously pulled 
releases that were available as tarballs and re-released them with the 
same version number. Sure, it was a while ago, but it has happened, and 
if I'm not completely mistaken, more than once.

Anyway. My apologies for the incorrect statement in that case, and just 
let me (or us, really) know when it's time to update the webpage.

//Magnus

Tom Lane a écrit :
> Magnus Hagander <magnus@hagander.net> writes:
>   
>> On Tue, Jan 08, 2008 at 11:23:13AM -0500, Tom Lane wrote:
>>     
>>> We didn't include 8.3RC1 in the security announcement because Josh
>>> wanted to make a separate announcement for it, but from every
>>> perspective except the PR one, it's out.
>>>       
>
>   
>> There has been no annonucement whatsoever. Our web site stll claims beta4
>> is the current version. I was under the impression that this tarball, like
>> all others, are considered preliminary until announced one way or another.
>>     
>
> Uh, no, that isn't the project policy.  If we were to find some fatal
> problem in RC1 at this point, we'd spin an RC2, precisely because RC1
> has been up on the servers for a couple days now and confusion would
> inevitably result if we tried to redefine what RC1 was.
>   
For example, Martin Pitt push rc1 on Sat, 05 Jan 2008 19:19:46 +0100 
into Debian.
> The lack of an announcement is not my bailiwick, but tarball-making
> is.  Once a tarball appears in the public FTP directories, it's
> official, and there's no reason to discourage people from using it.
>
>             regards, tom lane
>
> ---------------------------(end of broadcast)---------------------------
> TIP 6: explain analyze is your friend
>   


-- 
Cédric Villemain
Administrateur de Base de Données
Cel: +33 (0)6 74 15 56 53
http://dalibo.com - http://dalibo.org

Magnus Hagander wrote:
> Tom Lane wrote:
>> Magnus Hagander <magnus@hagander.net> writes:
>>> On Tue, Jan 08, 2008 at 11:23:13AM -0500, Tom Lane wrote:
>>>> We didn't include 8.3RC1 in the security announcement because Josh
>>>> wanted to make a separate announcement for it, but from every
>>>> perspective except the PR one, it's out.
>>
>>> There has been no annonucement whatsoever. Our web site stll claims 
>>> beta4
>>> is the current version. I was under the impression that this tarball, 
>>> like
>>> all others, are considered preliminary until announced one way or 
>>> another.
>>
>> Uh, no, that isn't the project policy.  If we were to find some fatal
>> problem in RC1 at this point, we'd spin an RC2, precisely because RC1
>> has been up on the servers for a couple days now and confusion would
>> inevitably result if we tried to redefine what RC1 was.
>>
>> The lack of an announcement is not my bailiwick, but tarball-making
>> is.  Once a tarball appears in the public FTP directories, it's
>> official, and there's no reason to discourage people from using it.
> 
> Ok. That's not how I thought it was, and we have previously pulled 
> releases that were available as tarballs and re-released them with the 
> same version number. Sure, it was a while ago, but it has happened, and 
> if I'm not completely mistaken, more than once.

yeah that is my recollection too ...

> 
> Anyway. My apologies for the incorrect statement in that case, and just 
> let me (or us, really) know when it's time to update the webpage.

yeah while several of us noticed that new RC1 tarballs went up it was 
not immediatly clear to me that "we" can now announce RC1 on the website.
Confusion on our major release policy during the 8.2 release lead to the 
following document on the wiki:

http://developer.postgresql.org/index.php/MajorReleaseTimeline

if this page(which is only a shell but we could use that as an 
opportunity to improve it) has still any relevance - where exactly on 
that timeline are we now ?


Stefan

All,

> >> There has been no annonucement whatsoever. Our web site stll claims
> >> beta4 is the current version. I was under the impression that this
> >> tarball, like all others, are considered preliminary until announced
> >> one way or another.

Sorry about that.  I was confused and thought we were deliberately waiting 
24 hours between the security release and the RC1 announcement.  It's out 
now.

-- 
--Josh

Josh Berkus
PostgreSQL @ Sun
San Francisco

Tom Lane wrote:
> Joe Conway <mail@joeconway.com> writes:
>   
>> Did you want me to work on this? I could probably put some time into it 
>> this coming weekend.
>>     
>
> I'll try to get to it before that --- if no serious bugs come up this
> week, core is thinking of wrapping 8.3.0 at the end of the week, so
> it'd be nice to have this dealt with sooner than that.
>
>             regards, tom lane
>
>   

CREATE AGGREGATE array_concat(anyarray) ( SFUNC=array_cat, STYPE=anyarray
);

CREATE AGGREGATE array_build(anyelement) ( SFUNC=array_append, STYPE=anyarray
);

Sorry for previous message having no comments.

Just remark:
These aggregates created successfuly both in 8.2 and 8.3beta4:

CREATE AGGREGATE array_concat(anyarray) (SFUNC=array_cat,STYPE=anyarray
);

CREATE AGGREGATE array_build(anyelement) (SFUNC=array_append,STYPE=anyarray
);

But aggregate from first letter does not:

create aggregate build_group(anyelement, int4) ( SFUNC= add_group, STYPE = anyarray
);


Excuse me for being noisy and bad English.

Joe Conway <mail@joeconway.com> writes:
> Tom Lane wrote:
>> I think we could make enforce_generic_type_consistency() clearer by
>> adding an additional argument "bool allow_poly" which specifies
>> whether polymorphic "actual" argument and result types are allowed.

> This sounds like a reasonable plan to me.

>> Although this problem really goes quite far back, I think it's probably
>> not interesting to back-patch further than 8.2, because AFAICS the
>> interesting cases involve aggregates with more than one argument.

> I agree, especially since this is the first time anyone has complained.

I've applied a patch along these lines, although I desisted from
back-patching it.  It seems a bit like a new feature, and also I'm not
100% sure we have all the bases covered even yet.
        regards, tom lane