Re: Crosstab Problems

Lists: pgsql-generalpgsql-patches
From: Stefan Schwarzer <stefan(dot)schwarzer(at)grid(dot)unep(dot)ch>
To: pgsql-general(at)postgresql(dot)org
Subject: Crosstab Problems
Date: 2007-10-18 13:24:17
Message-ID: 4E6E765B-2899-4B85-9131-A8847FB06305@grid.unep.ch
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-general pgsql-patches

Hi there,

successfully installed the tablefunc package.

Now, I would like to transform this kind of result based on a normal
SQL:

c_name | year | value
---------------------------------------
Germany | 2001 | 123
Germany | 2002 | 125
Germany | 2003 | 128
Germany | 2004 | 132
Germany | 2005 | 135

Italy | 2001 | 412
Italy | 2002 | 429
Italy | 2003 | 456
Italy | 2004 | 465
Italy | 2005 | 477

to this one:

c_name | 2001 | 2002 | 2003 | 2004 | 2005
------------------------------------------------------------------------
Germany | 123 | 125 .....
Italy | 412 | .....

I use this SQL statement:

SELECT
*
FROM
crosstab(
'SELECT
c.name AS name,
year_start AS year,
value
FROM
agri_area AS d
LEFT JOIN
countries AS c ON c.id = id_country
WHERE
year_start = 2003 OR
year_start = 2002 OR
year_start = 2001
ORDER BY
name ASC,
year_start ASC;'
, 3)
AS ct(name varchar, y_2003 numeric, y_2002 numeric, y_2001 numeric)

I had a couple of problems getting there. But now that I have the
feeling that this is OK, it tells me this:

server closed the connection unexpectedly
This probably means the server terminated abnormally
before or while processing the request.

Can anyone tell me why? And how to get it right? Thanks for any advice!

Stef


From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Stefan Schwarzer <stefan(dot)schwarzer(at)grid(dot)unep(dot)ch>
Cc: pgsql-general(at)postgresql(dot)org
Subject: Re: Crosstab Problems
Date: 2007-10-18 14:51:18
Message-ID: 23401.1192719078@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-general pgsql-patches

Stefan Schwarzer <stefan(dot)schwarzer(at)grid(dot)unep(dot)ch> writes:
> I had a couple of problems getting there. But now that I have the
> feeling that this is OK, it tells me this:

> server closed the connection unexpectedly

Could you provide a self-contained test case for this? There's not
really enough information here for someone else to duplicate the
problem. Also, which PG version are you using?

regards, tom lane


From: Stefan Schwarzer <stefan(dot)schwarzer(at)grid(dot)unep(dot)ch>
To: pgsql-general(at)postgresql(dot)org
Subject: Re: Crosstab Problems
Date: 2007-10-18 15:17:22
Message-ID: CB88C718-3198-417B-8B7C-23E46F8C921C@grid.unep.ch
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-general pgsql-patches

> Could you provide a self-contained test case for this? There's not
> really enough information here for someone else to duplicate the
> problem. Also, which PG version are you using?

Wasn't sure what you ment with "a self containted test case". Is it
the raw data?

Here is a SQL dump for the table. One can just neglect the JOIN with
the countries table (which just replaces the country id with the
country name):

http://geodata.grid.unep.ch/download/sql_agri_area.sql.zip

But when re-doing the query now without the JOIN, it works (almost):

SELECT
*
FROM
crosstab(
'SELECT
id_country AS id,
year_start AS year,
value
FROM
agri_area AS d
WHERE
year_start = 2003 OR year_start = 2002 OR year_start =
2001 ORDER BY year_start ASC, id_country ASC;'
, 3)
AS ct(id int2, y_2003 numeric, y_2002 numeric, y_2001 numeric)

Now, the problem is that it lists three times the IDs, and only the
first year column is filled with values. The other two year columns
stay empty.

Thanks for any advice!

Stef


From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Stefan Schwarzer <stefan(dot)schwarzer(at)grid(dot)unep(dot)ch>
Cc: pgsql-general(at)postgresql(dot)org, Joe Conway <mail(at)joeconway(dot)com>
Subject: Re: Crosstab Problems
Date: 2007-10-18 15:47:37
Message-ID: 24199.1192722457@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-general pgsql-patches

Stefan Schwarzer <stefan(dot)schwarzer(at)grid(dot)unep(dot)ch> writes:
> Here is a SQL dump for the table. One can just neglect the JOIN with
> the countries table (which just replaces the country id with the
> country name):
> http://geodata.grid.unep.ch/download/sql_agri_area.sql.zip
> But when re-doing the query now without the JOIN, it works (almost):

OK, after poking at it, it seems that crosstab() isn't prepared for
null rowids. I can reproduce the crash without any data:

contrib_regression=# select * from crosstab(
'SELECT null::text as name, 10 as year, 42 as value', 3)
as ct(name text, year int, value int);
server closed the connection unexpectedly

Backtrace looks like

#0 0xc008774c in ?? () from /usr/lib/libc.1
#1 0x3eb0bc in MemoryContextStrdup (context=0x40167048, string=0x0)
at mcxt.c:662
#2 0xc0a5f2e4 in crosstab (fcinfo=0x7b03b858) at tablefunc.c:539
#3 0x239e24 in ExecMakeTableFunctionResult (funcexpr=0x401615e8,
econtext=0x401611f0, expectedDesc=0x401613a0, returnDesc=0x7b03b7d8)
at execQual.c:1566
#4 0x24d264 in FunctionNext (node=0x40161160) at nodeFunctionscan.c:68
#5 0x23ed8c in ExecScan (node=0x40161160,
accessMtd=0x400170b2 <DINFINITY+3218>) at execScan.c:68
#6 0x24d2c4 in ExecFunctionScan (node=0x40167048) at nodeFunctionscan.c:109

so it's trying to pstrdup a null result from SPI_getvalue.

Obviously it shouldn't crash, but I'm not sure what it *should* do in
this case. Joe?

In the meantime, it appears that you want to not use a LEFT JOIN here,
or else maybe COALESCE(c.name, '') so that a null isn't returned to
crosstab.

regards, tom lane


From: "Scott Marlowe" <scott(dot)marlowe(at)gmail(dot)com>
To: "Stefan Schwarzer" <stefan(dot)schwarzer(at)grid(dot)unep(dot)ch>
Cc: pgsql-general(at)postgresql(dot)org
Subject: Re: Crosstab Problems
Date: 2007-10-18 16:05:35
Message-ID: dcc563d10710180905n61aa8584jf835cc37bcd61412@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-general pgsql-patches

On 10/18/07, Stefan Schwarzer <stefan(dot)schwarzer(at)grid(dot)unep(dot)ch> wrote:
> > Could you provide a self-contained test case for this? There's not
> > really enough information here for someone else to duplicate the
> > problem. Also, which PG version are you using?
>
> Wasn't sure what you ment with "a self containted test case". Is it
> the raw data?
>
> Here is a SQL dump for the table. One can just neglect the JOIN with
> the countries table (which just replaces the country id with the
> country name):
>
> http://geodata.grid.unep.ch/download/sql_agri_area.sql.zip
>
> But when re-doing the query now without the JOIN, it works (almost):
>
> SELECT
> *
> FROM
> crosstab(
> 'SELECT
> id_country AS id,
> year_start AS year,
> value
> FROM
> agri_area AS d
> WHERE
> year_start = 2003 OR year_start = 2002 OR year_start =
> 2001 ORDER BY year_start ASC, id_country ASC;'
> , 3)
> AS ct(id int2, y_2003 numeric, y_2002 numeric, y_2001 numeric)
>
> Now, the problem is that it lists three times the IDs, and only the
> first year column is filled with values. The other two year columns
> stay empty.

I use crosstab for a rather large weekly report in our db and it works
fine, however, you can't feel it nulls. It needs all the holes filled
in, so to speak.

In mine I had to use generate_series to make sure all the rows were
there, then coalesce to make sure there were no nulls. You might need
to do something like that in yours. I'm trying it out now.


