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

Re: uuid patch 3.0 (8.3devel)

From: Neil Conway <neilc(at)samurai(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Gevik Babakhani <pgdev(at)xs4all(dot)nl>, pgsql-patches <pgsql-patches(at)postgresql(dot)org>
Subject: Re: uuid patch 3.0 (8.3devel)
Date: 2007-01-28 16:20:09
Message-ID: 1170001209.29138.33.camel@localhost.localdomain (view raw or flat)
Thread:
Lists: pgsql-patches
On Sat, 2007-01-27 at 23:53 -0500, Tom Lane wrote:
> A few comments after a quick once-over, perhaps you caught all this
> already...

Much of it :) But thanks for the review.

> uuid.c header is missing $PostgreSQL$ line, so is uuid.h,
> copyright notice in the latter seems wrong too.

I think the copyright header on completely new code should just include
the current year, shouldn't it? I've just marked both files as
"Copyright (c) 2007 [PGDG]".

> Generally I like to put "local var = PG_GETARG()" declarations
> first in a function, not randomly mixed in with other declarations
> of local variables.  Think of them as part of the function header.
> (Someday we might try to process them with some automatic script,
> too... so the less random stylistic variation, the better.)

Agreed on both points; of course, PG_GETARG_XXX(n) should also be
ordered in ascending order of "n".

> uuid.c contains some functions that are declared static and
> then defined without, please clean this up, and make sure
> it's not exporting anything it doesn't have to.

Notably, the uuid_t struct doesn't need to be exported.

> The regression test is probably expending a lot more cycles than are
> justified, eg what exactly is the point of the last part that inserts
> 32K random-data records?  If it ever did show a failure we'd be unable
> to reproduce it, so please at least lose the use of now() and random().

I just removed the test, as it seemed unlikely to be helpful.

Patch applied with various stylistic changes (including all of Tom's
suggestions), catversion bumped.

-Neil



In response to

Responses

pgsql-patches by date

Next:From: Tom LaneDate: 2007-01-28 16:40:06
Subject: Re: uuid patch 3.0 (8.3devel)
Previous:From: Gevik BabakhaniDate: 2007-01-28 15:37:37
Subject: Re: uuid patch 3.0 (8.3devel)

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