Re: [PATCHES] Tentative patch for making DROP put dependency info in DETAIL
On Wed, Jun 11, 2008 at 4:07 PM, Tom Lane [EMAIL PROTECTED] wrote: Agreed --- I committed what I had, anyone want to volunteer for refactoring the execution of DropStmt? Sure! see the attached patch... After looking again, I think that this is not technically very difficult, but coming up with something that looks tasteful to everyone might be tricky. In particular I didn't see a nice way to do it without using struct ObjectAddress in a bunch of header files that don't currently include dependency.h. A possible response to that is to move ObjectAddress into postgres.h, but that seems a bit ugly too. Ok I'm obviously missing something important... Why not Just make the various Remove* functions take a list? I'm not proposing this patch for actual submission, more of a would this work? If I'm not missing something glaring obvious Ill go ahead and make the rest of the Remove things behave the same way dropStmt_mutlidelete.patch Description: Binary data -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
[PATCHES] Better formatting of functions in pg_dump
Attached patch puts the metadata about a function, especially the
language name, at the top of the CREATE FUNCTION statement, above the
possibly long, multi-line function definition.
--
Greg Sabino Mullane
Index: pg_dump.c
===
RCS file: /projects/cvsroot/pgsql/src/bin/pg_dump/pg_dump.c,v
retrieving revision 1.492
diff -c -r1.492 pg_dump.c
*** pg_dump.c 16 May 2008 23:36:05 - 1.492
--- pg_dump.c 12 Jun 2008 13:07:28 -
***
*** 6775,6784
rettypename = getFormattedTypeName(finfo-prorettype, zeroAsOpaque);
appendPQExpBuffer(q, CREATE FUNCTION %s , funcsig);
! appendPQExpBuffer(q, RETURNS %s%s\n%s\nLANGUAGE %s,
(proretset[0] == 't') ? SETOF : ,
rettypename,
- asPart-data,
fmtId(lanname));
free(rettypename);
--- 6775,6783
rettypename = getFormattedTypeName(finfo-prorettype, zeroAsOpaque);
appendPQExpBuffer(q, CREATE FUNCTION %s , funcsig);
! appendPQExpBuffer(q, RETURNS %s%s\nLANGUAGE %s\n,
(proretset[0] == 't') ? SETOF : ,
rettypename,
fmtId(lanname));
free(rettypename);
***
*** 6786,6794
if (provolatile[0] != PROVOLATILE_VOLATILE)
{
if (provolatile[0] == PROVOLATILE_IMMUTABLE)
! appendPQExpBuffer(q, IMMUTABLE);
else if (provolatile[0] == PROVOLATILE_STABLE)
! appendPQExpBuffer(q, STABLE);
else if (provolatile[0] != PROVOLATILE_VOLATILE)
{
write_msg(NULL, unrecognized provolatile value for function \%s\\n,
--- 6785,6793
if (provolatile[0] != PROVOLATILE_VOLATILE)
{
if (provolatile[0] == PROVOLATILE_IMMUTABLE)
! appendPQExpBuffer(q, IMMUTABLE\n);
else if (provolatile[0] == PROVOLATILE_STABLE)
! appendPQExpBuffer(q, STABLE\n);
else if (provolatile[0] != PROVOLATILE_VOLATILE)
{
write_msg(NULL, unrecognized provolatile value for function \%s\\n,
***
*** 6798,6807
}
if (proisstrict[0] == 't')
! appendPQExpBuffer(q, STRICT);
if (prosecdef[0] == 't')
! appendPQExpBuffer(q, SECURITY DEFINER);
/*
* COST and ROWS are emitted only if present and not default, so as not to
--- 6797,6806
}
if (proisstrict[0] == 't')
! appendPQExpBuffer(q, STRICT\n);
if (prosecdef[0] == 't')
! appendPQExpBuffer(q, SECURITY DEFINER\n);
/*
* COST and ROWS are emitted only if present and not default, so as not to
***
*** 6814,6831
{
/* default cost is 1 */
if (strcmp(procost, 1) != 0)
! appendPQExpBuffer(q, COST %s, procost);
}
else
{
/* default cost is 100 */
if (strcmp(procost, 100) != 0)
! appendPQExpBuffer(q, COST %s, procost);
}
}
if (proretset[0] == 't'
strcmp(prorows, 0) != 0 strcmp(prorows, 1000) != 0)
! appendPQExpBuffer(q, ROWS %s, prorows);
for (i = 0; i nconfigitems; i++)
{
--- 6813,6830
{
/* default cost is 1 */
if (strcmp(procost, 1) != 0)
! appendPQExpBuffer(q, COST %s\n, procost);
}
else
{
/* default cost is 100 */
if (strcmp(procost, 100) != 0)
! appendPQExpBuffer(q,COST %s\n, procost);
}
}
if (proretset[0] == 't'
strcmp(prorows, 0) != 0 strcmp(prorows, 1000) != 0)
! appendPQExpBuffer(q,ROWS %s\n, prorows);
for (i = 0; i nconfigitems; i++)
{
***
*** 6837,6843
if (pos == NULL)
continue;
*pos++ = '\0';
! appendPQExpBuffer(q, \nSET %s TO , fmtId(configitem));
/*
* Some GUC variable names are 'LIST' type and hence must not be
--- 6836,6842
if (pos == NULL)
continue;
*pos++ = '\0';
! appendPQExpBuffer(q, SET %s TO , fmtId(configitem));
/*
* Some GUC variable names are 'LIST' type and hence must not be
***
*** 6848,6856
appendPQExpBuffer(q, %s, pos);
else
appendStringLiteralAH(q, pos, fout);
}
! appendPQExpBuffer(q, ;\n);
ArchiveEntry(fout, finfo-dobj.catId, finfo-dobj.dumpId,
funcsig_tag,
--- 6847,6857
appendPQExpBuffer(q, %s, pos);
else
appendStringLiteralAH(q, pos, fout);
+
+ appendPQExpBuffer(q, \n);
}
! appendPQExpBuffer(q, %s;\n, asPart-data);
ArchiveEntry(fout, finfo-dobj.catId, finfo-dobj.dumpId,
funcsig_tag,
--
Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] Better formatting of functions in pg_dump
Greg Sabino Mullane [EMAIL PROTECTED] writes: Attached patch puts the metadata about a function, especially the language name, at the top of the CREATE FUNCTION statement, above the possibly long, multi-line function definition. Why the random switching between newline-before and newline-after styles? Please be consistent. regards, tom lane -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] SQL: table function support
On Mon, Jun 09, 2008 at 05:56:59PM -0700, Neil Conway wrote: On Tue, 2008-06-03 at 13:03 +0200, Pavel Stehule wrote: this patch add support of table functions syntax like ANSI SQL 2003. I'm not necessarily opposed to this, but I wonder if we really need *more* syntax variants for declaring set-returning functions. The existing patchwork of features is confusing enough as it is... The way we declare set-returning functions ranges from odd to byzantine. A clear, easy-to-understand syntax (even if it's just sugar over something else) like Pavel's would go a long way toward getting developers actually to use them. Cheers, David. -- David Fetter [EMAIL PROTECTED] http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: [EMAIL PROTECTED] Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] SQL: table function support
David Fetter [EMAIL PROTECTED] writes: On Mon, Jun 09, 2008 at 05:56:59PM -0700, Neil Conway wrote: I'm not necessarily opposed to this, but I wonder if we really need *more* syntax variants for declaring set-returning functions. The existing patchwork of features is confusing enough as it is... The way we declare set-returning functions ranges from odd to byzantine. A clear, easy-to-understand syntax (even if it's just sugar over something else) like Pavel's would go a long way toward getting developers actually to use them. Apparently, whether the syntax is byzantine or not is in the eye of the beholder. I find the TABLE() syntax to be *less* clear. regards, tom lane -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
[PATCHES] relscan.h split
Hi, relscan.h is very widely used -- in particular it is included by some headers that want the IndexScanDesc and HeapScanDesc definitions in prototypes. However, most of the time they are just passing the struct through; they don't need to see the actual Heap/IndexScanDescData definitions. I propose the following patch which moves the struct definitions to a separate new header relscan_internal.h. Files that actually need the definitions can include the new header. They are not that many -- I count 22 inclusions, all of them in .c files. Headers only include the .h file, which has the benefit that since it is a lean file, it needn't include all the other headers needed by the struct declaration. Zdenek says that this patch changes the number of times certain headers are opened (data gathered using dtrace): new old diff src/include/access/skey.h 347 465 -118 src/include/utils/rel.h 851 921 -70 src/include/access/relscan.h340 360 -20 So it doesn't have a tremendous impact, but it does have some. -- Alvaro Herrerahttp://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support Index: contrib/pgrowlocks/pgrowlocks.c === RCS file: /home/alvherre/Code/cvs/pgsql/contrib/pgrowlocks/pgrowlocks.c,v retrieving revision 1.10 diff -c -p -r1.10 pgrowlocks.c *** contrib/pgrowlocks/pgrowlocks.c 12 May 2008 00:00:43 - 1.10 --- contrib/pgrowlocks/pgrowlocks.c 9 Jun 2008 01:49:31 - *** *** 26,31 --- 26,32 #include access/heapam.h #include access/multixact.h + #include access/relscan_internal.h #include access/xact.h #include catalog/namespace.h #include funcapi.h Index: contrib/pgstattuple/pgstattuple.c === RCS file: /home/alvherre/Code/cvs/pgsql/contrib/pgstattuple/pgstattuple.c,v retrieving revision 1.35 diff -c -p -r1.35 pgstattuple.c *** contrib/pgstattuple/pgstattuple.c 16 May 2008 17:31:17 - 1.35 --- contrib/pgstattuple/pgstattuple.c 9 Jun 2008 01:49:44 - *** *** 29,34 --- 29,35 #include access/heapam.h #include access/htup.h #include access/nbtree.h + #include access/relscan_internal.h #include catalog/namespace.h #include funcapi.h #include miscadmin.h Index: src/backend/access/gin/ginget.c === RCS file: /home/alvherre/Code/cvs/pgsql/src/backend/access/gin/ginget.c,v retrieving revision 1.16 diff -c -p -r1.16 ginget.c *** src/backend/access/gin/ginget.c 16 May 2008 16:31:01 - 1.16 --- src/backend/access/gin/ginget.c 8 Jun 2008 23:58:24 - *** *** 15,20 --- 15,21 #include postgres.h #include access/gin.h + #include access/relscan_internal.h #include catalog/index.h #include miscadmin.h #include storage/bufmgr.h Index: src/backend/access/gin/ginscan.c === RCS file: /home/alvherre/Code/cvs/pgsql/src/backend/access/gin/ginscan.c,v retrieving revision 1.14 diff -c -p -r1.14 ginscan.c *** src/backend/access/gin/ginscan.c 16 May 2008 16:31:01 - 1.14 --- src/backend/access/gin/ginscan.c 8 Jun 2008 23:59:31 - *** *** 16,21 --- 16,22 #include access/genam.h #include access/gin.h + #include access/relscan_internal.h #include pgstat.h #include storage/bufmgr.h #include utils/memutils.h Index: src/backend/access/gist/gistget.c === RCS file: /home/alvherre/Code/cvs/pgsql/src/backend/access/gist/gistget.c,v retrieving revision 1.73 diff -c -p -r1.73 gistget.c *** src/backend/access/gist/gistget.c 12 May 2008 00:00:44 - 1.73 --- src/backend/access/gist/gistget.c 8 Jun 2008 23:55:58 - *** *** 15,20 --- 15,21 #include postgres.h #include access/gist_private.h + #include access/relscan_internal.h #include executor/execdebug.h #include miscadmin.h #include pgstat.h Index: src/backend/access/gist/gistscan.c === RCS file: /home/alvherre/Code/cvs/pgsql/src/backend/access/gist/gistscan.c,v retrieving revision 1.69 diff -c -p -r1.69 gistscan.c *** src/backend/access/gist/gistscan.c 12 May 2008 00:00:44 - 1.69 --- src/backend/access/gist/gistscan.c 8 Jun 2008 23:56:23 - *** *** 17,22 --- 17,23 #include access/genam.h #include access/gist_private.h #include access/gistscan.h + #include
Re: [PATCHES] relscan.h split
Alvaro Herrera [EMAIL PROTECTED] writes: I propose the following patch which moves the struct definitions to a separate new header relscan_internal.h. This seems a little bizarre, seeing that there is almost nothing in relscan.h except those structs. Perhaps a better idea would be to put the opaque-pointer typedefs into heapam.h and genam.h respectively, and then see where you could remove inclusions of relscan.h. Also, it seemed like some of those .c files had no business poking into the scan structs anyway; particularly contrib. Did you check whether the inclusions could be avoided? regards, tom lane -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] Tentative patch for making DROP put dependency info in DETAIL
Alex Hunsaker escribió: Ok I'm obviously missing something important... Why not Just make the various Remove* functions take a list? I'm not proposing this patch for actual submission, more of a would this work? If I'm not missing something glaring obvious Ill go ahead and make the rest of the Remove things behave the same way I don't think there's anything wrong with that in principle. However, does your patch actually work? The changes in expected/ is unexpected, I think. -- Alvaro Herrerahttp://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] Tentative patch for making DROP put dependency info in DETAIL
On Thu, Jun 12, 2008 at 11:35 AM, Alvaro Herrera [EMAIL PROTECTED] wrote: I don't think there's anything wrong with that in principle. However, does your patch actually work? The changes in expected/ is unexpected, I think. Yeah I thought they looked a bit odd at first to. I thought it would just get rid of the duplicate NOTICES's. On closer look they don't NOITCE anymore because all the tables are listed in the drop. Here is an example: # with all them in in drop table create table test (a int primary key); create table test_a (a int references test); create table test_b (a int references test); drop table test, test_a, test_b cascade; DROP TABLE # now without test_b create table test (a int primary key); create table test_a (a int references test); create table test_b (a int references test); drop table test, test_a cascade; NOTICE: drop cascades to constraint test_b_a_fkey on table test_b DROP TABLE In fact you don't even need the cascade anymore if you specify all the dependent tables. So that certainly *seems* right to me. -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] Tentative patch for making DROP put dependency info in DETAIL
Alvaro Herrera [EMAIL PROTECTED] writes: Alex Hunsaker escribió: I'm not proposing this patch for actual submission, more of a would this work? If I'm not missing something glaring obvious Ill go ahead and make the rest of the Remove things behave the same way I don't think there's anything wrong with that in principle. However, does your patch actually work? The changes in expected/ is unexpected, I think. No, they look right --- we're losing gripes about earlier tables cascading to subobjects of later tables, which is exactly what we want. I don't really like the patch though; it seems kind of a brute force solution. You've got ProcessUtility iterating through a list of objects and doing a little bit of work on each one, and then making a new list that RemoveRelation (which is now misnamed) again iterates through and does a little bit of work on each one, and then passes that off *again*. There's no principled division of labor at work there; in particular it's far from obvious where things get locked. You've also managed to embed an assumption not previously present, that all the objects in the list are of exactly the same type. I think it would be better if the DropStmt loop were responsible for constructing a list of ObjectAddresses that it handed off directly to performMultipleDeletions. This is why I was imagining replacing RemoveRelation et al with something that passed back an ObjectAddress, and getting worried about introducing references to ObjectAddress into all those header files. Another possibility would be to get rid of RemoveRelation et al altogether and embed that code right into the DropStmt processing (though at this point we'd need to split it out of ProcessUtility; it's already a bit large for where it is). That didn't seem tremendously clean either, though it would at least have the virtue of improving consistency about where privilege checks etc get done. regards, tom lane -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] SQL: table function support
On Thu, Jun 12, 2008 at 12:33:57PM -0400, Tom Lane wrote: David Fetter [EMAIL PROTECTED] writes: On Mon, Jun 09, 2008 at 05:56:59PM -0700, Neil Conway wrote: I'm not necessarily opposed to this, but I wonder if we really need *more* syntax variants for declaring set-returning functions. The existing patchwork of features is confusing enough as it is... The way we declare set-returning functions ranges from odd to byzantine. A clear, easy-to-understand syntax (even if it's just sugar over something else) like Pavel's would go a long way toward getting developers actually to use them. Apparently, whether the syntax is byzantine or not is in the eye of the beholder. I find the TABLE() syntax to be *less* clear. I went and got reports from the field. Over the years, I've had to explain at great length and with no certain success to developers at a dozen different companies how to use OUT parameters. RETURNS TABLE(...) is *much* more intuitive to those people, who have a tendency to do things like create temp tables rather than figure out the OUT parameter syntax afresh. Cheers, David. -- David Fetter [EMAIL PROTECTED] http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: [EMAIL PROTECTED] Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] SQL: table function support
On Thu, 2008-06-12 at 12:05 -0700, David Fetter wrote: On Thu, Jun 12, 2008 at 12:33:57PM -0400, Tom Lane wrote: David Fetter [EMAIL PROTECTED] writes: On Mon, Jun 09, 2008 at 05:56:59PM -0700, Neil Conway wrote: I went and got reports from the field. Over the years, I've had to explain at great length and with no certain success to developers at a dozen different companies how to use OUT parameters. RETURNS TABLE(...) is *much* more intuitive to those people, who have a tendency to do things like create temp tables rather than figure out the OUT parameter syntax afresh. Regardless of whether anyone thinks they are byzantine (or not) if RETURNS TABLE() is in the standard. We should try and implement it if we can. Sincerely, Joshua D. Drake -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] SQL: table function support
On Thu, Jun 12, 2008 at 12:33:57PM -0400, Tom Lane wrote: David Fetter [EMAIL PROTECTED] writes: On Mon, Jun 09, 2008 at 05:56:59PM -0700, Neil Conway wrote: I'm not necessarily opposed to this, but I wonder if we really need *more* syntax variants for declaring set-returning functions. The existing patchwork of features is confusing enough as it is... The way we declare set-returning functions ranges from odd to byzantine. A clear, easy-to-understand syntax (even if it's just sugar over something else) like Pavel's would go a long way toward getting developers actually to use them. Apparently, whether the syntax is byzantine or not is in the eye of the beholder. I find the TABLE() syntax to be *less* clear. Perhaps, but I can see explaining it to my over-busy-non-doc-reading developers much more easily than the existing choices. Of course then they will all want to write set returning functions, so I may end up regretting it. -dg -- David Gould [EMAIL PROTECTED] 510 536 1443510 282 0869 If simplicity worked, the world would be overrun with insects. -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] Tentative patch for making DROP put dependency info in DETAIL
On Thu, Jun 12, 2008 at 11:58 AM, Tom Lane [EMAIL PROTECTED] wrote: I don't really like the patch though; it seems kind of a brute force solution. You've got ProcessUtility iterating through a list of objects and doing a little bit of work on each one, and then making a new list that RemoveRelation (which is now misnamed) again iterates through and does a little bit of work on each one, and then passes that off *again*. There's no principled division of labor at work there; in particular it's far from obvious where things get locked. You've also managed to embed an assumption not previously present, that all the objects in the list are of exactly the same type. Yep, I thought about doing the reverse. Namely Just passing the DropStmt to RemoveRelation(s). But then all the permission check functions are in utility.c. Splitting those out seemed to be about the same as splitting out all the ObjectAddress stuff... Plus with the potential ugliness I thought maybe this (as it is in the patch) way while still ugly, might be the less of two uglies :) And besides it was brain dead simple... I think it would be better if the DropStmt loop were responsible for constructing a list of ObjectAddresses that it handed off directly to performMultipleDeletions. This is why I was imagining replacing RemoveRelation et al with something that passed back an ObjectAddress, and getting worried about introducing references to ObjectAddress into all those header files. Another possibility would be to get rid of RemoveRelation et al altogether and embed that code right into the DropStmt processing (though at this point we'd need to split it out of ProcessUtility; it's already a bit large for where it is). That didn't seem tremendously clean either, though it would at least have the virtue of improving consistency about where privilege checks etc get done. It seems strange to have _not_ have the permission checks in RemoveRelation IMHO. Granted utility.c is the only thing that calls it... It seems equally strange to (re)move RemoveRelation and friends into utility.c. But really if some other user besides utility.c was going to use them wouldn't they want the permission checks? So my vote is to just move them into utility.c maybe even make them static (until someone else needs them obviosly). Make them return an ObjectAddress (so they wont be called RemoveType, but getTypeAddress or something) and be done with it. Thoughts? Unless you thought of a cleaner way ? :) -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] Tentative patch for making DROP put dependency info in DETAIL
Alex Hunsaker [EMAIL PROTECTED] writes: Yep, I thought about doing the reverse. Namely Just passing the DropStmt to RemoveRelation(s). But then all the permission check functions are in utility.c. Splitting those out seemed to be about the same as splitting out all the ObjectAddress stuff... Well, that might actually be a good approach: try to get ProcessUtility back down to being just a dispatch switch. It's pretty much of a wart that we're doing any permissions checking in utility.c at all. Possibly those functions should be moved to aclchk.c and then used from RemoveRelation(s) and friends, which would stay where they are but change API. I think the fundamental tension here is between two theories of organizing the code: we've got the notion of collect operations on an object type together, which leads to eg putting RemoveTSConfiguration in tsearchcmds.c, as against collect similar operations together, which leads to wanting all the DROP operations in the same module. In the abstract it's not too easy to choose between these, but I think you'll probably find that the first one works better here, mainly because each of those object-type modules knows how to work with a particular system catalog. A DROP module is going to find itself importing all the catalog headers plus probably utility routines from all over. So that's leading me to lean towards keeping RemoveRelation et al where they are and distributing the work currently done in ProcessUtility out to them. This sounds duplicative, but about all that really is there to duplicate is a foreach loop, which you're going to need anyway if the routines are to handle multiple objects. regards, tom lane -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] Tentative patch for making DROP put dependency info in DETAIL
On Thu, Jun 12, 2008 at 5:35 PM, Tom Lane [EMAIL PROTECTED] wrote: Alex Hunsaker [EMAIL PROTECTED] writes: Yep, I thought about doing the reverse. Namely Just passing the DropStmt to RemoveRelation(s). But then all the permission check functions are in utility.c. Splitting those out seemed to be about the same as splitting out all the ObjectAddress stuff... Well, that might actually be a good approach: try to get ProcessUtility back down to being just a dispatch switch. It's pretty much of a wart that we're doing any permissions checking in utility.c at all. Possibly those functions should be moved to aclchk.c and then used from RemoveRelation(s) and friends, which would stay where they are but change API. Ok Ill work up a patch. Whats that saying about sticking with your first instinct? -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] Better formatting of functions in pg_dump
-BEGIN PGP SIGNED MESSAGE- Hash: RIPEMD160 Attached patch puts the metadata about a function, especially the language name, at the top of the CREATE FUNCTION statement, above the possibly long, multi-line function definition. Why the random switching between newline-before and newline-after styles? Please be consistent. I thought they were all after. On second glance, they still seem all after? - -- Greg Sabino Mullane [EMAIL PROTECTED] End Point Corporation PGP Key: 0x14964AC8 200806122044 http://biglumber.com/x/web?pk=2529DF6AB8F79407E94445B4BC9B906714964AC8 -BEGIN PGP SIGNATURE- iEYEAREDAAYFAkhRww0ACgkQvJuQZxSWSshuQwCfYBjBLOVfJziHcyHRM4CNfCaY gncAoK+CehREYJQdvAXfizZIPjZog4c6 =A+aR -END PGP SIGNATURE- -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
[PATCHES] small typo in DTrace docs
Hi,
Attached is a small patch to fix some typos in probes.d path.
--
Euler Taveira de Oliveira
http://www.timbira.com/
Index: doc/src/sgml/monitoring.sgml
===
RCS file: /a/pgsql/dev/anoncvs/pgsql/doc/src/sgml/monitoring.sgml,v
retrieving revision 1.59
diff -c -r1.59 monitoring.sgml
*** doc/src/sgml/monitoring.sgml15 May 2008 00:17:39 - 1.59
--- doc/src/sgml/monitoring.sgml13 Jun 2008 04:42:57 -
***
*** 1190,1196
step
para
! Add the probe definitons to filenamesrc/backend/src/utils/probes.d/
/para
/step
--- 1190,1196
step
para
! Add the probe definitons to filenamesrc/backend/utils/probes.d/
/para
/step
***
*** 1224,1230
step
para
Add quoteprobe transaction__start(int);/quote to
! filenamesrc/backend/src/utils/probes.d/, and it should look like the
following:
programlisting
provider postgresql {
...
--- 1224,1230
step
para
Add quoteprobe transaction__start(int);/quote to
! filenamesrc/backend/utils/probes.d/, and it should look like the
following:
programlisting
provider postgresql {
...
***
*** 1247,1253
para
At compile time, transaction__start is converted to a macro called
TRACE_POSTGRESQL_TRANSACTION_START, and it resides in
! filenamesrc/backend/src/utils/probes.h/. Before recompiling, add
the single line macro to the appropriate location in the source code.
In this case, it looks like the following:
/para
--- 1247,1253
para
At compile time, transaction__start is converted to a macro called
TRACE_POSTGRESQL_TRANSACTION_START, and it resides in
! filenamesrc/backend/utils/probes.h/. Before recompiling, add
the single line macro to the appropriate location in the source code.
In this case, it looks like the following:
/para
--
Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] Tentative patch for making DROP put dependency info in DETAIL
On Thu, Jun 12, 2008 at 5:35 PM, Tom Lane [EMAIL PROTECTED] wrote: So that's leading me to lean towards keeping RemoveRelation et al where they are and distributing the work currently done in ProcessUtility out to them. This sounds duplicative, but about all that really is there to duplicate is a foreach loop, which you're going to need anyway if the routines are to handle multiple objects. Ok Here it is: -Moves CheckDropPermissions and friends from utility.c to aclchk.c (pg_drop_permission_check) -Makes all the Remove* functions take a DropStmt *, they each do their own foreach() loop and permission checks -removed RemoveView and RemoveIndex because they were exactly the same as RemoveRelation -added an s to the end of the Remove* functions to denote they may remove more than one (i.e. RemoveRelations) -consolidated RemoveType and RemoveDomain into a common function (static void removeHelper()) -made performMultipleDeletions when we only have one item we are deleting log the same way (with the object name) src/backend/catalog/aclchk.c | 154 +++ src/backend/catalog/dependency.c |9 +- src/backend/catalog/pg_conversion.c | 54 --- src/backend/commands/conversioncmds.c | 45 +++-- src/backend/commands/indexcmds.c | 27 --- src/backend/commands/schemacmds.c | 91 + src/backend/commands/tablecmds.c | 66 ++- src/backend/commands/tsearchcmds.c| 290 + src/backend/commands/typecmds.c | 189 --- src/backend/commands/view.c | 23 --- src/backend/tcop/utility.c| 288 + src/include/catalog/pg_conversion_fn.h|2 +- src/include/commands/conversioncmds.h |3 +- src/include/commands/defrem.h | 14 +- src/include/commands/schemacmds.h |2 +- src/include/commands/tablecmds.h |2 +- src/include/commands/typecmds.h |4 +- src/include/commands/view.h |2 +- src/include/utils/acl.h |1 + src/test/regress/expected/foreign_key.out | 11 - src/test/regress/expected/truncate.out|6 - 21 files changed, 645 insertions(+), 638 deletions(-) refactor_dropstmt.patch.gz Description: GNU Zip compressed data -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
