Re: [GENERAL] Crosstab Problems

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
Thread:
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.

In response to

Responses

Browse pgsql-general by date

  From Date Subject
Next Message D. Dante Lorenso 2007-10-26 02:01:53 Re: Selecting tree data
Previous Message Jorge Godoy 2007-10-26 01:42:44 Re: select count() out of memory

Browse pgsql-patches by date

  From Date Subject
Next Message Joe Conway 2007-10-26 04:40:04 Re: [GENERAL] Crosstab Problems
Previous Message Alvaro Herrera 2007-10-26 01:11:02 Re: Autovacuum cancellation