pg.ddx.io pgsql-hackers@postgresql.org mailing list archive
help / color / mirror / Atom feedpg_get_domaindef()
21+ messages / 7 participants
[nested] [flat]
* pg_get_domaindef()
@ 2006-10-26 05:33 FAST PostgreSQL <fastpgs@fast.fujitsu.com.au>
0 siblings, 1 reply; 21+ messages in thread
From: FAST PostgreSQL @ 2006-10-26 05:33 UTC (permalink / raw)
To: pgsql-hackers
Hi All,
I am now trying to implement pg_get_domaindef() function which is in the TODO
list and ran into a minor issue.
When the following command is given
CREATE DOMAIN testdomain AS text CONSTRAINT testconstraint NOT NULL;
I couldn't find the CONSTRAINT name ('testconstraint' in this case) being
stored in the system catalog. Any idea where I can find it?
Or is it acceptable, for the above statement, for pg_get_domaindef() to
return
CREATE DOMAIN testdomain AS text CONSTRAINT NOT NULL;
Rgds,
Arul Shaji
This is an email from Fujitsu Australia Software Technology Pty Ltd, ABN 27 003 693 481. It is confidential to the ordinary user of the email address to which it was addressed and may contain copyright and/or legally privileged information. No one else may read, print, store, copy or forward all or any of it or its attachments. If you receive this email in error, please return to sender. Thank you.
If you do not wish to receive commercial email messages from Fujitsu Australia Software Technology Pty Ltd, please email unsubscribe@fast.fujitsu.com.au
^ permalink raw reply [nested|flat] 21+ messages in thread
* Re: pg_get_domaindef()
@ 2006-10-26 14:27 Volkan YAZICI <yazicivo@ttnet.net.tr>
parent: FAST PostgreSQL <fastpgs@fast.fujitsu.com.au>
0 siblings, 1 reply; 21+ messages in thread
From: Volkan YAZICI @ 2006-10-26 14:27 UTC (permalink / raw)
To: FAST PostgreSQL <fastpgs@fast.fujitsu.com.au>; +Cc: pgsql-hackers
On Oct 26 03:33, FAST PostgreSQL wrote:
> I couldn't find the CONSTRAINT name ('testconstraint' in this case) being
> stored in the system catalog. Any idea where I can find it?
AFAIK, it is passed to the related procedure via a DomainIOData struct
that fcinfo->flinfo->fn_extra points to. (See domain_in() in
backend/utils/adt/domains.c)
Regards.
^ permalink raw reply [nested|flat] 21+ messages in thread
* Re: pg_get_domaindef()
@ 2006-10-26 14:52 Volkan YAZICI <yazicivo@ttnet.net.tr>
parent: Volkan YAZICI <yazicivo@ttnet.net.tr>
0 siblings, 0 replies; 21+ messages in thread
From: Volkan YAZICI @ 2006-10-26 14:52 UTC (permalink / raw)
To: FAST PostgreSQL <fastpgs@fast.fujitsu.com.au>; +Cc: pgsql-hackers
On Oct 26 05:27, Volkan YAZICI wrote:
> On Oct 26 03:33, FAST PostgreSQL wrote:
> > I couldn't find the CONSTRAINT name ('testconstraint' in this case) being
> > stored in the system catalog. Any idea where I can find it?
>
> AFAIK, it is passed to the related procedure via a DomainIOData struct
> that fcinfo->flinfo->fn_extra points to. (See domain_in() in
> backend/utils/adt/domains.c)
Ah, please excuse my wrong answer. See GetDomainConstraints() function
in the same file.
Regards.
^ permalink raw reply [nested|flat] 21+ messages in thread
* Re: pg_get_domaindef
@ 2007-01-19 06:02 Neil Conway <neilc@samurai.com>
parent: FAST PostgreSQL <fastpgs@fast.fujitsu.com.au>
3 siblings, 0 replies; 21+ messages in thread
From: Neil Conway @ 2007-01-19 06:02 UTC (permalink / raw)
To: FAST PostgreSQL <fastpgs@fast.fujitsu.com.au>; +Cc: pgsql-patches@postgresql.org
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
- why does this code append a "-" to the output when SPI_processed != 1,
rather than erroring out?
+ if (spirc != SPI_OK_SELECT)
+ elog(ERROR, "failed to get pg_type tuple for domain %u",
domainOid);
+ if (SPI_processed != 1)
+ appendStringInfo(&buf, "-");
+ else
+ {
- you probably want to elog(ERROR) if typeTuple is invalid:
+ /*
+ * Get the base type of the domain
+ */
+ typeTuple = SearchSysCache(TYPEOID,
+ ObjectIdGetDatum(basetypeOid),
+ 0, 0, 0);
+
+ if (HeapTupleIsValid(typeTuple))
+ {
+ typebasetype = pstrdup(NameStr(((Form_pg_type)
GETSTRUCT(typeTuple))->typname));
+ appendStringInfo(&buf, "%s ",
quote_identifier(typebasetype));
+ }
+ ReleaseSysCache(typeTuple);
It's also debatable whether the function ought to be using a
*combination* of the syscaches and manual system catalog queries, since
these two views may be inconsistent. I guess this is a widespread
problem, though.
+ 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.
-Neil
^ permalink raw reply [nested|flat] 21+ messages in thread
* pg_get_domaindef
@ 2007-01-19 15:28 FAST PostgreSQL <fastpgs@fast.fujitsu.com.au>
0 siblings, 4 replies; 21+ messages in thread
From: FAST PostgreSQL @ 2007-01-19 15:28 UTC (permalink / raw)
To: pgsql-patches@postgresql.org
Hi,
Attached is a small patch that implements the pg_get_domaindef(oid) function.
Somethings I am not sure about the patch and which I can fix based on your
input are as follows.
- I have used SPI interface. I saw that in ruleutils some methods make use
of SPI interface whereas others use internal APIs. I wasn't sure about the
reasons. If this is wrong, I can attempt a rewrite using internal APIs.
- This function does not output the constraint name. I couldn't find the
constraint name given by the user as part of the create domain command being
stored in any system catalogs. The entry in pg_constraints store a system
assigned name. But this may be because of my limited knowledge of the
internals.
- documentation. If this patch is good then I can do this later.
There may be other issues, which I will be happy to fix.
Rgds,
Arul Shaji
This is an email from Fujitsu Australia Software Technology Pty Ltd, ABN 27 003 693 481. It is confidential to the ordinary user of the email address to which it was addressed and may contain copyright and/or legally privileged information. No one else may read, print, store, copy or forward all or any of it or its attachments. If you receive this email in error, please return to sender. Thank you.
If you do not wish to receive commercial email messages from Fujitsu Australia Software Technology Pty Ltd, please email unsubscribe@fast.fujitsu.com.au
Attachments:
pg_get_domaindef.patch (text/x-diff, 8.2K) download
*** pgsql/src/include/catalog/pg_proc.h 2007-01-18 15:27:15.000000000 +1100
--- workingpgsql/src/include/catalog/pg_proc.h 2007-01-18 16:23:14.000000000 +1100
***************
*** 2311,2317 ****
DESCR("deparse an encoded expression");
DATA(insert OID = 1665 ( pg_get_serial_sequence PGNSP PGUID 12 f f t f s 2 25 "25 25" _null_ _null_ _null_ pg_get_serial_sequence - _null_ ));
DESCR("name of sequence for a serial column");
!
/* Generic referential integrity constraint triggers */
DATA(insert OID = 1644 ( RI_FKey_check_ins PGNSP PGUID 12 f f t f v 0 2279 "" _null_ _null_ _null_ RI_FKey_check_ins - _null_ ));
--- 2311,2318 ----
DESCR("deparse an encoded expression");
DATA(insert OID = 1665 ( pg_get_serial_sequence PGNSP PGUID 12 f f t f s 2 25 "25 25" _null_ _null_ _null_ pg_get_serial_sequence - _null_ ));
DESCR("name of sequence for a serial column");
! DATA(insert OID = 2950 ( pg_get_domaindef PGNSP PGUID 12 f f t f s 1 25 "26" _null_ _null_ _null_ pg_get_domaindef - _null_ ));
! DESCR("domain description");
/* Generic referential integrity constraint triggers */
DATA(insert OID = 1644 ( RI_FKey_check_ins PGNSP PGUID 12 f f t f v 0 2279 "" _null_ _null_ _null_ RI_FKey_check_ins - _null_ ));
*** pgsql/src/include/utils/builtins.h 2007-01-18 15:27:15.000000000 +1100
--- workingpgsql/src/include/utils/builtins.h 2007-01-18 16:05:01.000000000 +1100
***************
*** 534,539 ****
--- 534,540 ----
extern Datum pg_get_expr_ext(PG_FUNCTION_ARGS);
extern Datum pg_get_userbyid(PG_FUNCTION_ARGS);
extern Datum pg_get_serial_sequence(PG_FUNCTION_ARGS);
+ extern Datum pg_get_domaindef(PG_FUNCTION_ARGS);
extern char *deparse_expression(Node *expr, List *dpcontext,
bool forceprefix, bool showimplicit);
extern List *deparse_context_for(const char *aliasname, Oid relid);
*** pgsql/src/backend/utils/adt/ruleutils.c 2007-01-16 16:28:58.000000000 +1100
--- workingpgsql/src/backend/utils/adt/ruleutils.c 2007-01-19 14:14:51.776708712 +1100
***************
*** 105,110 ****
--- 105,115 ----
static void *plan_getviewrule = NULL;
static char *query_getviewrule = "SELECT * FROM pg_catalog.pg_rewrite WHERE ev_class = $1 AND rulename = $2";
+ // Queries to extract domain description
+ static void *plan_getdomainbyOid = NULL;
+ static char *query_getdomainbyOid = "SELECT * FROM pg_catalog.pg_type WHERE oid = $1 AND typtype = 'd'";
+ static void *plan_getconstraintbycontypid = NULL;
+ static char *query_getconstraintbycontypid = "SELECT oid, contype FROM pg_catalog.pg_constraint WHERE contypid = $1";
/* ----------
* Local functions
***************
*** 127,132 ****
--- 132,138 ----
int prettyFlags);
static char *pg_get_expr_worker(text *expr, Oid relid, char *relname,
int prettyFlags);
+ static char *pg_get_domaindef_worker(Oid domainOid, int prettyFlags);
static void make_ruledef(StringInfo buf, HeapTuple ruletup, TupleDesc rulettc,
int prettyFlags);
static void make_viewdef(StringInfo buf, HeapTuple ruletup, TupleDesc rulettc,
***************
*** 416,421 ****
--- 422,625 ----
return buf.data;
}
+ /*
+ * get_domaindef - Get the definition of a domain
+ */
+ Datum
+ pg_get_domaindef(PG_FUNCTION_ARGS)
+ {
+ Oid domainOid = PG_GETARG_OID(0);
+ PG_RETURN_TEXT_P(string_to_text(pg_get_domaindef_worker(domainOid, 0)));
+ }
+
+ /*
+ * worker code for getting domaindef
+ */
+ static char *
+ pg_get_domaindef_worker(Oid domainOid, int prettyFlags)
+ {
+ Datum args[1];
+ char nulls[1];
+ int spirc;
+ HeapTuple domaintup;
+ HeapTuple typeTuple;
+ TupleDesc domainttc;
+ StringInfoData buf;
+
+ char *domainname;
+ char *typebasetype = NULL;
+ char *typdefault;
+ char *constraint = NULL;
+ char * contype = NULL;
+
+ int fno;
+ Datum dat;
+ bool isnull;
+ bool typnotnull = false;
+
+ Oid basetypeOid;
+ Oid constraintOid;
+
+ /*
+ * Do this first so that string is alloc'd in outer context not SPI's.
+ */
+ initStringInfo(&buf);
+
+ /*
+ * Connect to SPI manager
+ */
+ if (SPI_connect() != SPI_OK_CONNECT)
+ elog(ERROR, "SPI_connect failed");
+
+ /*
+ * On the first call prepare the plan to lookup pg_type. We read
+ * pg_type over the SPI manager instead of using the syscache to be
+ * checked for read access on pg_type.
+ */
+ if (plan_getdomainbyOid == NULL)
+ {
+ Oid argtypes[1];
+ void *plan;
+
+ argtypes[0] = OIDOID;
+ plan = SPI_prepare(query_getdomainbyOid, 1, argtypes);
+ if (plan == NULL)
+ elog(ERROR, "SPI_prepare failed for \"%s\"", query_getdomainbyOid);
+ plan_getdomainbyOid = SPI_saveplan(plan);
+ }
+
+ /*
+ * Get the pg_type tuple for this domain
+ */
+ args[0] = ObjectIdGetDatum(domainOid);
+ nulls[0] = ' ';
+ spirc = SPI_execute_plan(plan_getdomainbyOid, args, nulls, true, 1);
+ if (spirc != SPI_OK_SELECT)
+ elog(ERROR, "failed to get pg_type tuple for domain %u", domainOid);
+ if (SPI_processed != 1)
+ appendStringInfo(&buf, "-");
+ else
+ {
+ /*
+ * Get the types definition and put it into executors memory
+ */
+ domaintup = SPI_tuptable->vals[0];
+ domainttc = SPI_tuptable->tupdesc;
+
+ /*
+ * Get the attribute values from the tuple
+ */
+ fno = SPI_fnumber(domainttc, "typname");
+ dat = SPI_getbinval(domaintup, domainttc, fno, &isnull);
+ Assert(!isnull);
+ domainname = NameStr(*(DatumGetName(dat)));
+
+ /*
+ * Start building the domain definition text
+ */
+ appendStringInfo(&buf, "CREATE DOMAIN %s AS ",
+ quote_identifier(domainname));
+
+ fno = SPI_fnumber(domainttc, "typbasetype");
+ dat = SPI_getbinval(domaintup, domainttc, fno, &isnull);
+ Assert(!isnull);
+ basetypeOid = DatumGetObjectId(dat);
+
+ /*
+ * Get the base type of the domain
+ */
+ typeTuple = SearchSysCache(TYPEOID,
+ ObjectIdGetDatum(basetypeOid),
+ 0, 0, 0);
+
+ if (HeapTupleIsValid(typeTuple))
+ {
+ typebasetype = pstrdup(NameStr(((Form_pg_type) GETSTRUCT(typeTuple))->typname));
+ appendStringInfo(&buf, "%s ", quote_identifier(typebasetype));
+ }
+ ReleaseSysCache(typeTuple);
+
+ /*
+ * Get the default values if any.
+ */
+ fno = SPI_fnumber(domainttc, "typdefault");
+ typdefault = SPI_getvalue(domaintup, domainttc, fno);
+
+ if (typdefault != NULL)
+ {
+ appendStringInfo(&buf, "DEFAULT %s ", quote_identifier(typdefault));
+ }
+
+ /*
+ * Get the constraints. We won't have the constraint name
+ * if it is just a NOT NULL constraint.
+ */
+ fno = SPI_fnumber(domainttc, "typnotnull");
+ dat = SPI_getbinval(domaintup, domainttc, fno, &isnull);
+ Assert(!isnull);
+ typnotnull = DatumGetBool(dat);
+
+ if (plan_getconstraintbycontypid == NULL)
+ {
+ Oid argtypes[1];
+ void *plan;
+
+ argtypes[0] = OIDOID;
+ plan = SPI_prepare(query_getconstraintbycontypid, 1, argtypes);
+
+ if (plan != NULL)
+ plan_getconstraintbycontypid = SPI_saveplan(plan);
+ }
+ args[0] = ObjectIdGetDatum(domainOid);
+ nulls[0] = ' ';
+ SPI_execute_plan(plan_getconstraintbycontypid, args, nulls, true, 1);
+
+ if (SPI_processed == 1)
+ {
+ /*
+ * Get the types definition and put it into executors memory
+ */
+ domaintup = SPI_tuptable->vals[0];
+ domainttc = SPI_tuptable->tupdesc;
+
+ fno = SPI_fnumber(domainttc, "oid");
+ dat = SPI_getbinval(domaintup, domainttc, fno, &isnull);
+ Assert(!isnull);
+ constraintOid = DatumGetObjectId(dat);
+ constraint = pg_get_constraintdef_worker(constraintOid, false, 0);
+
+ //To see if the constraint is a check constraint
+ fno = SPI_fnumber(domainttc, "contype");
+ contype = SPI_getvalue(domaintup, domainttc, fno);
+ }
+
+ 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));
+ }
+ }
+
+ /*
+ * Disconnect from SPI manager
+ */
+ if (SPI_finish() != SPI_OK_FINISH)
+ elog(ERROR, "SPI_finish failed");
+
+ return buf.data;
+ }
+
+
/* ----------
* get_triggerdef - Get the definition of a trigger
* ----------
* Re: pg_get_domaindef
@ 2007-01-19 17:44 FAST PostgreSQL <fastpgs@fast.fujitsu.com.au>
parent: FAST PostgreSQL <fastpgs@fast.fujitsu.com.au>
3 siblings, 0 replies; 21+ messages in thread
From: FAST PostgreSQL @ 2007-01-19 17:44 UTC (permalink / raw)
To: Neil Conway <neilc@samurai.com>; +Cc: pgsql-patches@postgresql.org
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
This is an email from Fujitsu Australia Software Technology Pty Ltd, ABN 27 003 693 481. It is confidential to the ordinary user of the email address to which it was addressed and may contain copyright and/or legally privileged information. No one else may read, print, store, copy or forward all or any of it or its attachments. If you receive this email in error, please return to sender. Thank you.
If you do not wish to receive commercial email messages from Fujitsu Australia Software Technology Pty Ltd, please email unsubscribe@fast.fujitsu.com.au
^ permalink raw reply [nested|flat] 21+ messages in thread
* Re: pg_get_domaindef
@ 2007-01-24 14:19 Andrew Dunstan <andrew@dunslane.net>
parent: FAST PostgreSQL <fastpgs@fast.fujitsu.com.au>
0 siblings, 1 reply; 21+ messages in thread
From: Andrew Dunstan @ 2007-01-24 14:19 UTC (permalink / raw)
To: FAST PostgreSQL <fastpgs@fast.fujitsu.com.au>; +Cc: Neil Conway <neilc@samurai.com>; pgsql-patches@postgresql.org
FAST PostgreSQL wrote:
> Please find attached the patch with modifications
>
are you proposing to implement the other functions in this TODO item
(pg_get_acldef(), pg_get_typedefault(), pg_get_attrdef(),
pg_get_tabledef(), pg_get_functiondef() ) ?
cheers
andrew
^ permalink raw reply [nested|flat] 21+ messages in thread
* Re: pg_get_domaindef
@ 2007-01-24 15:25 Tom Lane <tgl@sss.pgh.pa.us>
parent: Andrew Dunstan <andrew@dunslane.net>
0 siblings, 4 replies; 21+ messages in thread
From: Tom Lane @ 2007-01-24 15:25 UTC (permalink / raw)
To: Andrew Dunstan <andrew@dunslane.net>; +Cc: FAST PostgreSQL <fastpgs@fast.fujitsu.com.au>; Neil Conway <neilc@samurai.com>; pgsql-patches@postgresql.org
Andrew Dunstan <andrew@dunslane.net> writes:
> FAST PostgreSQL wrote:
>> Please find attached the patch with modifications
> are you proposing to implement the other functions in this TODO item
> (pg_get_acldef(), pg_get_typedefault(), pg_get_attrdef(),
> pg_get_tabledef(), pg_get_functiondef() ) ?
I haven't entirely understood the use case for any of these. It's not
pg_dump, for a number of reasons: one being that pg_dump still has to
support older backend versions, and another being that every time we
let backend SnapshotNow functions get involved, we take another hit to
pg_dump's claim to produce a consistent MVCC snapshot.
But my real objection is: do we really want to support duplicative code
in both pg_dump and the backend? Updating pg_dump is already a major
PITA whenever one adds a new feature; doubling that work isn't
attractive. (And it'd be double, not just a copy-and-paste, because of
the large difference in the operating environment.) So I want to hear a
seriously convincing use-case that will justify the maintenance load we
are setting up for ourselves. "Somebody might want this" is not
adequate.
Perhaps a better area of work would be the often-proposed refactoring of
pg_dump into a library and driver program, wherein the library could
expose individual functions such as "fetch the SQL definition of this
object". Unfortunately, that'll be a huge project with no payoff until
the end...
regards, tom lane
^ permalink raw reply [nested|flat] 21+ messages in thread
* Re: [pgsql-patches] pg_get_domaindef
@ 2007-01-24 15:53 Andrew Dunstan <andrew@dunslane.net>
parent: Tom Lane <tgl@sss.pgh.pa.us>
3 siblings, 0 replies; 21+ messages in thread
From: Andrew Dunstan @ 2007-01-24 15:53 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: FAST PostgreSQL <fastpgs@fast.fujitsu.com.au>; Neil Conway <neilc@samurai.com>; pgsql-hackers
[ redirecting discussion to -hackers, where it seems more appropriate ]
Tom Lane wrote:
> Andrew Dunstan <andrew@dunslane.net> writes:
>
>> FAST PostgreSQL wrote:
>>
>>> Please find attached the patch with modifications
>>>
>
>
>> are you proposing to implement the other functions in this TODO item
>> (pg_get_acldef(), pg_get_typedefault(), pg_get_attrdef(),
>> pg_get_tabledef(), pg_get_functiondef() ) ?
>>
>
> I haven't entirely understood the use case for any of these. It's not
> pg_dump, for a number of reasons: one being that pg_dump still has to
> support older backend versions, and another being that every time we
> let backend SnapshotNow functions get involved, we take another hit to
> pg_dump's claim to produce a consistent MVCC snapshot.
>
> But my real objection is: do we really want to support duplicative code
> in both pg_dump and the backend? Updating pg_dump is already a major
> PITA whenever one adds a new feature; doubling that work isn't
> attractive. (And it'd be double, not just a copy-and-paste, because of
> the large difference in the operating environment.) So I want to hear a
> seriously convincing use-case that will justify the maintenance load we
> are setting up for ourselves. "Somebody might want this" is not
> adequate.
>
> Perhaps a better area of work would be the often-proposed refactoring of
> pg_dump into a library and driver program, wherein the library could
> expose individual functions such as "fetch the SQL definition of this
> object". Unfortunately, that'll be a huge project with no payoff until
> the end...
>
>
>
I agree entirely. I'm not sure how big the refactoring would be, but I
do think it's a good goal. Neil mentioned something about it the other day.
It is a worry though that we have an item on the TODO list that has been
worked on and now we might say "Thanks, but no thanks". That's not a
good way to make friends for PostgreSQL. This is why I think we need the
TODO list to be somewhat authoritative, i.e. a list of things that we
have some sort of consensus about doing and commitment to accepting, at
least in principle.
cheers
andrew
^ permalink raw reply [nested|flat] 21+ messages in thread
* Re: pg_get_domaindef
@ 2007-01-24 16:18 FAST PostgreSQL <fastpgs@fast.fujitsu.com.au>
parent: FAST PostgreSQL <fastpgs@fast.fujitsu.com.au>
3 siblings, 1 reply; 21+ messages in thread
From: FAST PostgreSQL @ 2007-01-24 16:18 UTC (permalink / raw)
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
This is an email from Fujitsu Australia Software Technology Pty Ltd, ABN 27 003 693 481. It is confidential to the ordinary user of the email address to which it was addressed and may contain copyright and/or legally privileged information. No one else may read, print, store, copy or forward all or any of it or its attachments. If you receive this email in error, please return to sender. Thank you.
If you do not wish to receive commercial email messages from Fujitsu Australia Software Technology Pty Ltd, please email unsubscribe@fast.fujitsu.com.au
Attachments:
pg_get_domaindef.patch (text/x-diff, 8.0K) download
*** pgsql/src/include/catalog/pg_proc.h 2007-01-24 13:58:40.291574752 +1100
--- workingpgsql/src/include/catalog/pg_proc.h 2007-01-24 15:12:02.410350976 +1100
***************
*** 2315,2321 ****
DESCR("deparse an encoded expression");
DATA(insert OID = 1665 ( pg_get_serial_sequence PGNSP PGUID 12 1 0 f f t f s 2 25 "25 25" _null_ _null_ _null_ pg_get_serial_sequence - _null_ ));
DESCR("name of sequence for a serial column");
!
/* Generic referential integrity constraint triggers */
DATA(insert OID = 1644 ( RI_FKey_check_ins PGNSP PGUID 12 1 0 f f t f v 0 2279 "" _null_ _null_ _null_ RI_FKey_check_ins - _null_ ));
--- 2315,2322 ----
DESCR("deparse an encoded expression");
DATA(insert OID = 1665 ( pg_get_serial_sequence PGNSP PGUID 12 1 0 f f t f s 2 25 "25 25" _null_ _null_ _null_ pg_get_serial_sequence - _null_ ));
DESCR("name of sequence for a serial column");
! DATA(insert OID = 2950 ( pg_get_domaindef PGNSP PGUID 12 1 0 f f t f s 1 25 "26" _null_ _null_ _null_ pg_get_domaindef - _null_ ));
! DESCR("domain description");
/* Generic referential integrity constraint triggers */
DATA(insert OID = 1644 ( RI_FKey_check_ins PGNSP PGUID 12 1 0 f f t f v 0 2279 "" _null_ _null_ _null_ RI_FKey_check_ins - _null_ ));
*** pgsql/src/include/utils/builtins.h 2007-01-18 15:27:15.000000000 +1100
--- workingpgsql/src/include/utils/builtins.h 2007-01-24 14:12:48.167677992 +1100
***************
*** 534,539 ****
--- 534,540 ----
extern Datum pg_get_expr_ext(PG_FUNCTION_ARGS);
extern Datum pg_get_userbyid(PG_FUNCTION_ARGS);
extern Datum pg_get_serial_sequence(PG_FUNCTION_ARGS);
+ extern Datum pg_get_domaindef(PG_FUNCTION_ARGS);
extern char *deparse_expression(Node *expr, List *dpcontext,
bool forceprefix, bool showimplicit);
extern List *deparse_context_for(const char *aliasname, Oid relid);
*** pgsql/src/backend/utils/adt/ruleutils.c 2007-01-24 13:58:28.417379904 +1100
--- workingpgsql/src/backend/utils/adt/ruleutils.c 2007-01-24 15:18:57.944180232 +1100
***************
*** 113,118 ****
--- 113,123 ----
static void *plan_getviewrule = NULL;
static char *query_getviewrule = "SELECT * FROM pg_catalog.pg_rewrite WHERE ev_class = $1 AND rulename = $2";
+ /* Queries to extract domain description */
+ static void *plan_getdomainbyOid = NULL;
+ static char *query_getdomainbyOid = "SELECT * FROM pg_catalog.pg_type WHERE oid = $1 AND typtype = 'd'";
+ static void *plan_getconstraintbycontypid = NULL;
+ static char *query_getconstraintbycontypid = "SELECT oid, contype FROM pg_catalog.pg_constraint WHERE contypid = $1";
/* ----------
* Local functions
***************
*** 135,140 ****
--- 140,146 ----
int prettyFlags);
static char *pg_get_expr_worker(text *expr, Oid relid, char *relname,
int prettyFlags);
+ static char *pg_get_domaindef_worker(Oid domainOid, int prettyFlags);
static void make_ruledef(StringInfo buf, HeapTuple ruletup, TupleDesc rulettc,
int prettyFlags);
static void make_viewdef(StringInfo buf, HeapTuple ruletup, TupleDesc rulettc,
***************
*** 424,429 ****
--- 430,625 ----
return buf.data;
}
+ /*
+ * get_domaindef - Get the definition of a domain
+ */
+ Datum
+ pg_get_domaindef(PG_FUNCTION_ARGS)
+ {
+ Oid domainOid = PG_GETARG_OID(0);
+ PG_RETURN_TEXT_P(string_to_text(pg_get_domaindef_worker(domainOid, 0)));
+ }
+
+ /*
+ * worker code for getting domaindef
+ */
+ static char *
+ pg_get_domaindef_worker(Oid domainOid, int prettyFlags)
+ {
+ Datum args[1];
+ char nulls[1];
+ int spirc;
+ HeapTuple domaintup;
+ HeapTuple typeTuple;
+ TupleDesc domainttc;
+ StringInfoData buf;
+
+ char *domainname;
+ char *typebasetype = NULL;
+ char *typdefault;
+ char *constraint = NULL;
+
+ int fno;
+ Datum dat;
+ bool isnull;
+ bool typnotnull = false;
+
+ Oid basetypeOid;
+ Oid constraintOid;
+
+ /*
+ * Do this first so that string is alloc'd in outer context not SPI's.
+ */
+ initStringInfo(&buf);
+
+ /*
+ * Connect to SPI manager
+ */
+ if (SPI_connect() != SPI_OK_CONNECT)
+ elog(ERROR, "SPI_connect failed");
+
+ /*
+ * On the first call prepare the plan to lookup pg_type. We read
+ * pg_type over the SPI manager instead of using the syscache to be
+ * checked for read access on pg_type.
+ */
+ if (plan_getdomainbyOid == NULL)
+ {
+ Oid argtypes[1];
+ void *plan;
+
+ argtypes[0] = OIDOID;
+ plan = SPI_prepare(query_getdomainbyOid, 1, argtypes);
+ if (plan == NULL)
+ elog(ERROR, "SPI_prepare failed for \"%s\"", query_getdomainbyOid);
+ plan_getdomainbyOid = SPI_saveplan(plan);
+ }
+
+ /*
+ * Get the pg_type tuple for this domain
+ */
+ args[0] = ObjectIdGetDatum(domainOid);
+ nulls[0] = ' ';
+ spirc = SPI_execute_plan(plan_getdomainbyOid, args, nulls, true, 1);
+ if (spirc != SPI_OK_SELECT)
+ elog(ERROR, "failed to get pg_type tuple for domain %u", domainOid);
+ if (SPI_processed != 1)
+ appendStringInfo(&buf, "-");
+ else
+ {
+ /*
+ * Get the types definition and put it into executors memory
+ */
+ domaintup = SPI_tuptable->vals[0];
+ domainttc = SPI_tuptable->tupdesc;
+
+ /*
+ * Get the attribute values from the tuple
+ */
+ fno = SPI_fnumber(domainttc, "typname");
+ dat = SPI_getbinval(domaintup, domainttc, fno, &isnull);
+ Assert(!isnull);
+ domainname = NameStr(*(DatumGetName(dat)));
+
+ /*
+ * Start building the domain definition text
+ */
+ appendStringInfo(&buf, "CREATE DOMAIN %s AS ",
+ quote_identifier(domainname));
+
+ fno = SPI_fnumber(domainttc, "typbasetype");
+ dat = SPI_getbinval(domaintup, domainttc, fno, &isnull);
+ Assert(!isnull);
+ basetypeOid = DatumGetObjectId(dat);
+
+ /*
+ * Get the base type of the domain
+ */
+ typeTuple = SearchSysCache(TYPEOID,
+ ObjectIdGetDatum(basetypeOid),
+ 0, 0, 0);
+
+ if (!HeapTupleIsValid(typeTuple))
+ elog(ERROR, "cache lookup failed for type %u", basetypeOid);
+
+ typebasetype = pstrdup(NameStr(((Form_pg_type) GETSTRUCT(typeTuple))->typname));
+ appendStringInfo(&buf, "%s ", quote_identifier(typebasetype));
+ ReleaseSysCache(typeTuple);
+
+ /*
+ * Get the default values if any.
+ */
+ fno = SPI_fnumber(domainttc, "typdefault");
+ typdefault = SPI_getvalue(domaintup, domainttc, fno);
+
+ if (typdefault != NULL)
+ {
+ appendStringInfo(&buf, "DEFAULT %s ", quote_identifier(typdefault));
+ }
+
+ /*
+ * Get the constraints. We won't have the constraint name
+ * if it is just a NOT NULL constraint.
+ */
+ fno = SPI_fnumber(domainttc, "typnotnull");
+ dat = SPI_getbinval(domaintup, domainttc, fno, &isnull);
+ Assert(!isnull);
+ typnotnull = DatumGetBool(dat);
+
+ if (plan_getconstraintbycontypid == NULL)
+ {
+ Oid argtypes[1];
+ void *plan;
+
+ argtypes[0] = OIDOID;
+ plan = SPI_prepare(query_getconstraintbycontypid, 1, argtypes);
+
+ if (plan != NULL)
+ plan_getconstraintbycontypid = SPI_saveplan(plan);
+ }
+ args[0] = ObjectIdGetDatum(domainOid);
+ nulls[0] = ' ';
+ SPI_execute_plan(plan_getconstraintbycontypid, args, nulls, true, 1);
+
+ if (SPI_processed == 1)
+ {
+ /*
+ * Get the types definition and put it into executors memory
+ */
+ domaintup = SPI_tuptable->vals[0];
+ domainttc = SPI_tuptable->tupdesc;
+
+ fno = SPI_fnumber(domainttc, "oid");
+ dat = SPI_getbinval(domaintup, domainttc, fno, &isnull);
+ Assert(!isnull);
+ constraintOid = DatumGetObjectId(dat);
+ constraint = pg_get_constraintdef_worker(constraintOid, false, 0);
+ }
+
+ if (typnotnull)
+ {
+ appendStringInfo(&buf, "CONSTRAINT NOT NULL ");
+ }
+ else if (constraint != NULL)
+ {
+ appendStringInfo(&buf, "CONSTRAINT ");
+ }
+
+ if (constraint != NULL)
+ {
+ appendStringInfo(&buf, quote_identifier(constraint));
+ }
+ }
+
+ /*
+ * Disconnect from SPI manager
+ */
+ if (SPI_finish() != SPI_OK_FINISH)
+ elog(ERROR, "SPI_finish failed");
+
+ return buf.data;
+ }
+
/* ----------
* get_triggerdef - Get the definition of a trigger
* ----------
* Re: pg_get_domaindef
@ 2007-01-25 04:37 Bruce Momjian <bruce@momjian.us>
parent: Tom Lane <tgl@sss.pgh.pa.us>
3 siblings, 1 reply; 21+ messages in thread
From: Bruce Momjian @ 2007-01-25 04:37 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Andrew Dunstan <andrew@dunslane.net>; FAST PostgreSQL <fastpgs@fast.fujitsu.com.au>; Neil Conway <neilc@samurai.com>; pgsql-patches@postgresql.org
Tom Lane wrote:
> Andrew Dunstan <andrew@dunslane.net> writes:
> > FAST PostgreSQL wrote:
> >> Please find attached the patch with modifications
>
> > are you proposing to implement the other functions in this TODO item
> > (pg_get_acldef(), pg_get_typedefault(), pg_get_attrdef(),
> > pg_get_tabledef(), pg_get_functiondef() ) ?
>
> I haven't entirely understood the use case for any of these. It's not
> pg_dump, for a number of reasons: one being that pg_dump still has to
> support older backend versions, and another being that every time we
> let backend SnapshotNow functions get involved, we take another hit to
> pg_dump's claim to produce a consistent MVCC snapshot.
>
> But my real objection is: do we really want to support duplicative code
> in both pg_dump and the backend? Updating pg_dump is already a major
> PITA whenever one adds a new feature; doubling that work isn't
> attractive. (And it'd be double, not just a copy-and-paste, because of
> the large difference in the operating environment.) So I want to hear a
> seriously convincing use-case that will justify the maintenance load we
> are setting up for ourselves. "Somebody might want this" is not
> adequate.
I realize it is problem to have the function in two places in our code,
but if we don't make a user-accessible version, every application has to
roll their own version and update it for our system catalog changes.
--
Bruce Momjian bruce@momjian.us
EnterpriseDB http://www.enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +
^ permalink raw reply [nested|flat] 21+ messages in thread
* Re: pg_get_domaindef
@ 2007-01-25 05:33 Tom Lane <tgl@sss.pgh.pa.us>
parent: Bruce Momjian <bruce@momjian.us>
0 siblings, 1 reply; 21+ messages in thread
From: Tom Lane @ 2007-01-25 05:33 UTC (permalink / raw)
To: Bruce Momjian <bruce@momjian.us>; +Cc: Andrew Dunstan <andrew@dunslane.net>; FAST PostgreSQL <fastpgs@fast.fujitsu.com.au>; Neil Conway <neilc@samurai.com>; pgsql-patches@postgresql.org
Bruce Momjian <bruce@momjian.us> writes:
> Tom Lane wrote:
>> ... convincing use-case that will justify the maintenance load we
>> are setting up for ourselves. "Somebody might want this" is not
>> adequate.
> I realize it is problem to have the function in two places in our code,
> but if we don't make a user-accessible version, every application has to
> roll their own version and update it for our system catalog changes.
Nope, wrong, you are assuming the conclusion. Exactly which apps have
to have this?
regards, tom lane
^ permalink raw reply [nested|flat] 21+ messages in thread
* Re: pg_get_domaindef
@ 2007-01-25 06:23 Gavin Sherry <swm@alcove.com.au>
parent: Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 0 replies; 21+ messages in thread
From: Gavin Sherry @ 2007-01-25 06:23 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Bruce Momjian <bruce@momjian.us>; Andrew Dunstan <andrew@dunslane.net>; FAST PostgreSQL <fastpgs@fast.fujitsu.com.au>; Neil Conway <neilc@samurai.com>; pgsql-patches@postgresql.org
On Thu, 25 Jan 2007, Tom Lane wrote:
> Bruce Momjian <bruce@momjian.us> writes:
> > Tom Lane wrote:
> >> ... convincing use-case that will justify the maintenance load we
> >> are setting up for ourselves. "Somebody might want this" is not
> >> adequate.
>
> > I realize it is problem to have the function in two places in our code,
> > but if we don't make a user-accessible version, every application has to
> > roll their own version and update it for our system catalog changes.
>
> Nope, wrong, you are assuming the conclusion. Exactly which apps have
> to have this?
Well, the alternative interfaces like pgadmin and ppa. That said, I prefer
the idea of breaking out the queries in pg_dump and psql into a library.
Like you say up thread, that's a big project and it's an all or nothing
proposition.
Thanks,
Gavin
^ permalink raw reply [nested|flat] 21+ messages in thread
* Re: pg_get_domaindef
@ 2007-01-25 06:46 Gavin Sherry <swm@alcove.com.au>
parent: Tom Lane <tgl@sss.pgh.pa.us>
3 siblings, 1 reply; 21+ messages in thread
From: Gavin Sherry @ 2007-01-25 06:46 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Andrew Dunstan <andrew@dunslane.net>; FAST PostgreSQL <fastpgs@fast.fujitsu.com.au>; Neil Conway <neilc@samurai.com>; pgsql-patches@postgresql.org
On Wed, 24 Jan 2007, Tom Lane wrote:
> Andrew Dunstan <andrew@dunslane.net> writes:
> > FAST PostgreSQL wrote:
> >> Please find attached the patch with modifications
>
> > are you proposing to implement the other functions in this TODO item
> > (pg_get_acldef(), pg_get_typedefault(), pg_get_attrdef(),
> > pg_get_tabledef(), pg_get_functiondef() ) ?
>
> I haven't entirely understood the use case for any of these. It's not
> pg_dump, for a number of reasons: one being that pg_dump still has to
> support older backend versions, and another being that every time we
> let backend SnapshotNow functions get involved, we take another hit to
> pg_dump's claim to produce a consistent MVCC snapshot.
I was talking to AndrewSN on irc about this. He proposed that we supply
two versions (yes I hear the collective groan) of the SQL functions: a
fast one (SnapshotNow) and an accurate one (which doesn't use
SnapshotNow).
The accurate version is important not just for pg_dump but for a host of
people who interact with the system catalogs. If anyone's wondering why
people are interacting with system catalogs in the first place, they need
look know further than a monitoring application which checks system health
and sanity on a regular basis. Combine that with some of the SnapshotNow
based get def functions and common enough DDL (like temp table creation)
and you start getting errors which look much more serious than what they
are.
Implementing the accurate version might be done via SPI. This is a
headache though. It's starting to look like pulling the guts out of
pg_dump and putting it in a library :-). Maybe the read place for this is
actually pgfoundry?
Thanks,
Gavin
^ permalink raw reply [nested|flat] 21+ messages in thread
* Re: pg_get_domaindef
@ 2007-01-25 07:06 Tom Lane <tgl@sss.pgh.pa.us>
parent: Gavin Sherry <swm@alcove.com.au>
0 siblings, 1 reply; 21+ messages in thread
From: Tom Lane @ 2007-01-25 07:06 UTC (permalink / raw)
To: Gavin Sherry <swm@alcove.com.au>; +Cc: Andrew Dunstan <andrew@dunslane.net>; FAST PostgreSQL <fastpgs@fast.fujitsu.com.au>; Neil Conway <neilc@samurai.com>; pgsql-patches@postgresql.org
Gavin Sherry <swm@alcove.com.au> writes:
> I was talking to AndrewSN on irc about this. He proposed that we supply
> two versions (yes I hear the collective groan) of the SQL functions: a
> fast one (SnapshotNow) and an accurate one (which doesn't use
> SnapshotNow).
Um, that's such a fundamental misconception that it's got to be nipped
in the bud. The reason the backend tends to operate on SnapshotNow is
that it can't afford to be working with obsolete schema data. As an
example, you'd certainly not be happy if your updates to a table
disappeared into nowhere because your backend was working against a
snapshot that said table X was in tablespace Y, when meanwhile someone
had committed a transaction that moved it to tablespace Z. On the other
hand, pg_dump is entirely not about applying updates; it would like to
have a consistent read-only snapshot as of a time that might be many
hours ago by the time it's done. Both viewpoints are "accurate" for
their respective purposes; neither is chosen because it is "fast".
We might indeed need two sets of functions, but if you categorize them
like that you'll never get it right.
regards, tom lane
^ permalink raw reply [nested|flat] 21+ messages in thread
* Re: pg_get_domaindef
@ 2007-01-25 07:18 Gavin Sherry <swm@alcove.com.au>
parent: Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 0 replies; 21+ messages in thread
From: Gavin Sherry @ 2007-01-25 07:18 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Andrew Dunstan <andrew@dunslane.net>; FAST PostgreSQL <fastpgs@fast.fujitsu.com.au>; Neil Conway <neilc@samurai.com>; pgsql-patches@postgresql.org
On Thu, 25 Jan 2007, Tom Lane wrote:
> Gavin Sherry <swm@alcove.com.au> writes:
> > I was talking to AndrewSN on irc about this. He proposed that we supply
> > two versions (yes I hear the collective groan) of the SQL functions: a
> > fast one (SnapshotNow) and an accurate one (which doesn't use
> > SnapshotNow).
>
> Um, that's such a fundamental misconception that it's got to be nipped
> in the bud. The reason the backend tends to operate on SnapshotNow is
Oops. Poor choice of words.
Thanks,
Gavin
^ permalink raw reply [nested|flat] 21+ messages in thread
* Re: [pgsql-patches] pg_get_domaindef
@ 2007-02-07 23:36 FAST PostgreSQL <fastpgs@fast.fujitsu.com.au>
parent: FAST PostgreSQL <fastpgs@fast.fujitsu.com.au>
3 siblings, 1 reply; 21+ messages in thread
From: FAST PostgreSQL @ 2007-02-07 23:36 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; Andrew Dunstan <andrew@dunslane.net>; +Cc: Neil Conway <neilc@samurai.com>; pgsql-patches@postgresql.org
On Thu, 25 Jan 2007 02:25, Tom Lane wrote:
> Andrew Dunstan <andrew@dunslane.net> writes:
> > FAST PostgreSQL wrote:
> >> Please find attached the patch with modifications
> >
> > are you proposing to implement the other functions in this TODO item
> > (pg_get_acldef(), pg_get_typedefault(), pg_get_attrdef(),
> > pg_get_tabledef(), pg_get_functiondef() ) ?
>
> I haven't entirely understood the use case for any of these. It's not
Any consensus on these functions? If we decide against having these then its
better to remove them from the TODO list temporarily/permanently.........
Rgds,
Arul Shaji
> pg_dump, for a number of reasons: one being that pg_dump still has to
> support older backend versions, and another being that every time we
> let backend SnapshotNow functions get involved, we take another hit to
> pg_dump's claim to produce a consistent MVCC snapshot.
>
> But my real objection is: do we really want to support duplicative code
> in both pg_dump and the backend? Updating pg_dump is already a major
> PITA whenever one adds a new feature; doubling that work isn't
> attractive. (And it'd be double, not just a copy-and-paste, because of
> the large difference in the operating environment.) So I want to hear a
> seriously convincing use-case that will justify the maintenance load we
> are setting up for ourselves. "Somebody might want this" is not
> adequate.
>
> Perhaps a better area of work would be the often-proposed refactoring of
> pg_dump into a library and driver program, wherein the library could
> expose individual functions such as "fetch the SQL definition of this
> object". Unfortunately, that'll be a huge project with no payoff until
> the end...
>
> regards, tom lane
This is an email from Fujitsu Australia Software Technology Pty Ltd, ABN 27 003 693 481. It is confidential to the ordinary user of the email address to which it was addressed and may contain copyright and/or legally privileged information. No one else may read, print, store, copy or forward all or any of it or its attachments. If you receive this email in error, please return to sender. Thank you.
If you do not wish to receive commercial email messages from Fujitsu Australia Software Technology Pty Ltd, please email unsubscribe@fast.fujitsu.com.au
^ permalink raw reply [nested|flat] 21+ messages in thread
* Re: [pgsql-patches] pg_get_domaindef
@ 2007-02-20 22:10 Bruce Momjian <bruce@momjian.us>
parent: Tom Lane <tgl@sss.pgh.pa.us>
3 siblings, 2 replies; 21+ messages in thread
From: Bruce Momjian @ 2007-02-20 22:10 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Andrew Dunstan <andrew@dunslane.net>; FAST PostgreSQL <fastpgs@fast.fujitsu.com.au>; Neil Conway <neilc@samurai.com>; pgsql-hackers
I always felt is was better for us to have server functions that return
schema-specific data rather than require every application to define its
own functions. I realize they are duplicated in pg_dump, but even if we
made an external library that pg_dump could share with applications,
would it only be available to C applications? That seems quite
limiting.
Of course, if people don't need these functions, then we shouldn't have
them.
Seems we have to decide on this one so we can update the TODO or apply
the patch.
---------------------------------------------------------------------------
Tom Lane wrote:
> Andrew Dunstan <andrew@dunslane.net> writes:
> > FAST PostgreSQL wrote:
> >> Please find attached the patch with modifications
>
> > are you proposing to implement the other functions in this TODO item
> > (pg_get_acldef(), pg_get_typedefault(), pg_get_attrdef(),
> > pg_get_tabledef(), pg_get_functiondef() ) ?
>
> I haven't entirely understood the use case for any of these. It's not
> pg_dump, for a number of reasons: one being that pg_dump still has to
> support older backend versions, and another being that every time we
> let backend SnapshotNow functions get involved, we take another hit to
> pg_dump's claim to produce a consistent MVCC snapshot.
>
> But my real objection is: do we really want to support duplicative code
> in both pg_dump and the backend? Updating pg_dump is already a major
> PITA whenever one adds a new feature; doubling that work isn't
> attractive. (And it'd be double, not just a copy-and-paste, because of
> the large difference in the operating environment.) So I want to hear a
> seriously convincing use-case that will justify the maintenance load we
> are setting up for ourselves. "Somebody might want this" is not
> adequate.
>
> Perhaps a better area of work would be the often-proposed refactoring of
> pg_dump into a library and driver program, wherein the library could
> expose individual functions such as "fetch the SQL definition of this
> object". Unfortunately, that'll be a huge project with no payoff until
> the end...
>
> regards, tom lane
>
> ---------------------------(end of broadcast)---------------------------
> TIP 2: Don't 'kill -9' the postmaster
--
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. +
^ permalink raw reply [nested|flat] 21+ messages in thread
* Re: [pgsql-patches] pg_get_domaindef
@ 2007-02-20 22:30 Andrew Dunstan <andrew@dunslane.net>
parent: Bruce Momjian <bruce@momjian.us>
1 sibling, 0 replies; 21+ messages in thread
From: Andrew Dunstan @ 2007-02-20 22:30 UTC (permalink / raw)
To: Bruce Momjian <bruce@momjian.us>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; FAST PostgreSQL <fastpgs@fast.fujitsu.com.au>; Neil Conway <neilc@samurai.com>; pgsql-hackers
Bruce Momjian wrote:
> I always felt is was better for us to have server functions that return
> schema-specific data rather than require every application to define its
> own functions. I realize they are duplicated in pg_dump, but even if we
> made an external library that pg_dump could share with applications,
> would it only be available to C applications? That seems quite
> limiting.
>
I don't think so.
I imagine that the maintainers of DBD::Pg and phppgadmin, for example,
would be very likely to expose them.
And I can certainly imagine using/exposing them in some psql slash commands.
cheers
andrew
^ permalink raw reply [nested|flat] 21+ messages in thread
* Re: [pgsql-patches] pg_get_domaindef
@ 2007-03-02 01:19 FAST PostgreSQL <fastpgs@fast.fujitsu.com.au>
parent: Bruce Momjian <bruce@momjian.us>
1 sibling, 0 replies; 21+ messages in thread
From: FAST PostgreSQL @ 2007-03-02 01:19 UTC (permalink / raw)
To: Bruce Momjian <bruce@momjian.us>; Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Andrew Dunstan <andrew@dunslane.net>; Neil Conway <neilc@samurai.com>; pgsql-patches@postgresql.org
On Wed, 21 Feb 2007 09:10, Bruce Momjian wrote:
Hi,
Following up this patch with a contribution statement.
'With permission from the Managing Director, Fujitsu Australia Software
Technology, I am granting the PostgreSQL Global Development Group the
non-revokable right to distribute the source code in this patch under the BSD
license.'
Rgds,
Arul Shaji
> I always felt is was better for us to have server functions that return
> schema-specific data rather than require every application to define its
> own functions. I realize they are duplicated in pg_dump, but even if we
> made an external library that pg_dump could share with applications,
> would it only be available to C applications? That seems quite
> limiting.
>
> Of course, if people don't need these functions, then we shouldn't have
> them.
>
> Seems we have to decide on this one so we can update the TODO or apply
> the patch.
>
> ---------------------------------------------------------------------------
>
> Tom Lane wrote:
> > Andrew Dunstan <andrew@dunslane.net> writes:
> > > FAST PostgreSQL wrote:
> > >> Please find attached the patch with modifications
> > >
> > > are you proposing to implement the other functions in this TODO item
> > > (pg_get_acldef(), pg_get_typedefault(), pg_get_attrdef(),
> > > pg_get_tabledef(), pg_get_functiondef() ) ?
> >
> > I haven't entirely understood the use case for any of these. It's not
> > pg_dump, for a number of reasons: one being that pg_dump still has to
> > support older backend versions, and another being that every time we
> > let backend SnapshotNow functions get involved, we take another hit to
> > pg_dump's claim to produce a consistent MVCC snapshot.
> >
> > But my real objection is: do we really want to support duplicative code
> > in both pg_dump and the backend? Updating pg_dump is already a major
> > PITA whenever one adds a new feature; doubling that work isn't
> > attractive. (And it'd be double, not just a copy-and-paste, because of
> > the large difference in the operating environment.) So I want to hear a
> > seriously convincing use-case that will justify the maintenance load we
> > are setting up for ourselves. "Somebody might want this" is not
> > adequate.
> >
> > Perhaps a better area of work would be the often-proposed refactoring of
> > pg_dump into a library and driver program, wherein the library could
> > expose individual functions such as "fetch the SQL definition of this
> > object". Unfortunately, that'll be a huge project with no payoff until
> > the end...
> >
> > regards, tom lane
> >
> > ---------------------------(end of broadcast)---------------------------
> > TIP 2: Don't 'kill -9' the postmaster
This is an email from Fujitsu Australia Software Technology Pty Ltd, ABN 27 003 693 481. It is confidential to the ordinary user of the email address to which it was addressed and may contain copyright and/or legally privileged information. No one else may read, print, store, copy or forward all or any of it or its attachments. If you receive this email in error, please return to sender. Thank you.
If you do not wish to receive commercial email messages from Fujitsu Australia Software Technology Pty Ltd, please email unsubscribe@fast.fujitsu.com.au
^ permalink raw reply [nested|flat] 21+ messages in thread
* Re: [pgsql-patches] pg_get_domaindef
@ 2007-03-27 17:27 Bruce Momjian <bruce@momjian.us>
parent: FAST PostgreSQL <fastpgs@fast.fujitsu.com.au>
0 siblings, 0 replies; 21+ messages in thread
From: Bruce Momjian @ 2007-03-27 17:27 UTC (permalink / raw)
To: FAST PostgreSQL <fastpgs@fast.fujitsu.com.au>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; Andrew Dunstan <andrew@dunslane.net>; Neil Conway <neilc@samurai.com>; pgsql-patches@postgresql.org
I have remove this TODO item:
* %Add pg_get_acldef(), pg_get_typedefault(), pg_get_attrdef(),
pg_get_tabledef(), pg_get_domaindef(), pg_get_functiondef()
These would be for application use, not for use by pg_dump.
Seems there is lack of interest in adding this feature because of
maintanance concerns.
The attached patch is rejected for the same reason. Sorry for the
confusion.
---------------------------------------------------------------------------
FAST PostgreSQL wrote:
> On Thu, 25 Jan 2007 02:25, Tom Lane wrote:
> > Andrew Dunstan <andrew@dunslane.net> writes:
> > > FAST PostgreSQL wrote:
> > >> Please find attached the patch with modifications
> > >
> > > are you proposing to implement the other functions in this TODO item
> > > (pg_get_acldef(), pg_get_typedefault(), pg_get_attrdef(),
> > > pg_get_tabledef(), pg_get_functiondef() ) ?
> >
> > I haven't entirely understood the use case for any of these. It's not
>
> Any consensus on these functions? If we decide against having these then its
> better to remove them from the TODO list temporarily/permanently.........
>
> Rgds,
> Arul Shaji
>
>
> > pg_dump, for a number of reasons: one being that pg_dump still has to
> > support older backend versions, and another being that every time we
> > let backend SnapshotNow functions get involved, we take another hit to
> > pg_dump's claim to produce a consistent MVCC snapshot.
> >
> > But my real objection is: do we really want to support duplicative code
> > in both pg_dump and the backend? Updating pg_dump is already a major
> > PITA whenever one adds a new feature; doubling that work isn't
> > attractive. (And it'd be double, not just a copy-and-paste, because of
> > the large difference in the operating environment.) So I want to hear a
> > seriously convincing use-case that will justify the maintenance load we
> > are setting up for ourselves. "Somebody might want this" is not
> > adequate.
> >
> > Perhaps a better area of work would be the often-proposed refactoring of
> > pg_dump into a library and driver program, wherein the library could
> > expose individual functions such as "fetch the SQL definition of this
> > object". Unfortunately, that'll be a huge project with no payoff until
> > the end...
> >
> > regards, tom lane
> This is an email from Fujitsu Australia Software Technology Pty Ltd, ABN 27 003 693 481. It is confidential to the ordinary user of the email address to which it was addressed and may contain copyright and/or legally privileged information. No one else may read, print, store, copy or forward all or any of it or its attachments. If you receive this email in error, please return to sender. Thank you.
>
> If you do not wish to receive commercial email messages from Fujitsu Australia Software Technology Pty Ltd, please email unsubscribe@fast.fujitsu.com.au
>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 3: Have you checked our extensive FAQ?
>
> http://www.postgresql.org/docs/faq
--
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. +
^ permalink raw reply [nested|flat] 21+ messages in thread
end of thread, other threads:[~2007-03-27 17:27 UTC | newest]
Thread overview: 21+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2006-10-26 05:33 pg_get_domaindef() FAST PostgreSQL <fastpgs@fast.fujitsu.com.au>
2006-10-26 14:27 ` Volkan YAZICI <yazicivo@ttnet.net.tr>
2006-10-26 14:52 ` Volkan YAZICI <yazicivo@ttnet.net.tr>
2007-01-19 15:28 pg_get_domaindef FAST PostgreSQL <fastpgs@fast.fujitsu.com.au>
2007-01-19 06:02 ` Re: pg_get_domaindef Neil Conway <neilc@samurai.com>
2007-01-19 17:44 ` Re: pg_get_domaindef FAST PostgreSQL <fastpgs@fast.fujitsu.com.au>
2007-01-24 16:18 ` Re: pg_get_domaindef FAST PostgreSQL <fastpgs@fast.fujitsu.com.au>
2007-01-24 14:19 ` Re: pg_get_domaindef Andrew Dunstan <andrew@dunslane.net>
2007-01-24 15:25 ` Re: pg_get_domaindef Tom Lane <tgl@sss.pgh.pa.us>
2007-01-24 15:53 ` Re: [pgsql-patches] pg_get_domaindef Andrew Dunstan <andrew@dunslane.net>
2007-01-25 04:37 ` Re: pg_get_domaindef Bruce Momjian <bruce@momjian.us>
2007-01-25 05:33 ` Re: pg_get_domaindef Tom Lane <tgl@sss.pgh.pa.us>
2007-01-25 06:23 ` Re: pg_get_domaindef Gavin Sherry <swm@alcove.com.au>
2007-01-25 06:46 ` Re: pg_get_domaindef Gavin Sherry <swm@alcove.com.au>
2007-01-25 07:06 ` Re: pg_get_domaindef Tom Lane <tgl@sss.pgh.pa.us>
2007-01-25 07:18 ` Re: pg_get_domaindef Gavin Sherry <swm@alcove.com.au>
2007-02-20 22:10 ` Re: [pgsql-patches] pg_get_domaindef Bruce Momjian <bruce@momjian.us>
2007-02-20 22:30 ` Re: [pgsql-patches] pg_get_domaindef Andrew Dunstan <andrew@dunslane.net>
2007-03-02 01:19 ` Re: [pgsql-patches] pg_get_domaindef FAST PostgreSQL <fastpgs@fast.fujitsu.com.au>
2007-02-07 23:36 ` Re: [pgsql-patches] pg_get_domaindef FAST PostgreSQL <fastpgs@fast.fujitsu.com.au>
2007-03-27 17:27 ` Re: [pgsql-patches] pg_get_domaindef Bruce Momjian <bruce@momjian.us>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox