Обсуждение: 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@fast.fujitsu.com.au> To: Neil Conway <neilc@samurai.com> Cc: pgsql-patches@postgresql.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 -------------------------------------------------------
Вложения
Thanks, but after lots of discussion, it turns out the community doesn't want to add such functions, and I have removed it from the TODO list. --------------------------------------------------------------------------- FAST PostgreSQL wrote: > 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@fast.fujitsu.com.au> > To: Neil Conway <neilc@samurai.com> > Cc: pgsql-patches@postgresql.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 > > ------------------------------------------------------- > > [ Attachment, skipping... ] > > ---------------------------(end of broadcast)--------------------------- > TIP 7: You can help support the PostgreSQL project by donating at > > http://www.postgresql.org/about/donate -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. +
On Fri, Mar 30, 2007 at 01:45:21PM -0400, Bruce Momjian wrote: > Thanks, but after lots of discussion, it turns out the community > doesn't want to add such functions, and I have removed it from the > TODO list. From what I recall of the discussion, the lack of interest was in actually stepping up and doing it, not in the feature itself. Cheers, D -- David Fetter <david@fetter.org> http://fetter.org/ phone: +1 415 235 3778 AIM: dfetter666 Skype: davidfetter Remember to vote! Consider donating to PostgreSQL: http://www.postgresql.org/about/donate
David Fetter <david@fetter.org> writes: > On Fri, Mar 30, 2007 at 01:45:21PM -0400, Bruce Momjian wrote: >> Thanks, but after lots of discussion, it turns out the community >> doesn't want to add such functions, and I have removed it from the >> TODO list. > From what I recall of the discussion, the lack of interest was in > actually stepping up and doing it, not in the feature itself. Um, no: there were serious concerns first about the snapshot semantics (MVCC vs SnapshotNow behavior) and second that we'd be condemning ourselves to support duplicative functionality in pg_dump and the backend for the foreseeable future. regards, tom lane