From: "Scott Marlowe" <scott(dot)marlowe(at)gmail(dot)com>
To: "Stefan Schwarzer" <stefan(dot)schwarzer(at)grid(dot)unep(dot)ch>
Cc: pgsql-general(at)postgresql(dot)org
Subject: Re: Crosstab Problems
Date: 2007-10-18 16:36:24
Message-ID: dcc563d10710180936o61f78fc5n883757783f70e2f1@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-general pgsql-patches

On 10/18/07, Stefan Schwarzer <stefan(dot)schwarzer(at)grid(dot)unep(dot)ch> wrote:
> But when re-doing the query now without the JOIN, it works (almost):
>
> SELECT
> *
> FROM
> crosstab(
> 'SELECT
> id_country AS id,
> year_start AS year,
> value
> FROM
> agri_area AS d
> WHERE
> year_start = 2003 OR year_start = 2002 OR year_start =
> 2001 ORDER BY year_start ASC, id_country ASC;'
> , 3)
> AS ct(id int2, y_2003 numeric, y_2002 numeric, y_2001 numeric)
>
> Now, the problem is that it lists three times the IDs, and only the
> first year column is filled with values. The other two year columns
> stay empty.

You missed this point in the docs:

Notes

1. The sql result must be ordered by 1,2.
Change your order by to that and it works fine.


