Skip site navigation (1) Skip section navigation (2)

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 (view raw or flat)
Thread:
Lists: pgsql-generalpgsql-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

pgsql-patches by date

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

pgsql-general by date

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

Privacy Policy | About PostgreSQL
Copyright © 1996-2014 The PostgreSQL Global Development Group