Fwd: Re: [pgsql-patches] pg_get_domaindef

Resubmitting this patch without the legal disclaimers attached to it.

Rgds,
Arul Shaji

---------- FWD: ----------

Subject: Re: [pgsql-patches] pg_get_domaindef
Date: Thu, 25 Jan 2007 03:18:30 +1100
From: Arul Shaji <fastpgs(at)fast(dot)fujitsu(dot)com(dot)au>
To: Neil Conway <neilc(at)samurai(dot)com>
Cc: pgsql-patches(at)postgresql(dot)org

Please find attached the patch with modifications

Rgds,
Arul Shaji

On Sat, 20 Jan 2007 04:44, FAST PostgreSQL wrote:
> On Fri, 19 Jan 2007 17:02, Neil Conway wrote:
> > On Sat, 2007-01-20 at 02:28 +1100, FAST PostgreSQL wrote:
> > > Attached is a small patch that implements the pg_get_domaindef(oid)
> > > function.
> >
> > A few minor comments:
> >
> > - don't use C++-style comments
>
> OK. Can do.
>
> > - why does this code append a "-" to the output when SPI_processed != 1,
> > rather than erroring out?
>
> get_ruledef() does the same. As the user gets a '-' in that case when a
> non-existent oid is given, I just wanted to be consistent. Maybe a wrong
> idea ?
>
> > - you probably want to elog(ERROR) if typeTuple is invalid:
>
> Of course.
>
> > + if (typnotnull || constraint != NULL)
> > + {
> > + if ( ( (contype != NULL) && (strcmp(contype,
> > "c") != 0) ) || typnotnull )
> > + {
> > + appendStringInfo(&buf, "CONSTRAINT ");
> > + }
> > + if (typnotnull)
> > + {
> > + appendStringInfo(&buf, "NOT NULL ");
> > + }
> > + }
> > + if (constraint != NULL)
> > + {
> > + appendStringInfo(&buf,
> > quote_identifier(constraint));
> > + }
> >
> > This logic seems pretty awkward. Perhaps simpler would be a check for
> > typnotnull (and then appending "CONSTRAINT NOT NULL"), and then handling
> > the non-typnotnull branch separately.
>
> Yeah agree.
>
> > -Neil
>
> Rgds,
> Arul Shaji
>
>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 6: explain analyze is your friend

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