From: Joe Conway <mail(at)joeconway(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Stefan Schwarzer <stefan(dot)schwarzer(at)grid(dot)unep(dot)ch>, pgsql-general(at)postgresql(dot)org
Subject: Re: Crosstab Problems
Date: 2007-10-18 18:37:59
Message-ID: 4717A807.1030209@joeconway.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-general pgsql-patches

Tom Lane wrote:
> so it's trying to pstrdup a null result from SPI_getvalue.
>
> Obviously it shouldn't crash, but I'm not sure what it *should* do in
> this case. Joe?

The row is pretty useless without a rowid in this context -- it seems
like the best thing to do would be to skip those rows entirely. Of
course you could argue I suppose that it ought to throw an ERROR and
bail out entirely. Maybe a good compromise would be to skip the row but
throw a NOTICE?

Joe


From: Jorge Godoy <jgodoy(at)gmail(dot)com>
To: pgsql-general(at)postgresql(dot)org
Cc: Joe Conway <mail(at)joeconway(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Stefan Schwarzer <stefan(dot)schwarzer(at)grid(dot)unep(dot)ch>
Subject: Re: Crosstab Problems
Date: 2007-10-18 23:02:54
Message-ID: 200710182102.56340.jgodoy@gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-general pgsql-patches

Em Thursday 18 October 2007 16:37:59 Joe Conway escreveu:
> Tom Lane wrote:
> > so it's trying to pstrdup a null result from SPI_getvalue.
> >
> > Obviously it shouldn't crash, but I'm not sure what it *should* do in
> > this case. Joe?
>
> The row is pretty useless without a rowid in this context -- it seems
> like the best thing to do would be to skip those rows entirely. Of
> course you could argue I suppose that it ought to throw an ERROR and
> bail out entirely. Maybe a good compromise would be to skip the row but
> throw a NOTICE?

If I were using it and having this problem I'd rather have an ERROR. It isn't
uncommon for people not look at their logs and it isn't uncommon for them
just run command from some language using a database adapter that might not
return the NOTICE output. The ERROR wouldn't pass unnoticed.

--
Jorge Godoy <jgodoy(at)gmail(dot)com>


From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Jorge Godoy <jgodoy(at)gmail(dot)com>
Cc: pgsql-general(at)postgresql(dot)org, Joe Conway <mail(at)joeconway(dot)com>, Stefan Schwarzer <stefan(dot)schwarzer(at)grid(dot)unep(dot)ch>
Subject: Re: Crosstab Problems
Date: 2007-10-19 03:09:12
Message-ID: 3161.1192763352@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-general pgsql-patches

Jorge Godoy <jgodoy(at)gmail(dot)com> writes:
> Em Thursday 18 October 2007 16:37:59 Joe Conway escreveu:
>> The row is pretty useless without a rowid in this context -- it seems
>> like the best thing to do would be to skip those rows entirely. Of
>> course you could argue I suppose that it ought to throw an ERROR and
>> bail out entirely. Maybe a good compromise would be to skip the row but
>> throw a NOTICE?

> If I were using it and having this problem I'd rather have an ERROR.

I can think of four reasonably credible alternatives:

1. Treat NULL rowid as a category in its own right. This would conform
with the behavior of GROUP BY and DISTINCT, for instance.

2. Throw an ERROR if NULL rowid is seen.

3. Throw a NOTICE or WARNING (hopefully only one message not repeated
ones) if NULL rowid is seen, then ignore the row.

4. Silently ignore rows with NULL rowid.

Not being a heavy user of crosstab(), I'm not sure which of these is the
most appropriate, but #1 seems the most defensible from a theoretical
perspective.

Since the bug has gone undiscovered this long, it seems obvious that
not too many people actually try to feed null rowids to crosstab; so
expending a lot of effort to fix it is probably not reasonable.
If you don't like #1 I'd vote for #2 second.

regards, tom lane


From: Joe Conway <mail(at)joeconway(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Jorge Godoy <jgodoy(at)gmail(dot)com>, pgsql-general(at)postgresql(dot)org, Stefan Schwarzer <stefan(dot)schwarzer(at)grid(dot)unep(dot)ch>
Subject: Re: Crosstab Problems
Date: 2007-10-19 04:09:33
Message-ID: 47182DFD.6090703@joeconway.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-general pgsql-patches

Tom Lane wrote:
> Jorge Godoy <jgodoy(at)gmail(dot)com> writes:
>> Em Thursday 18 October 2007 16:37:59 Joe Conway escreveu:
>>> The row is pretty useless without a rowid in this context -- it seems
>>> like the best thing to do would be to skip those rows entirely. Of
>>> course you could argue I suppose that it ought to throw an ERROR and
>>> bail out entirely. Maybe a good compromise would be to skip the row but
>>> throw a NOTICE?
>
>> If I were using it and having this problem I'd rather have an ERROR.
>
> I can think of four reasonably credible alternatives:
>
> 1. Treat NULL rowid as a category in its own right. This would conform
> with the behavior of GROUP BY and DISTINCT, for instance.
>
> 2. Throw an ERROR if NULL rowid is seen.

> Not being a heavy user of crosstab(), I'm not sure which of these is the
> most appropriate, but #1 seems the most defensible from a theoretical
> perspective.
>
> Since the bug has gone undiscovered this long, it seems obvious that
> not too many people actually try to feed null rowids to crosstab; so
> expending a lot of effort to fix it is probably not reasonable.
> If you don't like #1 I'd vote for #2 second.

Hadn't really thought about #1, but now that you mention it, it does
make sense. #1 gets my vote too. I'll pick this up next week if that's OK.

Joe


From: Stefan Schwarzer <stefan(dot)schwarzer(at)grid(dot)unep(dot)ch>
To: pgsql-general(at)postgresql(dot)org
Subject: Re: Crosstab Problems
Date: 2007-10-19 08:45:16
Message-ID: 73994295-1801-493D-B48C-5783FAA863DC@grid.unep.ch
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-general pgsql-patches

>> But when re-doing the query now without the JOIN, it works (almost):
>>
>> SELECT
>> *
>> FROM
>> crosstab(
>> 'SELECT
>> id_country AS id,
>> year_start AS year,
>> value
>> FROM
>> agri_area AS d
>> WHERE
>> year_start = 2003 OR year_start = 2002 OR year_start =
>> 2001 ORDER BY year_start ASC, id_country ASC;'
>> , 3)
>> AS ct(id int2, y_2003 numeric, y_2002 numeric, y_2001 numeric)
>>
>> Now, the problem is that it lists three times the IDs, and only the
>> first year column is filled with values. The other two year columns
>> stay empty.
>
> You missed this point in the docs:
>
> Notes
>
> 1. The sql result must be ordered by 1,2.
> Change your order by to that and it works fine.

Oh, great. No, haven't seen it. Now it works. Thanks a lot!

Just for the completeness, I attach the SQL.

SELECT
*
FROM
crosstab(
'SELECT
COALESCE(c.name, ''''),
year_start AS year,
value
FROM
agri_area AS d
LEFT JOIN
countries AS c ON c.id = id_country
WHERE
year_start = 2003 OR year_start = 2002 OR year_start = 2001
GROUP BY
name, id_country, year_start, value
ORDER BY 1,2;'
, 3)
AS ct(name varchar, y_2003 numeric, y_2002 numeric, y_2001 numeric)


From: Gregory Stark <stark(at)enterprisedb(dot)com>
To: "Tom Lane" <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: "Jorge Godoy" <jgodoy(at)gmail(dot)com>, <pgsql-general(at)postgresql(dot)org>, "Joe Conway" <mail(at)joeconway(dot)com>, "Stefan Schwarzer" <stefan(dot)schwarzer(at)grid(dot)unep(dot)ch>
Subject: Re: Crosstab Problems
Date: 2007-10-19 10:48:22
Message-ID: 87wstjmlc9.fsf@oxford.xeocode.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-general pgsql-patches

"Tom Lane" <tgl(at)sss(dot)pgh(dot)pa(dot)us> writes:

> 3. Throw a NOTICE or WARNING (hopefully only one message not repeated
> ones) if NULL rowid is seen, then ignore the row.

From my experience with OLTP I don't like this one. A warning for DML is
effectively the same as an error if you're running thousands of queries per
minute. The logs fill up and even if you filter the logs it imposes extra
run-time overhead. You end up having to avoid the warning just as if it had
been an error.

--
Gregory Stark
EnterpriseDB http://www.enterprisedb.com


From: Joe Conway <mail(at)joeconway(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Jorge Godoy <jgodoy(at)gmail(dot)com>, pgsql-general(at)postgresql(dot)org, Stefan Schwarzer <stefan(dot)schwarzer(at)grid(dot)unep(dot)ch>
Subject: Re: Crosstab Problems
Date: 2007-10-25 02:26:16
Message-ID: 471FFEC8.4040409@joeconway.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-general pgsql-patches

Tom Lane wrote:
> Jorge Godoy <jgodoy(at)gmail(dot)com> writes:
>> Em Thursday 18 October 2007 16:37:59 Joe Conway escreveu:
>>> The row is pretty useless without a rowid in this context -- it seems
>>> like the best thing to do would be to skip those rows entirely. Of
>>> course you could argue I suppose that it ought to throw an ERROR and
>>> bail out entirely. Maybe a good compromise would be to skip the row but
>>> throw a NOTICE?
>
>> If I were using it and having this problem I'd rather have an ERROR.
>
> I can think of four reasonably credible alternatives:
>
> 1. Treat NULL rowid as a category in its own right. This would conform
> with the behavior of GROUP BY and DISTINCT, for instance.

> 4. Silently ignore rows with NULL rowid.

After looking closer I realized that #4 was my original intention, and
there was even code attempting to implement it, but just not very well ;-(.

In any case, the attached changes the behavior to #1 for both flavors of
crosstab (the original crosstab(text, int) and the usually more useful
crosstab(text, text)).

It is appropriate for 8.3 but not back-patching as it changes behavior
in a non-backward compatible way and is probably too invasive anyway.
I'll do something much simpler just to prevent crashing for 8.2 and earlier.

If there are no objections I'll apply Thursday.

Joe

Attachment Content-Type Size
tablefunc.1.diff text/x-patch 6.7 KB

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Joe Conway <mail(at)joeconway(dot)com>
Cc: Jorge Godoy <jgodoy(at)gmail(dot)com>, pgsql-general(at)postgresql(dot)org, Stefan Schwarzer <stefan(dot)schwarzer(at)grid(dot)unep(dot)ch>
Subject: Re: Crosstab Problems
Date: 2007-10-25 04:37:26
Message-ID: 6329.1193287046@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-general pgsql-patches

Joe Conway <mail(at)joeconway(dot)com> writes:
> Tom Lane wrote:
>> 1. Treat NULL rowid as a category in its own right. This would conform
>> with the behavior of GROUP BY and DISTINCT, for instance.

> In any case, the attached changes the behavior to #1 for both flavors of
> crosstab (the original crosstab(text, int) and the usually more useful
> crosstab(text, text)).

> It is appropriate for 8.3 but not back-patching as it changes behavior
> in a non-backward compatible way and is probably too invasive anyway.

Um, if the previous code crashed in this case, why would you worry about
being backward-compatible with it? You're effectively changing the
behavior anyway, so you might as well make it do what you've decided is
the right thing.

regards, tom lane


From: "Scott Marlowe" <scott(dot)marlowe(at)gmail(dot)com>
To: "Tom Lane" <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: "Joe Conway" <mail(at)joeconway(dot)com>, "Jorge Godoy" <jgodoy(at)gmail(dot)com>, pgsql-general(at)postgresql(dot)org, "Stefan Schwarzer" <stefan(dot)schwarzer(at)grid(dot)unep(dot)ch>
Subject: Re: Crosstab Problems
Date: 2007-10-25 14:29:33
Message-ID: dcc563d10710250729s45cc418apf4044003b077f0cb@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-general pgsql-patches

On 10/24/07, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Joe Conway <mail(at)joeconway(dot)com> writes:
> > Tom Lane wrote:
> >> 1. Treat NULL rowid as a category in its own right. This would conform
> >> with the behavior of GROUP BY and DISTINCT, for instance.
>
> > In any case, the attached changes the behavior to #1 for both flavors of
> > crosstab (the original crosstab(text, int) and the usually more useful
> > crosstab(text, text)).
>
> > It is appropriate for 8.3 but not back-patching as it changes behavior
> > in a non-backward compatible way and is probably too invasive anyway.
>
> Um, if the previous code crashed in this case, why would you worry about
> being backward-compatible with it? You're effectively changing the
> behavior anyway, so you might as well make it do what you've decided is
> the right thing.

As a crosstab user, I agree with Tom.


From: Reg Me Please <regmeplease(at)gmail(dot)com>
To: pgsql-general(at)postgresql(dot)org
Cc: "Scott Marlowe" <scott(dot)marlowe(at)gmail(dot)com>, "Tom Lane" <tgl(at)sss(dot)pgh(dot)pa(dot)us>, "Joe Conway" <mail(at)joeconway(dot)com>, "Jorge Godoy" <jgodoy(at)gmail(dot)com>, "Stefan Schwarzer" <stefan(dot)schwarzer(at)grid(dot)unep(dot)ch>
Subject: Re: Crosstab Problems
Date: 2007-10-25 14:46:26
Message-ID: 200710251646.27060.regmeplease@gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-general pgsql-patches

Il Thursday 25 October 2007 16:29:33 Scott Marlowe ha scritto:
> On 10/24/07, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> > Joe Conway <mail(at)joeconway(dot)com> writes:
> > > Tom Lane wrote:
> > >> 1. Treat NULL rowid as a category in its own right. This would
> > >> conform with the behavior of GROUP BY and DISTINCT, for instance.
> > >
> > > In any case, the attached changes the behavior to #1 for both flavors
> > > of crosstab (the original crosstab(text, int) and the usually more
> > > useful crosstab(text, text)).
> > >
> > > It is appropriate for 8.3 but not back-patching as it changes behavior
> > > in a non-backward compatible way and is probably too invasive anyway.
> >
> > Um, if the previous code crashed in this case, why would you worry about
> > being backward-compatible with it? You're effectively changing the
> > behavior anyway, so you might as well make it do what you've decided is
> > the right thing.
>
> As a crosstab user, I agree with Tom.

If I can throw in my EUR 0.01 contrib, I would agree with Joe (thanks for your
wonderful crosstab).
If crosstab in 8.3 will have a different behaviour *and* it's not part of the
core features, then I'd prefer to correct it.
In any case developers will have to cope with discrepancies when going to 8.3
and you can bet they won't remain with 8.2 when 8.3 will be rolled out.

And, by the way, why not including the crosstab as a standard feature?
I think it deserves it!


From: Joe Conway <mail(at)joeconway(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Jorge Godoy <jgodoy(at)gmail(dot)com>, scott(dot)marlowe(at)gmail(dot)com, Stefan Schwarzer <stefan(dot)schwarzer(at)grid(dot)unep(dot)ch>, regmeplease(at)gmail(dot)com, "Patches (PostgreSQL)" <pgsql-patches(at)postgresql(dot)org>
Subject: Re: [GENERAL] Crosstab Problems
Date: 2007-10-25 22:36:24
Message-ID: 47211A68.4050201@joeconway.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-general pgsql-patches

Tom Lane wrote:
> Joe Conway <mail(at)joeconway(dot)com> writes:
>> Tom Lane wrote:
>>> 1. Treat NULL rowid as a category in its own right. This would conform
>>> with the behavior of GROUP BY and DISTINCT, for instance.
>
>> In any case, the attached changes the behavior to #1 for both flavors of
>> crosstab (the original crosstab(text, int) and the usually more useful
>> crosstab(text, text)).
>
>> It is appropriate for 8.3 but not back-patching as it changes behavior
>> in a non-backward compatible way and is probably too invasive anyway.
>
> Um, if the previous code crashed in this case, why would you worry about
> being backward-compatible with it? You're effectively changing the
> behavior anyway, so you might as well make it do what you've decided is
> the right thing.

Well, maybe the attached patches better explain what I mean.

In the case of the 8.2 patch, a very small code change allows new
regression data including NULL rowids to:

1) not crash
2) have no impact otherwise

The much bigger 8.3 patch shows that for the very same new regression
data, there is a significant impact on the output (i.e. NULL rowids get
their own output row as discussed).

I'm still leaning toward applying the 8.2 patch for back branches but
I'll bow to the general consensus.

Joe

Attachment Content-Type Size
tablefunc.head.diff text/x-patch 24.2 KB
tablefunc.8.2.diff text/x-patch 3.5 KB

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Joe Conway <mail(at)joeconway(dot)com>
Cc: Jorge Godoy <jgodoy(at)gmail(dot)com>, scott(dot)marlowe(at)gmail(dot)com, Stefan Schwarzer <stefan(dot)schwarzer(at)grid(dot)unep(dot)ch>, regmeplease(at)gmail(dot)com, "Patches (PostgreSQL)" <pgsql-patches(at)postgresql(dot)org>
Subject: Re: [GENERAL] Crosstab Problems
Date: 2007-10-26 01:43:48
Message-ID: 397.1193363028@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-general pgsql-patches

Joe Conway <mail(at)joeconway(dot)com> writes:
> Well, maybe the attached patches better explain what I mean.

> In the case of the 8.2 patch, a very small code change allows new
> regression data including NULL rowids to:
> 1) not crash
> 2) have no impact otherwise

> The much bigger 8.3 patch shows that for the very same new regression
> data, there is a significant impact on the output (i.e. NULL rowids get
> their own output row as discussed).

> I'm still leaning toward applying the 8.2 patch for back branches but
> I'll bow to the general consensus.

I'd vote for the bigger patch all the way back. The smaller patch has
nothing to recommend it except being smaller. It replaces the crash
with a behavior that will change in 8.3, thus creating a potential
portability issue for users of (post-repair) back branches. Why not
get it right the first time?

A couple of minor thoughts:

* You could reduce the ugliness of many of the tests by introducing a
variant strcmp function that does the "right" things with NULL inputs.
It might also be worth adding a variant pstrdup that takes a NULL.

* Surely this bit:

> xpfree(lastrowid);
> ! if (rowid)
> ! lastrowid = pstrdup(rowid);
> }

needs to be:

if (rowid)
lastrowid = pstrdup(rowid);
else
lastrowid = NULL;

no? (Again the variant pstrdup would save some notation)

regards, tom lane

PS: I hear things are pretty crazy out your way -- hope the fire's
not too close to you.


From: Joe Conway <mail(at)joeconway(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Jorge Godoy <jgodoy(at)gmail(dot)com>, scott(dot)marlowe(at)gmail(dot)com, Stefan Schwarzer <stefan(dot)schwarzer(at)grid(dot)unep(dot)ch>, regmeplease(at)gmail(dot)com, "Patches (PostgreSQL)" <pgsql-patches(at)postgresql(dot)org>
Subject: Re: [GENERAL] Crosstab Problems
Date: 2007-10-26 04:40:04
Message-ID: 47216FA4.4080905@joeconway.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-general pgsql-patches

Tom Lane wrote:
> Joe Conway <mail(at)joeconway(dot)com> writes:
>> Well, maybe the attached patches better explain what I mean.
>
>> In the case of the 8.2 patch, a very small code change allows new
>> regression data including NULL rowids to:
>> 1) not crash
>> 2) have no impact otherwise
>
>> The much bigger 8.3 patch shows that for the very same new regression
>> data, there is a significant impact on the output (i.e. NULL rowids get
>> their own output row as discussed).
>
>> I'm still leaning toward applying the 8.2 patch for back branches but
>> I'll bow to the general consensus.
>
> I'd vote for the bigger patch all the way back. The smaller patch has
> nothing to recommend it except being smaller. It replaces the crash
> with a behavior that will change in 8.3, thus creating a potential
> portability issue for users of (post-repair) back branches. Why not
> get it right the first time?

OK, I can live with that.

> A couple of minor thoughts:
>
> * You could reduce the ugliness of many of the tests by introducing a
> variant strcmp function that does the "right" things with NULL inputs.
> It might also be worth adding a variant pstrdup that takes a NULL.

I had thoughts along those lines -- it would certainly make the code
more readable. I'll go ahead and do that but it won't be in time for a
26 October beta2.

> * Surely this bit:
>
>> xpfree(lastrowid);
>> ! if (rowid)
>> ! lastrowid = pstrdup(rowid);
>> }
>
> needs to be:
>
> if (rowid)
> lastrowid = pstrdup(rowid);
> else
> lastrowid = NULL;
>
> no? (Again the variant pstrdup would save some notation)

Well I had already defined xpfree like this:
8<------------------
#define xpfree(var_) \
do { \
if (var_ != NULL) \
{ \
pfree(var_); \
var_ = NULL; \
} \
} while (0)
8<------------------
so lastrowid is already NULL (I sometimes wish this was the default
behavior for pfree() itself). But the point about pstrdup variant is
well taken, and I guess the xpfree behavior is not obvious, so it
deserves at least a comment.

> regards, tom lane
>
> PS: I hear things are pretty crazy out your way -- hope the fire's
> not too close to you.

We packed and were ready to evacuate two or three times, but never
actually had to leave our house, thankfully. The closest the fire ever
got was about 4 miles, and at this point I don't think we're in any more
direct danger. But I know many people who were not so fortunate :-(

Joe


From: Joe Conway <mail(at)joeconway(dot)com>
To: Joe Conway <mail(at)joeconway(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Jorge Godoy <jgodoy(at)gmail(dot)com>, scott(dot)marlowe(at)gmail(dot)com, Stefan Schwarzer <stefan(dot)schwarzer(at)grid(dot)unep(dot)ch>, regmeplease(at)gmail(dot)com, "Patches (PostgreSQL)" <pgsql-patches(at)postgresql(dot)org>
Subject: Re: [GENERAL] Crosstab Problems
Date: 2007-10-26 05:45:20
Message-ID: 47217EF0.1040604@joeconway.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-general pgsql-patches

Joe Conway wrote:
> Tom Lane wrote:
>> A couple of minor thoughts:
>>
>> * You could reduce the ugliness of many of the tests by introducing a
>> variant strcmp function that does the "right" things with NULL inputs.
>> It might also be worth adding a variant pstrdup that takes a NULL.
>
> I had thoughts along those lines -- it would certainly make the code
> more readable. I'll go ahead and do that but it won't be in time for a
> 26 October beta2.

I'm not quite ready to commit this, mostly because I'd like to give the
rest of tablefunc.c the once-over for similar issues related to not
checking for NULL return values from SPI_getvalue(). But it is close
enough if needed for a beta2 tomorrow -- let me know if we plan to
bundle up beta2 and I'll get it in.

Thanks,

Joe

Attachment Content-Type Size
tablefunc.head.diff text/x-patch 24.2 KB

From: Bruce Momjian <bruce(at)momjian(dot)us>
To: Joe Conway <mail(at)joeconway(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Jorge Godoy <jgodoy(at)gmail(dot)com>, scott(dot)marlowe(at)gmail(dot)com, Stefan Schwarzer <stefan(dot)schwarzer(at)grid(dot)unep(dot)ch>, regmeplease(at)gmail(dot)com, "Patches (PostgreSQL)" <pgsql-patches(at)postgresql(dot)org>
Subject: Re: [GENERAL] Crosstab Problems
Date: 2007-11-09 21:20:17
Message-ID: 200711092120.lA9LKHD28819@momjian.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-general pgsql-patches


Joe, are you nearly ready to apply this?

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

Joe Conway wrote:
> Joe Conway wrote:
> > Tom Lane wrote:
> >> A couple of minor thoughts:
> >>
> >> * You could reduce the ugliness of many of the tests by introducing a
> >> variant strcmp function that does the "right" things with NULL inputs.
> >> It might also be worth adding a variant pstrdup that takes a NULL.
> >
> > I had thoughts along those lines -- it would certainly make the code
> > more readable. I'll go ahead and do that but it won't be in time for a
> > 26 October beta2.
>
> I'm not quite ready to commit this, mostly because I'd like to give the
> rest of tablefunc.c the once-over for similar issues related to not
> checking for NULL return values from SPI_getvalue(). But it is close
> enough if needed for a beta2 tomorrow -- let me know if we plan to
> bundle up beta2 and I'll get it in.
>
> Thanks,
>
> Joe

[ text/x-patch is unsupported, treating like TEXT/PLAIN ]

> Index: tablefunc.c
> ===================================================================
> RCS file: /opt/src/cvs/pgsql/contrib/tablefunc/tablefunc.c,v
> retrieving revision 1.47
> diff -c -r1.47 tablefunc.c
> *** tablefunc.c 3 Mar 2007 19:32:54 -0000 1.47
> --- tablefunc.c 26 Oct 2007 05:35:23 -0000
> ***************
> *** 106,111 ****
> --- 106,123 ----
> } \
> } while (0)
>
> + #define xpstrdup(tgtvar_, srcvar_) \
> + do { \
> + if (srcvar_) \
> + tgtvar_ = pstrdup(srcvar_); \
> + else \
> + tgtvar_ = NULL; \
> + } while (0)
> +
> + #define xstreq(tgtvar_, srcvar_) \
> + (((tgtvar_ == NULL) && (srcvar_ == NULL)) || \
> + ((tgtvar_ != NULL) && (srcvar_ != NULL) && (strcmp(tgtvar_, srcvar_) == 0)))
> +
> /* sign, 10 digits, '\0' */
> #define INT32_STRLEN 12
>
> ***************
> *** 355,360 ****
> --- 367,373 ----
> crosstab_fctx *fctx;
> int i;
> int num_categories;
> + bool firstpass = false;
> MemoryContext oldcontext;
>
> /* stuff done only on the first call of the function */
> ***************
> *** 469,474 ****
> --- 482,488 ----
> funcctx->max_calls = proc;
>
> MemoryContextSwitchTo(oldcontext);
> + firstpass = true;
> }
>
> /* stuff done on every call of the function */
> ***************
> *** 500,506 ****
> HeapTuple tuple;
> Datum result;
> char **values;
> ! bool allnulls = true;
>
> while (true)
> {
> --- 514,520 ----
> HeapTuple tuple;
> Datum result;
> char **values;
> ! bool skip_tuple = false;
>
> while (true)
> {
> ***************
> *** 530,555 ****
> rowid = SPI_getvalue(spi_tuple, spi_tupdesc, 1);
>
> /*
> ! * If this is the first pass through the values for this rowid
> ! * set it, otherwise make sure it hasn't changed on us. Also
> ! * check to see if the rowid is the same as that of the last
> ! * tuple sent -- if so, skip this tuple entirely
> */
> if (i == 0)
> - values[0] = pstrdup(rowid);
> -
> - if ((rowid != NULL) && (strcmp(rowid, values[0]) == 0))
> {
> ! if ((lastrowid != NULL) && (strcmp(rowid, lastrowid) == 0))
> break;
> ! else if (allnulls == true)
> ! allnulls = false;
>
> /*
> ! * Get the next category item value, which is alway
> * attribute number three.
> *
> ! * Be careful to sssign the value to the array index based
> * on which category we are presently processing.
> */
> values[1 + i] = SPI_getvalue(spi_tuple, spi_tupdesc, 3);
> --- 544,578 ----
> rowid = SPI_getvalue(spi_tuple, spi_tupdesc, 1);
>
> /*
> ! * If this is the first pass through the values for this
> ! * rowid, set the first column to rowid
> */
> if (i == 0)
> {
> ! xpstrdup(values[0], rowid);
> !
> ! /*
> ! * Check to see if the rowid is the same as that of the last
> ! * tuple sent -- if so, skip this tuple entirely
> ! */
> ! if (!firstpass && xstreq(lastrowid, rowid))
> ! {
> ! skip_tuple = true;
> break;
> ! }
> ! }
>
> + /*
> + * If rowid hasn't changed on us, continue building the
> + * ouput tuple.
> + */
> + if (xstreq(rowid, values[0]))
> + {
> /*
> ! * Get the next category item value, which is always
> * attribute number three.
> *
> ! * Be careful to assign the value to the array index based
> * on which category we are presently processing.
> */
> values[1 + i] = SPI_getvalue(spi_tuple, spi_tupdesc, 3);
> ***************
> *** 572,597 ****
> call_cntr = --funcctx->call_cntr;
> break;
> }
> !
> ! if (rowid != NULL)
> ! xpfree(rowid);
> }
>
> ! xpfree(fctx->lastrowid);
>
> ! if (values[0] != NULL)
> ! {
> ! /*
> ! * switch to memory context appropriate for multiple function
> ! * calls
> ! */
> ! oldcontext = MemoryContextSwitchTo(funcctx->multi_call_memory_ctx);
>
> ! lastrowid = fctx->lastrowid = pstrdup(values[0]);
> ! MemoryContextSwitchTo(oldcontext);
> ! }
>
> ! if (!allnulls)
> {
> /* build the tuple */
> tuple = BuildTupleFromCStrings(attinmeta, values);
> --- 595,616 ----
> call_cntr = --funcctx->call_cntr;
> break;
> }
> ! xpfree(rowid);
> }
>
> ! /*
> ! * switch to memory context appropriate for multiple function
> ! * calls
> ! */
> ! oldcontext = MemoryContextSwitchTo(funcctx->multi_call_memory_ctx);
>
> ! xpfree(fctx->lastrowid);
> ! xpstrdup(fctx->lastrowid, values[0]);
> ! lastrowid = fctx->lastrowid;
>
> ! MemoryContextSwitchTo(oldcontext);
>
> ! if (!skip_tuple)
> {
> /* build the tuple */
> tuple = BuildTupleFromCStrings(attinmeta, values);
> ***************
> *** 625,630 ****
> --- 644,652 ----
> SPI_finish();
> SRF_RETURN_DONE(funcctx);
> }
> +
> + /* need to reset this before the next tuple is started */
> + skip_tuple = false;
> }
> }
> }
> ***************
> *** 856,861 ****
> --- 878,884 ----
> int ncols = spi_tupdesc->natts;
> char *rowid;
> char *lastrowid = NULL;
> + bool firstpass = true;
> int i,
> j;
> int result_ncols;
> ***************
> *** 918,938 ****
> /* get the rowid from the current sql result tuple */
> rowid = SPI_getvalue(spi_tuple, spi_tupdesc, 1);
>
> - /* if rowid is null, skip this tuple entirely */
> - if (rowid == NULL)
> - continue;
> -
> /*
> * if we're on a new output row, grab the column values up to
> * column N-2 now
> */
> ! if ((lastrowid == NULL) || (strcmp(rowid, lastrowid) != 0))
> {
> /*
> * a new row means we need to flush the old one first, unless
> * we're on the very first row
> */
> ! if (lastrowid != NULL)
> {
> /* rowid changed, flush the previous output row */
> tuple = BuildTupleFromCStrings(attinmeta, values);
> --- 941,957 ----
> /* get the rowid from the current sql result tuple */
> rowid = SPI_getvalue(spi_tuple, spi_tupdesc, 1);
>
> /*
> * if we're on a new output row, grab the column values up to
> * column N-2 now
> */
> ! if (firstpass || !xstreq(lastrowid, rowid))
> {
> /*
> * a new row means we need to flush the old one first, unless
> * we're on the very first row
> */
> ! if (!firstpass)
> {
> /* rowid changed, flush the previous output row */
> tuple = BuildTupleFromCStrings(attinmeta, values);
> ***************
> *** 949,954 ****
> --- 968,976 ----
> values[0] = rowid;
> for (j = 1; j < ncols - 2; j++)
> values[j] = SPI_getvalue(spi_tuple, spi_tupdesc, j + 1);
> +
> + /* we're no longer on the first pass */
> + firstpass = false;
> }
>
> /* look up the category and fill in the appropriate column */
> ***************
> *** 964,970 ****
> }
>
> xpfree(lastrowid);
> ! lastrowid = pstrdup(rowid);
> }
>
> /* flush the last output row */
> --- 986,992 ----
> }
>
> xpfree(lastrowid);
> ! xpstrdup(lastrowid, rowid);
> }
>
> /* flush the last output row */
> Index: data/ct.data
> ===================================================================
> RCS file: /opt/src/cvs/pgsql/contrib/tablefunc/data/ct.data,v
> retrieving revision 1.1
> diff -c -r1.1 ct.data
> *** data/ct.data 12 Sep 2002 00:14:40 -0000 1.1
> --- data/ct.data 25 Oct 2007 21:45:49 -0000
> ***************
> *** 12,14 ****
> --- 12,18 ----
> 12 group2 test4 att1 val4
> 13 group2 test4 att2 val5
> 14 group2 test4 att3 val6
> + 15 group1 \N att1 val9
> + 16 group1 \N att2 val10
> + 17 group1 \N att3 val11
> + 18 group1 \N att4 val12
> Index: expected/tablefunc.out
> ===================================================================
> RCS file: /opt/src/cvs/pgsql/contrib/tablefunc/expected/tablefunc.out,v
> retrieving revision 1.13
> diff -c -r1.13 tablefunc.out
> *** expected/tablefunc.out 27 Feb 2006 16:09:49 -0000 1.13
> --- expected/tablefunc.out 25 Oct 2007 22:24:01 -0000
> ***************
> *** 23,64 ****
> ----------+------------+------------
> test1 | val2 | val3
> test2 | val6 | val7
> ! (2 rows)
>
> SELECT * FROM crosstab3('SELECT rowid, attribute, val FROM ct where rowclass = ''group1'' and (attribute = ''att2'' or attribute = ''att3'') ORDER BY 1,2;');
> row_name | category_1 | category_2 | category_3
> ----------+------------+------------+------------
> test1 | val2 | val3 |
> test2 | val6 | val7 |
> ! (2 rows)
>
> SELECT * FROM crosstab4('SELECT rowid, attribute, val FROM ct where rowclass = ''group1'' and (attribute = ''att2'' or attribute = ''att3'') ORDER BY 1,2;');
> row_name | category_1 | category_2 | category_3 | category_4
> ----------+------------+------------+------------+------------
> test1 | val2 | val3 | |
> test2 | val6 | val7 | |
> ! (2 rows)
>
> SELECT * FROM crosstab2('SELECT rowid, attribute, val FROM ct where rowclass = ''group1'' ORDER BY 1,2;');
> row_name | category_1 | category_2
> ----------+------------+------------
> test1 | val1 | val2
> test2 | val5 | val6
> ! (2 rows)
>
> SELECT * FROM crosstab3('SELECT rowid, attribute, val FROM ct where rowclass = ''group1'' ORDER BY 1,2;');
> row_name | category_1 | category_2 | category_3
> ----------+------------+------------+------------
> test1 | val1 | val2 | val3
> test2 | val5 | val6 | val7
> ! (2 rows)
>
> SELECT * FROM crosstab4('SELECT rowid, attribute, val FROM ct where rowclass = ''group1'' ORDER BY 1,2;');
> row_name | category_1 | category_2 | category_3 | category_4
> ----------+------------+------------+------------+------------
> test1 | val1 | val2 | val3 | val4
> test2 | val5 | val6 | val7 | val8
> ! (2 rows)
>
> SELECT * FROM crosstab2('SELECT rowid, attribute, val FROM ct where rowclass = ''group2'' and (attribute = ''att1'' or attribute = ''att2'') ORDER BY 1,2;');
> row_name | category_1 | category_2
> --- 23,70 ----
> ----------+------------+------------
> test1 | val2 | val3
> test2 | val6 | val7
> ! | val10 | val11
> ! (3 rows)
>
> SELECT * FROM crosstab3('SELECT rowid, attribute, val FROM ct where rowclass = ''group1'' and (attribute = ''att2'' or attribute = ''att3'') ORDER BY 1,2;');
> row_name | category_1 | category_2 | category_3
> ----------+------------+------------+------------
> test1 | val2 | val3 |
> test2 | val6 | val7 |
> ! | val10 | val11 |
> ! (3 rows)
>
> SELECT * FROM crosstab4('SELECT rowid, attribute, val FROM ct where rowclass = ''group1'' and (attribute = ''att2'' or attribute = ''att3'') ORDER BY 1,2;');
> row_name | category_1 | category_2 | category_3 | category_4
> ----------+------------+------------+------------+------------
> test1 | val2 | val3 | |
> test2 | val6 | val7 | |
> ! | val10 | val11 | |
> ! (3 rows)
>
> SELECT * FROM crosstab2('SELECT rowid, attribute, val FROM ct where rowclass = ''group1'' ORDER BY 1,2;');
> row_name | category_1 | category_2
> ----------+------------+------------
> test1 | val1 | val2
> test2 | val5 | val6
> ! | val9 | val10
> ! (3 rows)
>
> SELECT * FROM crosstab3('SELECT rowid, attribute, val FROM ct where rowclass = ''group1'' ORDER BY 1,2;');
> row_name | category_1 | category_2 | category_3
> ----------+------------+------------+------------
> test1 | val1 | val2 | val3
> test2 | val5 | val6 | val7
> ! | val9 | val10 | val11
> ! (3 rows)
>
> SELECT * FROM crosstab4('SELECT rowid, attribute, val FROM ct where rowclass = ''group1'' ORDER BY 1,2;');
> row_name | category_1 | category_2 | category_3 | category_4
> ----------+------------+------------+------------+------------
> test1 | val1 | val2 | val3 | val4
> test2 | val5 | val6 | val7 | val8
> ! | val9 | val10 | val11 | val12
> ! (3 rows)
>
> SELECT * FROM crosstab2('SELECT rowid, attribute, val FROM ct where rowclass = ''group2'' and (attribute = ''att1'' or attribute = ''att2'') ORDER BY 1,2;');
> row_name | category_1 | category_2
> ***************
> *** 103,127 ****
> (2 rows)
>
> SELECT * FROM crosstab('SELECT rowid, attribute, val FROM ct where rowclass = ''group1'' ORDER BY 1,2;') AS c(rowid text, att1 text, att2 text);
> ! rowid | att1 | att2
> ! -------+------+------
> test1 | val1 | val2
> test2 | val5 | val6
> ! (2 rows)
>
> SELECT * FROM crosstab('SELECT rowid, attribute, val FROM ct where rowclass = ''group1'' ORDER BY 1,2;') AS c(rowid text, att1 text, att2 text, att3 text);
> ! rowid | att1 | att2 | att3
> ! -------+------+------+------
> ! test1 | val1 | val2 | val3
> ! test2 | val5 | val6 | val7
> ! (2 rows)
>
> SELECT * FROM crosstab('SELECT rowid, attribute, val FROM ct where rowclass = ''group1'' ORDER BY 1,2;') AS c(rowid text, att1 text, att2 text, att3 text, att4 text);
> ! rowid | att1 | att2 | att3 | att4
> ! -------+------+------+------+------
> ! test1 | val1 | val2 | val3 | val4
> ! test2 | val5 | val6 | val7 | val8
> ! (2 rows)
>
> -- check it works with OUT parameters, too
> CREATE FUNCTION crosstab_out(text,
> --- 109,136 ----
> (2 rows)
>
> SELECT * FROM crosstab('SELECT rowid, attribute, val FROM ct where rowclass = ''group1'' ORDER BY 1,2;') AS c(rowid text, att1 text, att2 text);
> ! rowid | att1 | att2
> ! -------+------+-------
> test1 | val1 | val2
> test2 | val5 | val6
> ! | val9 | val10
> ! (3 rows)
>
> SELECT * FROM crosstab('SELECT rowid, attribute, val FROM ct where rowclass = ''group1'' ORDER BY 1,2;') AS c(rowid text, att1 text, att2 text, att3 text);
> ! rowid | att1 | att2 | att3
> ! -------+------+-------+-------
> ! test1 | val1 | val2 | val3
> ! test2 | val5 | val6 | val7
> ! | val9 | val10 | val11
> ! (3 rows)
>
> SELECT * FROM crosstab('SELECT rowid, attribute, val FROM ct where rowclass = ''group1'' ORDER BY 1,2;') AS c(rowid text, att1 text, att2 text, att3 text, att4 text);
> ! rowid | att1 | att2 | att3 | att4
> ! -------+------+-------+-------+-------
> ! test1 | val1 | val2 | val3 | val4
> ! test2 | val5 | val6 | val7 | val8
> ! | val9 | val10 | val11 | val12
> ! (3 rows)
>
> -- check it works with OUT parameters, too
> CREATE FUNCTION crosstab_out(text,
> ***************
> *** 130,140 ****
> AS '$libdir/tablefunc','crosstab'
> LANGUAGE C STABLE STRICT;
> SELECT * FROM crosstab_out('SELECT rowid, attribute, val FROM ct where rowclass = ''group1'' ORDER BY 1,2;');
> ! rowid | att1 | att2 | att3
> ! -------+------+------+------
> ! test1 | val1 | val2 | val3
> ! test2 | val5 | val6 | val7
> ! (2 rows)
>
> --
> -- hash based crosstab
> --- 139,150 ----
> AS '$libdir/tablefunc','crosstab'
> LANGUAGE C STABLE STRICT;
> SELECT * FROM crosstab_out('SELECT rowid, attribute, val FROM ct where rowclass = ''group1'' ORDER BY 1,2;');
> ! rowid | att1 | att2 | att3
> ! -------+------+-------+-------
> ! test1 | val1 | val2 | val3
> ! test2 | val5 | val6 | val7
> ! | val9 | val10 | val11
> ! (3 rows)
>
> --
> -- hash based crosstab
> ***************
> *** 150,187 ****
> insert into cth values(DEFAULT,'test2','02 March 2003','test_result','FAIL');
> insert into cth values(DEFAULT,'test2','02 March 2003','test_startdate','01 March 2003');
> insert into cth values(DEFAULT,'test2','02 March 2003','volts','3.1234');
> -- return attributes as plain text
> SELECT * FROM crosstab(
> 'SELECT rowid, rowdt, attribute, val FROM cth ORDER BY 1',
> 'SELECT DISTINCT attribute FROM cth ORDER BY 1')
> AS c(rowid text, rowdt timestamp, temperature text, test_result text, test_startdate text, volts text);
> ! rowid | rowdt | temperature | test_result | test_startdate | volts
> ! -------+--------------------------+-------------+-------------+----------------+--------
> ! test1 | Sat Mar 01 00:00:00 2003 | 42 | PASS | | 2.6987
> ! test2 | Sun Mar 02 00:00:00 2003 | 53 | FAIL | 01 March 2003 | 3.1234
> ! (2 rows)
>
> -- this time without rowdt
> SELECT * FROM crosstab(
> 'SELECT rowid, attribute, val FROM cth ORDER BY 1',
> 'SELECT DISTINCT attribute FROM cth ORDER BY 1')
> AS c(rowid text, temperature text, test_result text, test_startdate text, volts text);
> ! rowid | temperature | test_result | test_startdate | volts
> ! -------+-------------+-------------+----------------+--------
> ! test1 | 42 | PASS | | 2.6987
> ! test2 | 53 | FAIL | 01 March 2003 | 3.1234
> ! (2 rows)
>
> -- convert attributes to specific datatypes
> SELECT * FROM crosstab(
> 'SELECT rowid, rowdt, attribute, val FROM cth ORDER BY 1',
> 'SELECT DISTINCT attribute FROM cth ORDER BY 1')
> AS c(rowid text, rowdt timestamp, temperature int4, test_result text, test_startdate timestamp, volts float8);
> ! rowid | rowdt | temperature | test_result | test_startdate | volts
> ! -------+--------------------------+-------------+-------------+--------------------------+--------
> ! test1 | Sat Mar 01 00:00:00 2003 | 42 | PASS | | 2.6987
> ! test2 | Sun Mar 02 00:00:00 2003 | 53 | FAIL | Sat Mar 01 00:00:00 2003 | 3.1234
> ! (2 rows)
>
> -- source query and category query out of sync
> SELECT * FROM crosstab(
> --- 160,205 ----
> insert into cth values(DEFAULT,'test2','02 March 2003','test_result','FAIL');
> insert into cth values(DEFAULT,'test2','02 March 2003','test_startdate','01 March 2003');
> insert into cth values(DEFAULT,'test2','02 March 2003','volts','3.1234');
> + -- next group tests for NULL rowids
> + insert into cth values(DEFAULT,NULL,'25 October 2007','temperature','57');
> + insert into cth values(DEFAULT,NULL,'25 October 2007','test_result','PASS');
> + insert into cth values(DEFAULT,NULL,'25 October 2007','test_startdate','24 October 2007');
> + insert into cth values(DEFAULT,NULL,'25 October 2007','volts','1.41234');
> -- return attributes as plain text
> SELECT * FROM crosstab(
> 'SELECT rowid, rowdt, attribute, val FROM cth ORDER BY 1',
> 'SELECT DISTINCT attribute FROM cth ORDER BY 1')
> AS c(rowid text, rowdt timestamp, temperature text, test_result text, test_startdate text, volts text);
> ! rowid | rowdt | temperature | test_result | test_startdate | volts
> ! -------+--------------------------+-------------+-------------+-----------------+---------
> ! test1 | Sat Mar 01 00:00:00 2003 | 42 | PASS | | 2.6987
> ! test2 | Sun Mar 02 00:00:00 2003 | 53 | FAIL | 01 March 2003 | 3.1234
> ! | Thu Oct 25 00:00:00 2007 | 57 | PASS | 24 October 2007 | 1.41234
> ! (3 rows)
>
> -- this time without rowdt
> SELECT * FROM crosstab(
> 'SELECT rowid, attribute, val FROM cth ORDER BY 1',
> 'SELECT DISTINCT attribute FROM cth ORDER BY 1')
> AS c(rowid text, temperature text, test_result text, test_startdate text, volts text);
> ! rowid | temperature | test_result | test_startdate | volts
> ! -------+-------------+-------------+-----------------+---------
> ! test1 | 42 | PASS | | 2.6987
> ! test2 | 53 | FAIL | 01 March 2003 | 3.1234
> ! | 57 | PASS | 24 October 2007 | 1.41234
> ! (3 rows)
>
> -- convert attributes to specific datatypes
> SELECT * FROM crosstab(
> 'SELECT rowid, rowdt, attribute, val FROM cth ORDER BY 1',
> 'SELECT DISTINCT attribute FROM cth ORDER BY 1')
> AS c(rowid text, rowdt timestamp, temperature int4, test_result text, test_startdate timestamp, volts float8);
> ! rowid | rowdt | temperature | test_result | test_startdate | volts
> ! -------+--------------------------+-------------+-------------+--------------------------+---------
> ! test1 | Sat Mar 01 00:00:00 2003 | 42 | PASS | | 2.6987
> ! test2 | Sun Mar 02 00:00:00 2003 | 53 | FAIL | Sat Mar 01 00:00:00 2003 | 3.1234
> ! | Thu Oct 25 00:00:00 2007 | 57 | PASS | Wed Oct 24 00:00:00 2007 | 1.41234
> ! (3 rows)
>
> -- source query and category query out of sync
> SELECT * FROM crosstab(
> ***************
> *** 192,198 ****
> -------+--------------------------+-------------+-------------+--------------------------
> test1 | Sat Mar 01 00:00:00 2003 | 42 | PASS |
> test2 | Sun Mar 02 00:00:00 2003 | 53 | FAIL | Sat Mar 01 00:00:00 2003
> ! (2 rows)
>
> -- if category query generates no rows, get expected error
> SELECT * FROM crosstab(
> --- 210,217 ----
> -------+--------------------------+-------------+-------------+--------------------------
> test1 | Sat Mar 01 00:00:00 2003 | 42 | PASS |
> test2 | Sun Mar 02 00:00:00 2003 | 53 | FAIL | Sat Mar 01 00:00:00 2003
> ! | Thu Oct 25 00:00:00 2007 | 57 | PASS | Wed Oct 24 00:00:00 2007
> ! (3 rows)
>
> -- if category query generates no rows, get expected error
> SELECT * FROM crosstab(
> ***************
> *** 235,245 ****
> SELECT * FROM crosstab_named(
> 'SELECT rowid, rowdt, attribute, val FROM cth ORDER BY 1',
> 'SELECT DISTINCT attribute FROM cth ORDER BY 1');
> ! rowid | rowdt | temperature | test_result | test_startdate | volts
> ! -------+--------------------------+-------------+-------------+--------------------------+--------
> ! test1 | Sat Mar 01 00:00:00 2003 | 42 | PASS | | 2.6987
> ! test2 | Sun Mar 02 00:00:00 2003 | 53 | FAIL | Sat Mar 01 00:00:00 2003 | 3.1234
> ! (2 rows)
>
> -- check it works with OUT parameters
> CREATE FUNCTION crosstab_out(text, text,
> --- 254,265 ----
> SELECT * FROM crosstab_named(
> 'SELECT rowid, rowdt, attribute, val FROM cth ORDER BY 1',
> 'SELECT DISTINCT attribute FROM cth ORDER BY 1');
> ! rowid | rowdt | temperature | test_result | test_startdate | volts
> ! -------+--------------------------+-------------+-------------+--------------------------+---------
> ! test1 | Sat Mar 01 00:00:00 2003 | 42 | PASS | | 2.6987
> ! test2 | Sun Mar 02 00:00:00 2003 | 53 | FAIL | Sat Mar 01 00:00:00 2003 | 3.1234
> ! | Thu Oct 25 00:00:00 2007 | 57 | PASS | Wed Oct 24 00:00:00 2007 | 1.41234
> ! (3 rows)
>
> -- check it works with OUT parameters
> CREATE FUNCTION crosstab_out(text, text,
> ***************
> *** 252,262 ****
> SELECT * FROM crosstab_out(
> 'SELECT rowid, rowdt, attribute, val FROM cth ORDER BY 1',
> 'SELECT DISTINCT attribute FROM cth ORDER BY 1');
> ! rowid | rowdt | temperature | test_result | test_startdate | volts
> ! -------+--------------------------+-------------+-------------+--------------------------+--------
> ! test1 | Sat Mar 01 00:00:00 2003 | 42 | PASS | | 2.6987
> ! test2 | Sun Mar 02 00:00:00 2003 | 53 | FAIL | Sat Mar 01 00:00:00 2003 | 3.1234
> ! (2 rows)
>
> --
> -- connectby
> --- 272,283 ----
> SELECT * FROM crosstab_out(
> 'SELECT rowid, rowdt, attribute, val FROM cth ORDER BY 1',
> 'SELECT DISTINCT attribute FROM cth ORDER BY 1');
> ! rowid | rowdt | temperature | test_result | test_startdate | volts
> ! -------+--------------------------+-------------+-------------+--------------------------+---------
> ! test1 | Sat Mar 01 00:00:00 2003 | 42 | PASS | | 2.6987
> ! test2 | Sun Mar 02 00:00:00 2003 | 53 | FAIL | Sat Mar 01 00:00:00 2003 | 3.1234
> ! | Thu Oct 25 00:00:00 2007 | 57 | PASS | Wed Oct 24 00:00:00 2007 | 1.41234
> ! (3 rows)
>
> --
> -- connectby
> Index: sql/tablefunc.sql
> ===================================================================
> RCS file: /opt/src/cvs/pgsql/contrib/tablefunc/sql/tablefunc.sql,v
> retrieving revision 1.12
> diff -c -r1.12 tablefunc.sql
> *** sql/tablefunc.sql 27 Feb 2006 16:09:49 -0000 1.12
> --- sql/tablefunc.sql 25 Oct 2007 22:20:09 -0000
> ***************
> *** 61,66 ****
> --- 61,71 ----
> insert into cth values(DEFAULT,'test2','02 March 2003','test_result','FAIL');
> insert into cth values(DEFAULT,'test2','02 March 2003','test_startdate','01 March 2003');
> insert into cth values(DEFAULT,'test2','02 March 2003','volts','3.1234');
> + -- next group tests for NULL rowids
> + insert into cth values(DEFAULT,NULL,'25 October 2007','temperature','57');
> + insert into cth values(DEFAULT,NULL,'25 October 2007','test_result','PASS');
> + insert into cth values(DEFAULT,NULL,'25 October 2007','test_startdate','24 October 2007');
> + insert into cth values(DEFAULT,NULL,'25 October 2007','volts','1.41234');
>
> -- return attributes as plain text
> SELECT * FROM crosstab(

>
> ---------------------------(end of broadcast)---------------------------
> TIP 4: Have you searched our list archives?
>
> http://archives.postgresql.org

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://postgres.enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +


From: Joe Conway <mail(at)joeconway(dot)com>
To: Bruce Momjian <bruce(at)momjian(dot)us>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Jorge Godoy <jgodoy(at)gmail(dot)com>, scott(dot)marlowe(at)gmail(dot)com, Stefan Schwarzer <stefan(dot)schwarzer(at)grid(dot)unep(dot)ch>, regmeplease(at)gmail(dot)com, "Patches (PostgreSQL)" <pgsql-patches(at)postgresql(dot)org>
Subject: Re: [GENERAL] Crosstab Problems
Date: 2007-11-09 23:33:15
Message-ID: 4734EE3B.7070203@joeconway.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-general pgsql-patches

Bruce Momjian wrote:
> Joe, are you nearly ready to apply this?
>

Yeah, sorry for the delay. By the end of the weekend.

Joe


From: Joe Conway <mail(at)joeconway(dot)com>
To: Bruce Momjian <bruce(at)momjian(dot)us>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Jorge Godoy <jgodoy(at)gmail(dot)com>, scott(dot)marlowe(at)gmail(dot)com, Stefan Schwarzer <stefan(dot)schwarzer(at)grid(dot)unep(dot)ch>, regmeplease(at)gmail(dot)com, "Patches (PostgreSQL)" <pgsql-patches(at)postgresql(dot)org>
Subject: Re: [GENERAL] Crosstab Problems
Date: 2007-11-10 05:04:51
Message-ID: 47353BF3.6020107@joeconway.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-general pgsql-patches

Bruce Momjian wrote:
> Joe, are you nearly ready to apply this?
>

Done (head and backwards to 7.3).

Joe