Re: [HACKERS] ALTER OBJECT any_name SET SCHEMA name
On Sat, Nov 27, 2010 at 2:17 PM, Dimitri Fontaine dimi...@2ndquadrant.fr wrote: Thanks! The _oid variants will have to re-appear in the alter extension set schema patch, which is the last of the series. Meanwhile, I will have to merge head with the current extension patch (already overdue for a new version, waiting on purpose) so that it no longer includes the cfparser and execute from file patches too (which have changed a lot underneath it already). I expected as much, but at least at that point it will be possible to test that code. I'm not sure there's lots of precedent for dealing with in-commitfest patches dependencies, so here's a summary of what I think would ideally happen next (ordering counts): 1. cfparser https://commitfest.postgresql.org/action/patch_view?id=413 ready for commiter 2. pg_execute_from_file https://commitfest.postgresql.org/action/patch_view?id=414 needs another review and maybe some more documentation 3. extensions https://commitfest.postgresql.org/action/patch_view?id=404 needs review and minor updating will need another version after merging the two previous patches 4. alter extension set schema https://commitfest.postgresql.org/action/patch_view?id=416 needs review and a reviewer will need bitrot fix (and adding in the _oid variants) would be better to adjust once the 3 previous are in Thanks, that's very helpful. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] ALTER OBJECT any_name SET SCHEMA name
Robert Haas robertmh...@gmail.com writes: Committed, after various changes and corrections. One noteworthy one is that I removed the _oid variants, since those would be dead code at the moment. Thanks! The _oid variants will have to re-appear in the alter extension set schema patch, which is the last of the series. Meanwhile, I will have to merge head with the current extension patch (already overdue for a new version, waiting on purpose) so that it no longer includes the cfparser and execute from file patches too (which have changed a lot underneath it already). I'm not sure there's lots of precedent for dealing with in-commitfest patches dependencies, so here's a summary of what I think would ideally happen next (ordering counts): 1. cfparser https://commitfest.postgresql.org/action/patch_view?id=413 ready for commiter 2. pg_execute_from_file https://commitfest.postgresql.org/action/patch_view?id=414 needs another review and maybe some more documentation 3. extensions https://commitfest.postgresql.org/action/patch_view?id=404 needs review and minor updating will need another version after merging the two previous patches 4. alter extension set schema https://commitfest.postgresql.org/action/patch_view?id=416 needs review and a reviewer will need bitrot fix (and adding in the _oid variants) would be better to adjust once the 3 previous are in Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] ALTER OBJECT any_name SET SCHEMA name
On Thu, Nov 25, 2010 at 5:00 PM, Dimitri Fontaine dimi...@2ndquadrant.fr wrote: Robert Haas robertmh...@gmail.com writes: Please do. Tab completion support should really be included in the patch - adding it as a separate patch is better than not having it, of course. Please find attached version 9 of the patch, which includes psql completion support of the SET SCHEMA variant of already supported ALTER commands. That means I didn't add ALTER OPERATOR [CLASS,FAMILY] completion support, my guess being there's no demand here, or the existing syntax variants would be there already. And if there's demand, I don't feel like it should be implemented as part of this very patch. Committed, after various changes and corrections. One noteworthy one is that I removed the _oid variants, since those would be dead code at the moment. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] ALTER OBJECT any_name SET SCHEMA name
Robert Haas robertmh...@gmail.com writes:
Please do. Tab completion support should really be included in the
patch - adding it as a separate patch is better than not having it, of
course.
Please find attached version 9 of the patch, which includes psql
completion support of the SET SCHEMA variant of already supported
ALTER commands.
That means I didn't add ALTER OPERATOR [CLASS,FAMILY] completion
support, my guess being there's no demand here, or the existing syntax
variants would be there already. And if there's demand, I don't feel
like it should be implemented as part of this very patch.
Regards,
--
Dimitri Fontaine
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
*** a/doc/src/sgml/ref/alter_conversion.sgml
--- b/doc/src/sgml/ref/alter_conversion.sgml
***
*** 23,28 PostgreSQL documentation
--- 23,29
synopsis
ALTER CONVERSION replaceablename/replaceable RENAME TO replaceablenew_name/replaceable
ALTER CONVERSION replaceablename/replaceable OWNER TO replaceablenew_owner/replaceable
+ ALTER CONVERSION replaceablename/replaceable SET SCHEMA replaceablenew_schema/replaceable
/synopsis
/refsynopsisdiv
***
*** 75,80 ALTER CONVERSION replaceablename/replaceable OWNER TO replaceablenew_owner
--- 76,90
/para
/listitem
/varlistentry
+
+varlistentry
+ termreplaceable class=parameternew_schema/replaceable/term
+ listitem
+ para
+ The new schema of the conversion.
+ /para
+ /listitem
+/varlistentry
/variablelist
/refsect1
*** a/doc/src/sgml/ref/alter_opclass.sgml
--- b/doc/src/sgml/ref/alter_opclass.sgml
***
*** 23,28 PostgreSQL documentation
--- 23,29
synopsis
ALTER OPERATOR CLASS replaceablename/replaceable USING replaceable class=parameterindex_method/replaceable RENAME TO replaceablenew_name/replaceable
ALTER OPERATOR CLASS replaceablename/replaceable USING replaceable class=parameterindex_method/replaceable OWNER TO replaceablenew_owner/replaceable
+ ALTER OPERATOR CLASS replaceablename/replaceable USING replaceable class=parameterindex_method/replaceable SET SCHEMA replaceablenew_schema/replaceable
/synopsis
/refsynopsisdiv
***
*** 85,90 ALTER OPERATOR CLASS replaceablename/replaceable USING replaceable class=p
--- 86,100
/para
/listitem
/varlistentry
+
+varlistentry
+ termreplaceable class=parameternew_schema/replaceable/term
+ listitem
+ para
+ The new schema for the operator class.
+ /para
+ /listitem
+/varlistentry
/variablelist
/refsect1
*** a/doc/src/sgml/ref/alter_operator.sgml
--- b/doc/src/sgml/ref/alter_operator.sgml
***
*** 22,27 PostgreSQL documentation
--- 22,28
refsynopsisdiv
synopsis
ALTER OPERATOR replaceablename/replaceable ( { replaceableleft_type/replaceable | NONE } , { replaceableright_type/replaceable | NONE } ) OWNER TO replaceablenew_owner/replaceable
+ ALTER OPERATOR replaceablename/replaceable ( { replaceableleft_type/replaceable | NONE } , { replaceableright_type/replaceable | NONE } ) SET SCHEMA replaceablenew_schema/replaceable
/synopsis
/refsynopsisdiv
***
*** 85,90 ALTER OPERATOR replaceablename/replaceable ( { replaceableleft_type/repla
--- 86,100
/para
/listitem
/varlistentry
+
+varlistentry
+ termreplaceable class=parameternew_schema/replaceable/term
+ listitem
+ para
+ The new schema of the operator.
+ /para
+ /listitem
+/varlistentry
/variablelist
/refsect1
*** a/doc/src/sgml/ref/alter_opfamily.sgml
--- b/doc/src/sgml/ref/alter_opfamily.sgml
***
*** 31,36 ALTER OPERATOR FAMILY replaceablename/replaceable USING replaceable class=
--- 31,37
} [, ... ]
ALTER OPERATOR FAMILY replaceablename/replaceable USING replaceable class=parameterindex_method/replaceable RENAME TO replaceablenew_name/replaceable
ALTER OPERATOR FAMILY replaceablename/replaceable USING replaceable class=parameterindex_method/replaceable OWNER TO replaceablenew_owner/replaceable
+ ALTER OPERATOR FAMILY replaceablename/replaceable USING replaceable class=parameterindex_method/replaceable SET SCHEMA replaceablenew_schema/replaceable
/synopsis
/refsynopsisdiv
***
*** 216,221 ALTER OPERATOR FAMILY replaceablename/replaceable USING replaceable class=
--- 217,231
/para
/listitem
/varlistentry
+
+varlistentry
+ termreplaceable class=parameternew_schema/replaceable/term
+ listitem
+ para
+ The new schema for the operator family.
+ /para
+ /listitem
+/varlistentry
/variablelist
para
*** a/doc/src/sgml/ref/alter_tsconfig.sgml
--- b/doc/src/sgml/ref/alter_tsconfig.sgml
***
*** 33,38 ALTER TEXT SEARCH CONFIGURATION replaceablename/replaceable
--- 33,39
DROP MAPPING [ IF EXISTS ]
Re: [HACKERS] ALTER OBJECT any_name SET SCHEMA name
Robert Haas robertmh...@gmail.com writes: Especially because you also posted some revs of the ALTER EXTENSION .. SET SCHEMA patch on this thread Yes, I tried to answer where questions have been raised, and that's not helping so much at review time. That's why I take the time to update the commit fest application each time I send a new patch version. Do you still want me to prepare another patch for adding in the tests the set schema variants that already existed but are not yet covered? Which are the one you did spot, btw? [rhaas pgsql]$ git grep 'SET SCHEMA' src/test/regress/ [rhaas pgsql]$ The existing 'set schema' tests are in lower case, so I just did it the same, try with git grep -i maybe :) grep -c 'set schema' ../postgresql-extension-patches/set_schema.v8.patch 28 Please do. Tab completion support should really be included in the patch - adding it as a separate patch is better than not having it, of course. Ok, will learn about this part of psql soon'ish, hopefully by today, given a reasonable amount of other distractions. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] ALTER OBJECT any_name SET SCHEMA name
On Sun, Nov 21, 2010 at 4:47 PM, Dimitri Fontaine dimi...@2ndquadrant.fr wrote: Robert Haas robertmh...@gmail.com writes: On Sat, Nov 20, 2010 at 11:23 PM, Robert Haas robertmh...@gmail.com wrote: Ah, nuts. I see now there's a v7. Never mind... OK. I looked at the right version, now. Hopefully. Yeah, that was the most recent one and I linked it in the commit fest application. Given the very fast feedback I got, there has been a lot of activity and patches versions produced, so that's easy to get confused. Especially because you also posted some revs of the ALTER EXTENSION .. SET SCHEMA patch on this thread It seems we have no regression tests at all for any of the existing SET SCHEMA commands. This seems like a good time to correct that oversight, and also add some for the new commands you're adding here. Yeah, it's time for me to have a look at regression tests :) Please find attached set_schema.v8.patch with tests for the added commands in the patch. (It might be helpful to submit the regression tests for the existing commands as a separate patch.) Also, you're missing psql tab completion support, which would be nice to have. Do you still want me to prepare another patch for adding in the tests the set schema variants that already existed but are not yet covered? Which are the one you did spot, btw? [rhaas pgsql]$ git grep 'SET SCHEMA' src/test/regress/ [rhaas pgsql]$ Completion support for psql. Isn't that stepping on David's toes? :) I'll see about that later if needed, maybe sometime tomorrow… Please do. Tab completion support should really be included in the patch - adding it as a separate patch is better than not having it, of course. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] ALTER OBJECT any_name SET SCHEMA name
On Sat, Nov 20, 2010 at 11:23 PM, Robert Haas robertmh...@gmail.com wrote: Ah, nuts. I see now there's a v7. Never mind... OK. I looked at the right version, now. Hopefully. It seems we have no regression tests at all for any of the existing SET SCHEMA commands. This seems like a good time to correct that oversight, and also add some for the new commands you're adding here. (It might be helpful to submit the regression tests for the existing commands as a separate patch.) Also, you're missing psql tab completion support, which would be nice to have. In CheckSetNamespace() you have the message 'already exists in schema' there where the existing, similar checks say 'is already in schema', which is a bit confusing. But that code looks useful, and in fact I think we should use it for the existing object types also to avoid code duplication. This is technically a regression in terms of translatability, since instead of a single string that says something like 'function %s is already in schema %s', you'll have '%s is already in schema %s', and where the first %s is provided by getObjectDescription(). But that doesn't seem like a problem, because (1) we're already doing it that way for dependency error messages anyway and (2) as far as I can tell from a visual scan and some hacking with Google Translate, all of the languages for which we have backend translations put the object type next to the object name anyway. So, attached is a proposed patch that just adds CheckSetNamespace() and makes the existing SET SCHEMA commands use it. Barring objections, I'll go ahead and commit this part. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company check_set_namespace.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] ALTER OBJECT any_name SET SCHEMA name
On Sun, Nov 21, 2010 at 07:53:57AM -0500, Robert Haas wrote: On Sat, Nov 20, 2010 at 11:23 PM, Robert Haas robertmh...@gmail.com wrote: Ah, nuts. I see now there's a v7. Never mind... OK. I looked at the right version, now. Hopefully. It seems we have no regression tests at all for any of the existing SET SCHEMA commands. This seems like a good time to correct that oversight, and also add some for the new commands you're adding here. (It might be helpful to submit the regression tests for the existing commands as a separate patch.) Also, you're missing psql tab completion support, which would be nice to have. In CheckSetNamespace() you have the message 'already exists in schema' there where the existing, similar checks say 'is already in schema', which is a bit confusing. But that code looks useful, and in fact I think we should use it for the existing object types also to avoid code duplication. Should this really error out? It's just a NOOP, so perhaps a NOTICE would be more appropriate. Cheers, David. -- David Fetter da...@fetter.org http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fet...@gmail.com iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] ALTER OBJECT any_name SET SCHEMA name
On Nov 21, 2010, at 1:03 PM, David Fetter da...@fetter.org wrote: Should this really error out? It's just a NOOP, so perhaps a NOTICE would be more appropriate. Perhaps, but the purpose of this patch is to streamline the code, not change the behavior. ...Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] ALTER OBJECT any_name SET SCHEMA name
Robert Haas robertmh...@gmail.com writes: So, attached is a proposed patch that just adds CheckSetNamespace() and makes the existing SET SCHEMA commands use it. Barring objections, I'll go ahead and commit this part. Thank you for applying the new function to the existing code paths, that was needed as soon as the new function would get acceptance! :) Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] ALTER OBJECT any_name SET SCHEMA name
Robert Haas robertmh...@gmail.com writes:
On Sat, Nov 20, 2010 at 11:23 PM, Robert Haas robertmh...@gmail.com wrote:
Ah, nuts. I see now there's a v7. Never mind...
OK. I looked at the right version, now. Hopefully.
Yeah, that was the most recent one and I linked it in the commit fest
application. Given the very fast feedback I got, there has been a lot of
activity and patches versions produced, so that's easy to get confused.
It seems we have no regression tests at all for any of the existing
SET SCHEMA commands. This seems like a good time to correct that
oversight, and also add some for the new commands you're adding here.
Yeah, it's time for me to have a look at regression tests :)
Please find attached set_schema.v8.patch with tests for the added
commands in the patch.
(It might be helpful to submit the regression tests for the existing
commands as a separate patch.) Also, you're missing psql tab
completion support, which would be nice to have.
Do you still want me to prepare another patch for adding in the tests
the set schema variants that already existed but are not yet covered?
Which are the one you did spot, btw?
Completion support for psql. Isn't that stepping on David's toes? :)
I'll see about that later if needed, maybe sometime tomorrow…
Regards,
--
Dimitri Fontaine
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
*** a/doc/src/sgml/ref/alter_conversion.sgml
--- b/doc/src/sgml/ref/alter_conversion.sgml
***
*** 23,28 PostgreSQL documentation
--- 23,29
synopsis
ALTER CONVERSION replaceablename/replaceable RENAME TO replaceablenew_name/replaceable
ALTER CONVERSION replaceablename/replaceable OWNER TO replaceablenew_owner/replaceable
+ ALTER CONVERSION replaceablename/replaceable SET SCHEMA replaceablenew_schema/replaceable
/synopsis
/refsynopsisdiv
***
*** 75,80 ALTER CONVERSION replaceablename/replaceable OWNER TO replaceablenew_owner
--- 76,90
/para
/listitem
/varlistentry
+
+varlistentry
+ termreplaceable class=parameternew_schema/replaceable/term
+ listitem
+ para
+ The new schema of the conversion.
+ /para
+ /listitem
+/varlistentry
/variablelist
/refsect1
*** a/doc/src/sgml/ref/alter_opclass.sgml
--- b/doc/src/sgml/ref/alter_opclass.sgml
***
*** 23,28 PostgreSQL documentation
--- 23,29
synopsis
ALTER OPERATOR CLASS replaceablename/replaceable USING replaceable class=parameterindex_method/replaceable RENAME TO replaceablenew_name/replaceable
ALTER OPERATOR CLASS replaceablename/replaceable USING replaceable class=parameterindex_method/replaceable OWNER TO replaceablenew_owner/replaceable
+ ALTER OPERATOR CLASS replaceablename/replaceable USING replaceable class=parameterindex_method/replaceable SET SCHEMA replaceablenew_schema/replaceable
/synopsis
/refsynopsisdiv
***
*** 85,90 ALTER OPERATOR CLASS replaceablename/replaceable USING replaceable class=p
--- 86,100
/para
/listitem
/varlistentry
+
+varlistentry
+ termreplaceable class=parameternew_schema/replaceable/term
+ listitem
+ para
+ The new schema for the operator class.
+ /para
+ /listitem
+/varlistentry
/variablelist
/refsect1
*** a/doc/src/sgml/ref/alter_operator.sgml
--- b/doc/src/sgml/ref/alter_operator.sgml
***
*** 22,27 PostgreSQL documentation
--- 22,28
refsynopsisdiv
synopsis
ALTER OPERATOR replaceablename/replaceable ( { replaceableleft_type/replaceable | NONE } , { replaceableright_type/replaceable | NONE } ) OWNER TO replaceablenew_owner/replaceable
+ ALTER OPERATOR replaceablename/replaceable ( { replaceableleft_type/replaceable | NONE } , { replaceableright_type/replaceable | NONE } ) SET SCHEMA replaceablenew_schema/replaceable
/synopsis
/refsynopsisdiv
***
*** 85,90 ALTER OPERATOR replaceablename/replaceable ( { replaceableleft_type/repla
--- 86,100
/para
/listitem
/varlistentry
+
+varlistentry
+ termreplaceable class=parameternew_schema/replaceable/term
+ listitem
+ para
+ The new schema of the operator.
+ /para
+ /listitem
+/varlistentry
/variablelist
/refsect1
*** a/doc/src/sgml/ref/alter_opfamily.sgml
--- b/doc/src/sgml/ref/alter_opfamily.sgml
***
*** 31,36 ALTER OPERATOR FAMILY replaceablename/replaceable USING replaceable class=
--- 31,37
} [, ... ]
ALTER OPERATOR FAMILY replaceablename/replaceable USING replaceable class=parameterindex_method/replaceable RENAME TO replaceablenew_name/replaceable
ALTER OPERATOR FAMILY replaceablename/replaceable USING replaceable class=parameterindex_method/replaceable OWNER TO replaceablenew_owner/replaceable
+ ALTER OPERATOR FAMILY replaceablename/replaceable USING replaceable class=parameterindex_method/replaceable SET SCHEMA
Re: [HACKERS] ALTER OBJECT any_name SET SCHEMA name
On Thu, Nov 4, 2010 at 3:39 PM, Dimitri Fontaine dimi...@2ndquadrant.fr wrote: Tom Lane t...@sss.pgh.pa.us writes: Not having read the patch, but ... the idea that was in the back of my mind was to have a generic AlterObjectNamespace function that would take parameters approximately like the following: Please find attached what I came up with, that's the set_schema patch version 6. Looks good overall, but: In AlterObjectNamespace(), you reference ACL_KIND_CONVERSION, which I suspect actually needs to be yet another parameter to that function. I've had the thought before that we have a deplorable number of different ways of referring to object types in the back end: ACL_KIND_*, OCLASS_*, OBJECT_*, and class IDs. We should maybe look at unifying some or all of those. getObjectDescriptionOids() needs a prototype somewhere. And probably most significantly, you need to add docs and regression tests. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] ALTER OBJECT any_name SET SCHEMA name
On Sat, Nov 20, 2010 at 11:22 PM, Robert Haas robertmh...@gmail.com wrote: On Thu, Nov 4, 2010 at 3:39 PM, Dimitri Fontaine dimi...@2ndquadrant.fr wrote: Tom Lane t...@sss.pgh.pa.us writes: Not having read the patch, but ... the idea that was in the back of my mind was to have a generic AlterObjectNamespace function that would take parameters approximately like the following: Please find attached what I came up with, that's the set_schema patch version 6. Looks good overall, but: In AlterObjectNamespace(), you reference ACL_KIND_CONVERSION, which I suspect actually needs to be yet another parameter to that function. I've had the thought before that we have a deplorable number of different ways of referring to object types in the back end: ACL_KIND_*, OCLASS_*, OBJECT_*, and class IDs. We should maybe look at unifying some or all of those. getObjectDescriptionOids() needs a prototype somewhere. And probably most significantly, you need to add docs and regression tests. Ah, nuts. I see now there's a v7. Never mind... -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] ALTER OBJECT any_name SET SCHEMA name
Alvaro Herrera alvhe...@commandprompt.com writes:
The has_privs_of_role() call has the wrong ACL_KIND argument in the
error report.
Ah yes, I missed the acl_kind. It's a parameter of the function in the
v7 patch, attached.
(Nitpick: don't use e.g. at the end of the phrase. It seems strange
to me.)
Fixed too. I also added documentation of the new forms of the ALTER
commands, as it seems we're heading to something which needs it :)
Regards,
--
Dimitri Fontaine
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
*** a/doc/src/sgml/ref/alter_conversion.sgml
--- b/doc/src/sgml/ref/alter_conversion.sgml
***
*** 23,28 PostgreSQL documentation
--- 23,29
synopsis
ALTER CONVERSION replaceablename/replaceable RENAME TO replaceablenew_name/replaceable
ALTER CONVERSION replaceablename/replaceable OWNER TO replaceablenew_owner/replaceable
+ ALTER CONVERSION replaceablename/replaceable SET SCHEMA replaceablenew_schema/replaceable
/synopsis
/refsynopsisdiv
***
*** 75,80 ALTER CONVERSION replaceablename/replaceable OWNER TO replaceablenew_owner
--- 76,90
/para
/listitem
/varlistentry
+
+varlistentry
+ termreplaceable class=parameternew_schema/replaceable/term
+ listitem
+ para
+ The new schema of the conversion.
+ /para
+ /listitem
+/varlistentry
/variablelist
/refsect1
*** a/doc/src/sgml/ref/alter_opclass.sgml
--- b/doc/src/sgml/ref/alter_opclass.sgml
***
*** 23,28 PostgreSQL documentation
--- 23,29
synopsis
ALTER OPERATOR CLASS replaceablename/replaceable USING replaceable class=parameterindex_method/replaceable RENAME TO replaceablenew_name/replaceable
ALTER OPERATOR CLASS replaceablename/replaceable USING replaceable class=parameterindex_method/replaceable OWNER TO replaceablenew_owner/replaceable
+ ALTER OPERATOR CLASS replaceablename/replaceable USING replaceable class=parameterindex_method/replaceable SET SCHEMA replaceablenew_schema/replaceable
/synopsis
/refsynopsisdiv
***
*** 85,90 ALTER OPERATOR CLASS replaceablename/replaceable USING replaceable class=p
--- 86,100
/para
/listitem
/varlistentry
+
+varlistentry
+ termreplaceable class=parameternew_schema/replaceable/term
+ listitem
+ para
+ The new schema for the operator class.
+ /para
+ /listitem
+/varlistentry
/variablelist
/refsect1
*** a/doc/src/sgml/ref/alter_operator.sgml
--- b/doc/src/sgml/ref/alter_operator.sgml
***
*** 22,27 PostgreSQL documentation
--- 22,28
refsynopsisdiv
synopsis
ALTER OPERATOR replaceablename/replaceable ( { replaceableleft_type/replaceable | NONE } , { replaceableright_type/replaceable | NONE } ) OWNER TO replaceablenew_owner/replaceable
+ ALTER OPERATOR replaceablename/replaceable ( { replaceableleft_type/replaceable | NONE } , { replaceableright_type/replaceable | NONE } ) SET SCHEMA replaceablenew_schema/replaceable
/synopsis
/refsynopsisdiv
***
*** 85,90 ALTER OPERATOR replaceablename/replaceable ( { replaceableleft_type/repla
--- 86,100
/para
/listitem
/varlistentry
+
+varlistentry
+ termreplaceable class=parameternew_schema/replaceable/term
+ listitem
+ para
+ The new schema of the operator.
+ /para
+ /listitem
+/varlistentry
/variablelist
/refsect1
*** a/doc/src/sgml/ref/alter_opfamily.sgml
--- b/doc/src/sgml/ref/alter_opfamily.sgml
***
*** 31,36 ALTER OPERATOR FAMILY replaceablename/replaceable USING replaceable class=
--- 31,37
} [, ... ]
ALTER OPERATOR FAMILY replaceablename/replaceable USING replaceable class=parameterindex_method/replaceable RENAME TO replaceablenew_name/replaceable
ALTER OPERATOR FAMILY replaceablename/replaceable USING replaceable class=parameterindex_method/replaceable OWNER TO replaceablenew_owner/replaceable
+ ALTER OPERATOR FAMILY replaceablename/replaceable USING replaceable class=parameterindex_method/replaceable SET SCHEMA replaceablenew_schema/replaceable
/synopsis
/refsynopsisdiv
***
*** 200,205 ALTER OPERATOR FAMILY replaceablename/replaceable USING replaceable class=
--- 201,215
/para
/listitem
/varlistentry
+
+varlistentry
+ termreplaceable class=parameternew_schema/replaceable/term
+ listitem
+ para
+ The new schema for the operator family.
+ /para
+ /listitem
+/varlistentry
/variablelist
para
*** a/doc/src/sgml/ref/alter_tsconfig.sgml
--- b/doc/src/sgml/ref/alter_tsconfig.sgml
***
*** 33,38 ALTER TEXT SEARCH CONFIGURATION replaceablename/replaceable
--- 33,39
DROP MAPPING [ IF EXISTS ] FOR replaceable class=parametertoken_type/replaceable [, ... ]
ALTER TEXT SEARCH CONFIGURATION replaceablename/replaceable RENAME TO
Re: [HACKERS] ALTER OBJECT any_name SET SCHEMA name
Alvaro Herrera alvhe...@commandprompt.com writes:
Hmm, seeing the amount of new includes in extension.c, I wonder if it'd
be better to move AlterExtensionNamespace to alter.c.
It was mainly missing includes cleanup. The guts of the function is now
so short I can inline it in this mail:
targetObjects = listDependentObjects(object);
for (i = 0; i targetObjects-numrefs; i++)
{
ObjectAddress *thisobj = targetObjects-refs + i;
elog(DEBUG1, SET SCHEMA on %u: %s,
thisobj-objectId, getObjectDescription(thisobj));
AlterObjectNamespace_internal(thisobj, nspOid);
}
So really, I don't think moving it to alter.c would do any better,
considering that you would then have this file include dependency
related function.
Please find attached v3 patch with #include cleanup.
--
Dimitri Fontaine
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
*** a/src/backend/catalog/pg_depend.c
--- b/src/backend/catalog/pg_depend.c
***
*** 20,25
--- 20,27
#include catalog/indexing.h
#include catalog/pg_constraint.h
#include catalog/pg_depend.h
+ #include catalog/pg_extension.h
+ #include catalog/pg_namespace.h
#include miscadmin.h
#include utils/fmgroids.h
#include utils/lsyscache.h
***
*** 643,645 get_index_constraint(Oid indexId)
--- 645,699
return constraintId;
}
+
+ /*
+ * get_extension_namespace
+ * Given the OID of an extension, return the OID of the schema it
+ * depends on, or InvalidOid when not found
+ */
+ Oid
+ get_extension_namespace(Oid extensionId)
+ {
+ Oid nspId = InvalidOid;
+ Relation depRel;
+ ScanKeyData key[3];
+ SysScanDesc scan;
+ HeapTuple tup;
+
+ /* Search the dependency table for the index */
+ depRel = heap_open(DependRelationId, AccessShareLock);
+
+ ScanKeyInit(key[0],
+ Anum_pg_depend_classid,
+ BTEqualStrategyNumber, F_OIDEQ,
+ ObjectIdGetDatum(ExtensionRelationId));
+ ScanKeyInit(key[1],
+ Anum_pg_depend_objid,
+ BTEqualStrategyNumber, F_OIDEQ,
+ ObjectIdGetDatum(extensionId));
+ ScanKeyInit(key[2],
+ Anum_pg_depend_objsubid,
+ BTEqualStrategyNumber, F_INT4EQ,
+ Int32GetDatum(0));
+
+ scan = systable_beginscan(depRel, DependDependerIndexId, true,
+ SnapshotNow, 3, key);
+
+ while (HeapTupleIsValid(tup = systable_getnext(scan)))
+ {
+ Form_pg_depend deprec = (Form_pg_depend) GETSTRUCT(tup);
+
+ if (deprec-refclassid == NamespaceRelationId
+ deprec-refobjsubid == 0
+ deprec-deptype == DEPENDENCY_NORMAL)
+ {
+ nspId = deprec-refobjid;
+ break;
+ }
+ }
+
+ systable_endscan(scan);
+ heap_close(depRel, AccessShareLock);
+
+ return nspId;
+ }
*** a/src/backend/commands/alter.c
--- b/src/backend/commands/alter.c
***
*** 22,27
--- 22,28
#include commands/conversioncmds.h
#include commands/dbcommands.h
#include commands/defrem.h
+ #include commands/extension.h
#include commands/proclang.h
#include commands/schemacmds.h
#include commands/tablecmds.h
***
*** 189,194 ExecAlterObjectSchemaStmt(AlterObjectSchemaStmt *stmt)
--- 190,199
AlterConversionNamespace(stmt-object, stmt-newschema);
break;
+ case OBJECT_EXTENSION:
+ AlterExtensionNamespace(stmt-object, stmt-newschema);
+ break;
+
case OBJECT_FUNCTION:
AlterFunctionNamespace(stmt-object, stmt-objarg, false,
stmt-newschema);
***
*** 334,339 AlterObjectNamespace(Relation rel, int cacheId,
--- 339,436
NamespaceRelationId, oldNspOid, nspOid);
}
+ /*
+ * Do the SET SCHEMA depending on the object class.
+ *
+ * We only consider objects that have a namespace and that can exist
+ * without depending on another object (like a table) which will
+ * have its dependencies follow the SET SCHEMA operation.
+ */
+ void
+ AlterObjectNamespace_internal(ObjectAddress *thisobj, Oid nspOid)
+ {
+ switch (getObjectClass(thisobj))
+ {
+ case OCLASS_CLASS:
+ {
+ Relation classRel;
+ Relation rel = relation_open(thisobj-objectId, RowExclusiveLock);
+
+ switch (rel-rd_rel-relkind)
+ {
+ case RELKIND_COMPOSITE_TYPE:
+ /*
+ * just skip the pg_class entry, we have a pg_type
+ * entry too
+ */
+ break;
+
+ default:
+ classRel = heap_open(RelationRelationId, RowExclusiveLock);
+ AlterRelationNamespaceInternal(classRel,
+ RelationGetRelid(rel),
+ RelationGetNamespace(rel),
+ nspOid,
+ true);
+ heap_close(classRel, RowExclusiveLock);
+ break;
+ }
+ relation_close(rel, RowExclusiveLock);
+ break;
+ }
+
+ case OCLASS_PROC:
+ AlterFunctionNamespace_oid(thisobj-objectId, nspOid);
+ break;
+
+ case OCLASS_TYPE:
+ {
+ /* don't allow direct alteration of array types, skip */
+ Oid elemOid =
Re: [HACKERS] ALTER OBJECT any_name SET SCHEMA name
Excerpts from Dimitri Fontaine's message of vie nov 05 06:49:34 -0300 2010: Alvaro Herrera alvhe...@commandprompt.com writes: Hmm, seeing the amount of new includes in extension.c, I wonder if it'd be better to move AlterExtensionNamespace to alter.c. It was mainly missing includes cleanup. The guts of the function is now so short I can inline it in this mail: Ah, good. Please find attached v3 patch with #include cleanup. Frankly, the get_extension_namespace bit still feels wrong to me. I would have the namespace be present in the pg_extension catalog, even if it's not part of the primary key. This would let you answer the question: what schema did I install this extension in? (and display it in \dx commands etc) If you don't know that, then the ability to change it to another schema looks incomplete. Since we're now saying that moving the extension to another schema is a first-class operation, I think the data should be represented more explicitely in the catalog rather than being derived from pg_depend contents. -- Álvaro Herrera alvhe...@commandprompt.com The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] ALTER OBJECT any_name SET SCHEMA name
Alvaro Herrera alvhe...@commandprompt.com writes: Frankly, the get_extension_namespace bit still feels wrong to me. I would have the namespace be present in the pg_extension catalog, even if it's not part of the primary key. This would let you answer the question: what schema did I install this extension in? (and display it in \dx commands etc) dim=# \dx List of extensions Schema |Name| Version | Description ++--+-- utils | lo | 9.1devel | managing Larg utils | unaccent | 9.1devel | text search d utils | adminpack | 9.1devel | Administrativ utils | moddatetime| 9.1devel | functions for utils | isn| 9.1devel | data types fo ... I've cut in there obviously, but you get the idea. If you don't know that, then the ability to change it to another schema looks incomplete. Since we're now saying that moving the extension to another schema is a first-class operation, I think the data should be represented more explicitely in the catalog rather than being derived from pg_depend contents. Well, I'm thinking that: - namespace columns in the catalogs are actually all for objects that live in a schema and extension do not - pg_depend is a good source here, as it is for get_constraint_index and some other functions - maybe the problem is that parts of this patch should go into the main extension's one, where there's already the with schema foo feature, rather than be introduced in the alter extension set schema patch? Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] ALTER OBJECT any_name SET SCHEMA name
Dimitri Fontaine dimi...@2ndquadrant.fr writes: Alvaro Herrera alvhe...@commandprompt.com writes: Frankly, the get_extension_namespace bit still feels wrong to me. I would have the namespace be present in the pg_extension catalog, even if it's not part of the primary key. Well, I'm thinking that: - namespace columns in the catalogs are actually all for objects that live in a schema and extension do not I'm with Alvaro on this. If we're going to have an ALTER EXTENSION SET SCHEMA operation, then extensions must have a well-defined schema property, and it would be good if that connection were explicitly represented in the catalogs. Digging stuff out of pg_depend sucks; we have to do it in some other cases where we didn't foresee the connection in advance, but we can see it well enough here. BTW, I'm not even 100% convinced that the schema shouldn't be part of the extension's name, if we're going to make it work like this. Is there a reason I shouldn't be able to have both public.myextension and testing.myextension? If we're constraining all the objects owned by the extension to live in a single schema, this seems perfectly feasible. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] ALTER OBJECT any_name SET SCHEMA name
Tom Lane t...@sss.pgh.pa.us writes: I'm with Alvaro on this. If we're going to have an ALTER EXTENSION SET SCHEMA operation, then extensions must have a well-defined schema property, and it would be good if that connection were explicitly represented in the catalogs. Digging stuff out of pg_depend sucks; we have to do it in some other cases where we didn't foresee the connection in advance, but we can see it well enough here. Ok. So an extension using more than one schema is out, right? Not that I can see a strong use case, just thinking out loud. Also, do we keep the current notation or change it, or add to it: CREATE EXTENSION foo WITH SCHEMA utils; CREATE EXTENSION utils.foo; I guess if you schema qualify the extension's name we could use that as the schema name, but remember that the control file name would then be different from the (qualified and given) extension's name. Surely we would not try to find the utils.foo.control file, right? The schema name is also used as a placeholder in the extension SQL script, so it is somewhat weird to have it in the extension's name. BTW, I'm not even 100% convinced that the schema shouldn't be part of the extension's name, if we're going to make it work like this. Is there a reason I shouldn't be able to have both public.myextension and testing.myextension? If we're constraining all the objects owned by the extension to live in a single schema, this seems perfectly feasible. Are you proposing that an extension object is schema qualified? Would we lower creating extension privileges to database owners, too, rather than only superusers? Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] ALTER OBJECT any_name SET SCHEMA name
Dimitri Fontaine dimi...@2ndquadrant.fr writes: Tom Lane t...@sss.pgh.pa.us writes: BTW, I'm not even 100% convinced that the schema shouldn't be part of the extension's name, if we're going to make it work like this. Is there a reason I shouldn't be able to have both public.myextension and testing.myextension? If we're constraining all the objects owned by the extension to live in a single schema, this seems perfectly feasible. Are you proposing that an extension object is schema qualified? Dunno, I'm just asking the question. If it isn't, why not? Here's another question: if an extension's objects live (mostly or entirely) in schema X, what happens if the possibly-unprivileged owner of schema X decides to drop it? If the extension itself is considered to live within the schema, then the whole extension goes away seems like a natural answer. If not, you've got some problems. Would we lower creating extension privileges to database owners, too, rather than only superusers? That seems like an orthogonal question. I can see people wanting both behaviors though. Maybe an extension's config file should specify the privs needed to install it? regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] ALTER OBJECT any_name SET SCHEMA name
Tom Lane t...@sss.pgh.pa.us writes: Are you proposing that an extension object is schema qualified? Dunno, I'm just asking the question. If it isn't, why not? Because extension are much like languages for stored procedure, on the utility side rather than on the query side. The only queries that uses language directly are utility statements, yet functions will depend on them and live in some schema. That's how I see extensions too, a new utility. Nothing I expect to be visible in queries. They don't need schema, they are database globals. The objects that are depending on the extension may or may not live in a schema. A single extension script could create more than one schema and have objects spread there. So either we restrict an extension's to live in a single schema and we have to forbid changing the schema of the objects in there on their own (DEPENDENCY_INTERNAL should help), or we add a very simple check at ALTER EXTENSION ... SET SCHEMA time to error out when the extension depends on more than one schema. I'd do the later, obviously. Here's another question: if an extension's objects live (mostly or entirely) in schema X, what happens if the possibly-unprivileged owner of schema X decides to drop it? If the extension itself is considered to live within the schema, then the whole extension goes away seems like a natural answer. If not, you've got some problems. Currently, creating an extension is superuser only. So the owner of those objects is a superuser. My understanding is that the drop schema will then fail without any more code. Would we lower creating extension privileges to database owners, too, rather than only superusers? That seems like an orthogonal question. I can see people wanting both behaviors though. Maybe an extension's config file should specify the privs needed to install it? Orthogonal indeed, but it popped in my mind reading the previous mail, and reading your previous question I guess you see why :) I'm not for offering extension's author to control this behavior. As the extension will more often than not come with a shared object lib, and as the goal is to install SQL objects that will NOT be part of pg_dump output, my feeling is that superuser only makes most sense. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] ALTER OBJECT any_name SET SCHEMA name
Dimitri Fontaine dimi...@2ndquadrant.fr writes: Tom Lane t...@sss.pgh.pa.us writes: Here's another question: if an extension's objects live (mostly or entirely) in schema X, what happens if the possibly-unprivileged owner of schema X decides to drop it? If the extension itself is considered to live within the schema, then the whole extension goes away seems like a natural answer. If not, you've got some problems. Currently, creating an extension is superuser only. So the owner of those objects is a superuser. My understanding is that the drop schema will then fail without any more code. You're mistaken, and this case definitely does need more thought. A schema owner is presumed to have the unconditional right to drop anything in his schema, whether he owns it or not. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] ALTER OBJECT any_name SET SCHEMA name
Tom Lane t...@sss.pgh.pa.us writes: You're mistaken, and this case definitely does need more thought. A schema owner is presumed to have the unconditional right to drop anything in his schema, whether he owns it or not. Here a paste of how it works with current code. dim=# create schema bob authorization bob; CREATE SCHEMA dim=# alter extension unaccent set schema bob; ALTER EXTENSION dim=# \c - bob You are now connected to database dim as user bob. dim= drop schema bob; ERROR: cannot drop schema bob because other objects depend on it DETAIL: extension unaccent depends on schema bob HINT: Use DROP ... CASCADE to drop the dependent objects too. dim= drop schema bob cascade; NOTICE: drop cascades to extension unaccent DROP SCHEMA dim= \c - dim You are now connected to database dim as user dim. dim=# select installed from pg_extensions where name = 'unaccent'; installed --- f (1 row) Isn't that enough for you? -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] ALTER OBJECT any_name SET SCHEMA name
Excerpts from Dimitri Fontaine's message of vie nov 05 16:58:00 -0300 2010: dim= drop schema bob cascade; NOTICE: drop cascades to extension unaccent DROP SCHEMA dim= \c - dim You are now connected to database dim as user dim. dim=# select installed from pg_extensions where name = 'unaccent'; installed --- f (1 row) Basically you're saying that the owner of the schema in which the extension is installed can drop the extension ... even though, according to your previous argument, the extension is not in said schema :-) -- Álvaro Herrera alvhe...@commandprompt.com The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] ALTER OBJECT any_name SET SCHEMA name
Alvaro Herrera alvhe...@commandprompt.com writes: Basically you're saying that the owner of the schema in which the extension is installed can drop the extension ... even though, according to your previous argument, the extension is not in said schema :-) Yeah it's a case of defining things. The extension is not in the schema, it depends on it. So if you drop schema cascade, that cascades to drop extension. I'm not saying that the only way to do it sanely is the one I propose, but I did consider some alternatives and I did not code the current patch only by accident :) Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] ALTER OBJECT any_name SET SCHEMA name
Alvaro Herrera alvhe...@commandprompt.com writes:
FWIW I think you should use getObjectDescription, as in the attached
patch. (Note the patch is incomplete and does not compile because only
one caller to CheckSetNamespace has been fixed).
I had to re-add the object name to the CheckSetNamespace prototype to
handle this particular check:
/* check for duplicate name (more friendly than unique-index failure) */
if (SearchSysCacheExists2(TYPENAMENSP,
CStringGetDatum(name),
ObjectIdGetDatum(nspOid)))
If you know how to get some struct attribute given a char * holding its
name, in C, I would adapt the patch and work on the refactoring asked
for by Tom.
Apart from that, it was just about adapting the call sites, which is
done in the attached set_schema.5.patch. Thanks!
Also attached, please find the complete version of ALTER EXTENSION ext
SET SCHEMA name; with support for all contrib extensions. That's the
example that allows to see the API (AlterFooNamespace_oid and _internal
functions) in action: that should help devising the best refactoring.
Regards,
--
Dimitri Fontaine
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
*** a/src/backend/catalog/dependency.c
--- b/src/backend/catalog/dependency.c
***
*** 2706,2711 getObjectDescription(const ObjectAddress *object)
--- 2706,2726
}
/*
+ * getObjectDescriptionOids: as above, except the object is specified by Oids
+ */
+ char *
+ getObjectDescriptionOids(Oid classid, Oid objid)
+ {
+ ObjectAddress address;
+
+ address.classId = classid;
+ address.objectId = objid;
+ address.objectSubId = 0;
+
+ return getObjectDescription(address);
+ }
+
+ /*
* subroutine for getObjectDescription: describe a relation
*/
static void
*** a/src/backend/catalog/namespace.c
--- b/src/backend/catalog/namespace.c
***
*** 2340,2345 LookupCreationNamespace(const char *nspname)
--- 2340,2383
}
/*
+ * Check new namespace validity in ALTER OBJECT ... SET SCHEMA ... and
+ * ereport(ERROR, ...) in case of any problem.
+ */
+ void
+ CheckSetNamespace(Oid oldNspOid, Oid nspOid, Oid classid, Oid objid,
+ const char *name)
+ {
+ if (oldNspOid == nspOid)
+ ereport(ERROR,
+ (errcode(ERRCODE_DUPLICATE_OBJECT),
+ errmsg(%s already exists in schema \%s\,
+ getObjectDescriptionOids(classid, objid),
+ get_namespace_name(nspOid;
+
+ /* disallow renaming into or out of temp schemas */
+ if (isAnyTempNamespace(nspOid) || isAnyTempNamespace(oldNspOid))
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg(cannot move objects into or out of temporary schemas)));
+
+ /* same for TOAST schema */
+ if (nspOid == PG_TOAST_NAMESPACE || oldNspOid == PG_TOAST_NAMESPACE)
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg(cannot move objects into or out of TOAST schema)));
+
+ /* check for duplicate name (more friendly than unique-index failure) */
+ if (SearchSysCacheExists2(TYPENAMENSP,
+ CStringGetDatum(name),
+ ObjectIdGetDatum(nspOid)))
+ ereport(ERROR,
+ (errcode(ERRCODE_DUPLICATE_OBJECT),
+ errmsg(%s already exists in schema \%s\,
+ getObjectDescriptionOids(classid, objid),
+ get_namespace_name(nspOid;
+ }
+
+ /*
* QualifiedNameGetCreationNamespace
* Given a possibly-qualified name for an object (in List-of-Values
* format), determine what namespace the object should be created in.
*** a/src/backend/commands/alter.c
--- b/src/backend/commands/alter.c
***
*** 182,192 ExecAlterObjectSchemaStmt(AlterObjectSchemaStmt *stmt)
--- 182,208
stmt-newschema);
break;
+ case OBJECT_CONVERSION:
+ AlterConversionNamespace(stmt-object, stmt-newschema);
+ break;
+
case OBJECT_FUNCTION:
AlterFunctionNamespace(stmt-object, stmt-objarg, false,
stmt-newschema);
break;
+ case OBJECT_OPERATOR:
+ AlterOperatorNamespace(stmt-object, stmt-objarg, stmt-newschema);
+ break;
+
+ case OBJECT_OPCLASS:
+ AlterOpClassNamespace(stmt-object, stmt-objarg, stmt-newschema);
+ break;
+
+ case OBJECT_OPFAMILY:
+ AlterOpFamilyNamespace(stmt-object, stmt-objarg, stmt-newschema);
+ break;
+
case OBJECT_SEQUENCE:
case OBJECT_TABLE:
case OBJECT_VIEW:
***
*** 195,200 ExecAlterObjectSchemaStmt(AlterObjectSchemaStmt *stmt)
--- 211,232
stmt-objectType, AccessExclusiveLock);
break;
+ case OBJECT_TSPARSER:
+ AlterTSParserNamespace(stmt-object, stmt-newschema);
+ break;
+
+ case OBJECT_TSDICTIONARY:
+ AlterTSDictionaryNamespace(stmt-object, stmt-newschema);
+ break;
+
+ case OBJECT_TSTEMPLATE:
+ AlterTSTemplateNamespace(stmt-object, stmt-newschema);
+ break;
+
+ case
Re: [HACKERS] ALTER OBJECT any_name SET SCHEMA name
Excerpts from Dimitri Fontaine's message of jue nov 04 11:06:48 -0300 2010: Alvaro Herrera alvhe...@commandprompt.com writes: FWIW I think you should use getObjectDescription, as in the attached patch. (Note the patch is incomplete and does not compile because only one caller to CheckSetNamespace has been fixed). I had to re-add the object name to the CheckSetNamespace prototype to handle this particular check: /* check for duplicate name (more friendly than unique-index failure) */ if (SearchSysCacheExists2(TYPENAMENSP, CStringGetDatum(name), ObjectIdGetDatum(nspOid))) Hmm, this check is wrong anyway, because you're looking in the pg_type syscache for objects from an arbitrary catalog. That needs to be fixed somehow, but perhaps it needs to be handled by the callers, not in this routine. Otherwise you're going to need to pass the syscache ID, as well as Datums identifying the object, and the number of Datums. -- Álvaro Herrera alvhe...@commandprompt.com The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] ALTER OBJECT any_name SET SCHEMA name
Alvaro Herrera alvhe...@commandprompt.com writes:
/* check for duplicate name (more friendly than unique-index failure) */
if (SearchSysCacheExists2(TYPENAMENSP,
CStringGetDatum(name),
ObjectIdGetDatum(nspOid)))
Hmm, this check is wrong anyway, because you're looking in the pg_type
syscache for objects from an arbitrary catalog. That needs to be fixed
somehow, but perhaps it needs to be handled by the callers, not in this
routine. Otherwise you're going to need to pass the syscache ID, as
well as Datums identifying the object, and the number of Datums.
How embarrassing. I wonder why this works, too:
dim=# alter operator utils.@(utils.ltree, utils.ltree) set schema public;
ALTER OPERATOR
dim=# alter operator @(utils.ltree, utils.ltree) set schema utils;
ALTER OPERATOR
We have :
static void
AlterOperatorNamespace_internal(Relation rel, Oid operOid, Oid nspOid)
{
...
CheckSetNamespace(oldNspOid, nspOid,
OperatorRelationId, operOid,
NameStr(oprForm-oprname));
Well, I'll go fix as you say, putting the check back into the
callers. That won't help a bit with the code duplication feeling we have
when reading the patch, though. Any idea on this front?
Regards,
--
Dimitri Fontaine
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] ALTER OBJECT any_name SET SCHEMA name
Excerpts from Dimitri Fontaine's message of jue nov 04 11:06:48 -0300 2010: Also attached, please find the complete version of ALTER EXTENSION ext SET SCHEMA name; with support for all contrib extensions. That's the example that allows to see the API (AlterFooNamespace_oid and _internal functions) in action: that should help devising the best refactoring. Three comments, 1. wouldn't it make more sense to save the extension namespace in the extension catalog? 2. I think the guts of AlterExtensionNamespace (the large switch block) should be elsewhere, probably in alter.c 3. Not this patch, but I think using extension as a global variable name is a bad idea. -- Álvaro Herrera alvhe...@commandprompt.com The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] ALTER OBJECT any_name SET SCHEMA name
Dimitri Fontaine dimi...@2ndquadrant.fr writes: Well, I'll go fix as you say, putting the check back into the callers. That won't help a bit with the code duplication feeling we have when reading the patch, though. Any idea on this front? Not having read the patch, but ... the idea that was in the back of my mind was to have a generic AlterObjectNamespace function that would take parameters approximately like the following: OID of catalog containing object Column number of catalog's namespace column (Anum_xxx constant) OID of intended new namespace You could do a generic heap_open() on the catalog using the OID, and then use heap_modify_tuple to apply the namespace column update. It might be nice to include the object already exists check here too, which could probably be done if in addition the column number of the name column were passed in. Permission checks too, if the owner column number were passed in. Etc. Obviously this doesn't work for tables, but they're sufficiently complex beasts that it's not unusual for them to need a different code path. Doesn't help for functions/operators either, since their collision check isn't just name and namespace. But that's OK IMO. I'd be happy if we could unify the code paths for objects that have a single catalog entry to update and a simple name/namespace collision check to make. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] ALTER OBJECT any_name SET SCHEMA name
Alvaro Herrera alvhe...@commandprompt.com writes: 1. wouldn't it make more sense to save the extension namespace in the extension catalog? I don't think so, because the extension itself is not schema qualified. What lives in the namespace the extension depends on is not the extension itself, but its objects. 2. I think the guts of AlterExtensionNamespace (the large switch block) should be elsewhere, probably in alter.c Makes sense, will move there. 3. Not this patch, but I think using extension as a global variable name is a bad idea. What about create_extension_extension instead? I'm not thinking of something better, bikeshedding is opened. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] ALTER OBJECT any_name SET SCHEMA name
On Thu, Nov 4, 2010 at 7:52 AM, Dimitri Fontaine dimi...@2ndquadrant.fr wrote: What about create_extension_extension instead? I'm not thinking of something better, bikeshedding is opened. That doesn't seem very clear... I'm always suspicious of names that use the same word twice, and in this case I have no idea what this variable would supposedly refer to. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] ALTER OBJECT any_name SET SCHEMA name
Tom Lane t...@sss.pgh.pa.us writes: Not having read the patch, but ... the idea that was in the back of my mind was to have a generic AlterObjectNamespace function that would take parameters approximately like the following: OID of catalog containing object Column number of catalog's namespace column (Anum_xxx constant) OID of intended new namespace Ah, the trick is to use the Anum_xxx, of course. I couldn't get rid of thinking how to dynamically access by name... will have a try at that, thanks for the idea. You could do a generic heap_open() on the catalog using the OID, and then use heap_modify_tuple to apply the namespace column update. Thanks for pointing me to the right APIs: finding them is where the time is mostly spent as far as I'm concerned. It might be nice to include the object already exists check here too, which could probably be done if in addition the column number of the name column were passed in. Permission checks too, if the owner column number were passed in. Etc. Well it seems that depending on the object, sometime only superusers are allowed to edit things, and sometime the owner too. Will add a boolean superuser_only in the prototype. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] ALTER OBJECT any_name SET SCHEMA name
Robert Haas robertmh...@gmail.com writes: On Thu, Nov 4, 2010 at 7:52 AM, Dimitri Fontaine dimi...@2ndquadrant.fr wrote: What about create_extension_extension instead? I'm not thinking of something better, bikeshedding is opened. That doesn't seem very clear... I'm always suspicious of names that use the same word twice, and in this case I have no idea what this variable would supposedly refer to. The ObjectAddress of the extension currently being installed by the CREATE EXTENSION command we're in (executing the script). The variable create_extension is already a boolean only set to true if in the code path. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] ALTER OBJECT any_name SET SCHEMA name
On Thu, Nov 4, 2010 at 8:18 AM, Dimitri Fontaine dimi...@2ndquadrant.fr wrote: Robert Haas robertmh...@gmail.com writes: On Thu, Nov 4, 2010 at 7:52 AM, Dimitri Fontaine dimi...@2ndquadrant.fr wrote: What about create_extension_extension instead? I'm not thinking of something better, bikeshedding is opened. That doesn't seem very clear... I'm always suspicious of names that use the same word twice, and in this case I have no idea what this variable would supposedly refer to. The ObjectAddress of the extension currently being installed by the CREATE EXTENSION command we're in (executing the script). The variable create_extension is already a boolean only set to true if in the code path. How about calling it CurrentExtensionObjectAddress or something like that? And maybe you don't need a boolean: if (OidIsValid(CurrentExtensionObjectAddress.objid)) ... -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] ALTER OBJECT any_name SET SCHEMA name
Excerpts from Dimitri Fontaine's message of jue nov 04 11:37:37 -0300 2010: Alvaro Herrera alvhe...@commandprompt.com writes: /* check for duplicate name (more friendly than unique-index failure) */ if (SearchSysCacheExists2(TYPENAMENSP, CStringGetDatum(name), ObjectIdGetDatum(nspOid))) Hmm, this check is wrong anyway, because you're looking in the pg_type syscache for objects from an arbitrary catalog. That needs to be fixed somehow, but perhaps it needs to be handled by the callers, not in this routine. Otherwise you're going to need to pass the syscache ID, as well as Datums identifying the object, and the number of Datums. How embarrassing. I wonder why this works, too: dim=# alter operator utils.@(utils.ltree, utils.ltree) set schema public; ALTER OPERATOR Well, I guess the operator doesn't exist in the pg_type syscache, so it doesn't ereport(ERROR). Well, I'll go fix as you say, putting the check back into the callers. That won't help a bit with the code duplication feeling we have when reading the patch, though. Any idea on this front? I'm not really sure about the code duplication bits. There are plenty of things you cannot factor into common routines because of the need to handle different GETSTRUCT args, different number of syscache arguments, etc. The ALTER OWNER implementation is already duplicated for each database object. Your patch is already reducing duplication by moving some common checks into namespace.c. If there are more things that you could do by specifying a syscache ID and datums to be passed to it, perhaps it would make sense to use them as parameters to a function that does the whole bunch together. This probably doesn't belong into namespace.c though. -- Álvaro Herrera alvhe...@commandprompt.com The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] ALTER OBJECT any_name SET SCHEMA name
Excerpts from Dimitri Fontaine's message of jue nov 04 11:52:53 -0300 2010: Alvaro Herrera alvhe...@commandprompt.com writes: 3. Not this patch, but I think using extension as a global variable name is a bad idea. What about create_extension_extension instead? I'm not thinking of something better, bikeshedding is opened. CreateExtensionAddress? (I like CamelCase for this because it makes these variables stand out more against local ones, named in stuffed_lower_case). -- Álvaro Herrera alvhe...@commandprompt.com The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] ALTER OBJECT any_name SET SCHEMA name
Tom Lane t...@sss.pgh.pa.us writes:
Not having read the patch, but ... the idea that was in the back of
my mind was to have a generic AlterObjectNamespace function that
would take parameters approximately like the following:
Please find attached what I came up with, that's the set_schema patch
version 6.
Regards,
--
Dimitri Fontaine
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
*** a/src/backend/catalog/dependency.c
--- b/src/backend/catalog/dependency.c
***
*** 2706,2711 getObjectDescription(const ObjectAddress *object)
--- 2706,2726
}
/*
+ * getObjectDescriptionOids: as above, except the object is specified by Oids
+ */
+ char *
+ getObjectDescriptionOids(Oid classid, Oid objid)
+ {
+ ObjectAddress address;
+
+ address.classId = classid;
+ address.objectId = objid;
+ address.objectSubId = 0;
+
+ return getObjectDescription(address);
+ }
+
+ /*
* subroutine for getObjectDescription: describe a relation
*/
static void
*** a/src/backend/catalog/namespace.c
--- b/src/backend/catalog/namespace.c
***
*** 2340,2345 LookupCreationNamespace(const char *nspname)
--- 2340,2372
}
/*
+ * Check new namespace validity in ALTER OBJECT ... SET SCHEMA ... and
+ * ereport(ERROR, ...) in case of any problem.
+ */
+ void
+ CheckSetNamespace(Oid oldNspOid, Oid nspOid, Oid classid, Oid objid)
+ {
+ if (oldNspOid == nspOid)
+ ereport(ERROR,
+ (errcode(ERRCODE_DUPLICATE_OBJECT),
+ errmsg(%s already exists in schema \%s\,
+ getObjectDescriptionOids(classid, objid),
+ get_namespace_name(nspOid;
+
+ /* disallow renaming into or out of temp schemas */
+ if (isAnyTempNamespace(nspOid) || isAnyTempNamespace(oldNspOid))
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg(cannot move objects into or out of temporary schemas)));
+
+ /* same for TOAST schema */
+ if (nspOid == PG_TOAST_NAMESPACE || oldNspOid == PG_TOAST_NAMESPACE)
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg(cannot move objects into or out of TOAST schema)));
+ }
+
+ /*
* QualifiedNameGetCreationNamespace
* Given a possibly-qualified name for an object (in List-of-Values
* format), determine what namespace the object should be created in.
*** a/src/backend/commands/alter.c
--- b/src/backend/commands/alter.c
***
*** 14,21
--- 14,23
*/
#include postgres.h
+ #include catalog/indexing.h
#include catalog/namespace.h
#include catalog/pg_largeobject.h
+ #include catalog/pg_namespace.h
#include commands/alter.h
#include commands/conversioncmds.h
#include commands/dbcommands.h
***
*** 33,38
--- 35,41
#include utils/acl.h
#include utils/builtins.h
#include utils/lsyscache.h
+ #include utils/syscache.h
/*
***
*** 182,192 ExecAlterObjectSchemaStmt(AlterObjectSchemaStmt *stmt)
--- 185,211
stmt-newschema);
break;
+ case OBJECT_CONVERSION:
+ AlterConversionNamespace(stmt-object, stmt-newschema);
+ break;
+
case OBJECT_FUNCTION:
AlterFunctionNamespace(stmt-object, stmt-objarg, false,
stmt-newschema);
break;
+ case OBJECT_OPERATOR:
+ AlterOperatorNamespace(stmt-object, stmt-objarg, stmt-newschema);
+ break;
+
+ case OBJECT_OPCLASS:
+ AlterOpClassNamespace(stmt-object, stmt-objarg, stmt-newschema);
+ break;
+
+ case OBJECT_OPFAMILY:
+ AlterOpFamilyNamespace(stmt-object, stmt-objarg, stmt-newschema);
+ break;
+
case OBJECT_SEQUENCE:
case OBJECT_TABLE:
case OBJECT_VIEW:
***
*** 195,200 ExecAlterObjectSchemaStmt(AlterObjectSchemaStmt *stmt)
--- 214,235
stmt-objectType, AccessExclusiveLock);
break;
+ case OBJECT_TSPARSER:
+ AlterTSParserNamespace(stmt-object, stmt-newschema);
+ break;
+
+ case OBJECT_TSDICTIONARY:
+ AlterTSDictionaryNamespace(stmt-object, stmt-newschema);
+ break;
+
+ case OBJECT_TSTEMPLATE:
+ AlterTSTemplateNamespace(stmt-object, stmt-newschema);
+ break;
+
+ case OBJECT_TSCONFIGURATION:
+ AlterTSConfigurationNamespace(stmt-object, stmt-newschema);
+ break;
+
case OBJECT_TYPE:
case OBJECT_DOMAIN:
AlterTypeNamespace(stmt-object, stmt-newschema);
***
*** 207,212 ExecAlterObjectSchemaStmt(AlterObjectSchemaStmt *stmt)
--- 242,341
}
/*
+ * Generic function to change the namespace of a given object, for simple
+ * cases (won't work for tables or functions, e.g.)
+ *
+ * The AlterFooNamespace() calls just above will call a function whose job
+ * is to lookup the arguments for the generic function here.
+ *
+ * Relation must already by open, it's the responsibility of the caller to
+ * close it.
+ */
+ void
+ AlterObjectNamespace(Relation rel, int cacheId,
+ Oid classId, Oid objid, Oid nspOid,
+ int Anum_name, int Anum_namespace, int
Re: [HACKERS] ALTER OBJECT any_name SET SCHEMA name
Alvaro Herrera alvhe...@commandprompt.com writes:
2. I think the guts of AlterExtensionNamespace (the large switch block)
should be elsewhere, probably in alter.c
That's implemented in the alter_extension patch v2, and that's much
better, thanks for your continued input. Please note that it depends on
the new set_schema.6.patch.
(The problem with smaller patches is indeed the dependencies)
Regards,
--
Dimitri Fontaine
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
*** a/src/backend/catalog/pg_depend.c
--- b/src/backend/catalog/pg_depend.c
***
*** 20,25
--- 20,27
#include catalog/indexing.h
#include catalog/pg_constraint.h
#include catalog/pg_depend.h
+ #include catalog/pg_extension.h
+ #include catalog/pg_namespace.h
#include miscadmin.h
#include utils/fmgroids.h
#include utils/lsyscache.h
***
*** 643,645 get_index_constraint(Oid indexId)
--- 645,699
return constraintId;
}
+
+ /*
+ * get_extension_namespace
+ * Given the OID of an extension, return the OID of the schema it
+ * depends on, or InvalidOid when not found
+ */
+ Oid
+ get_extension_namespace(Oid extensionId)
+ {
+ Oid nspId = InvalidOid;
+ Relation depRel;
+ ScanKeyData key[3];
+ SysScanDesc scan;
+ HeapTuple tup;
+
+ /* Search the dependency table for the index */
+ depRel = heap_open(DependRelationId, AccessShareLock);
+
+ ScanKeyInit(key[0],
+ Anum_pg_depend_classid,
+ BTEqualStrategyNumber, F_OIDEQ,
+ ObjectIdGetDatum(ExtensionRelationId));
+ ScanKeyInit(key[1],
+ Anum_pg_depend_objid,
+ BTEqualStrategyNumber, F_OIDEQ,
+ ObjectIdGetDatum(extensionId));
+ ScanKeyInit(key[2],
+ Anum_pg_depend_objsubid,
+ BTEqualStrategyNumber, F_INT4EQ,
+ Int32GetDatum(0));
+
+ scan = systable_beginscan(depRel, DependDependerIndexId, true,
+ SnapshotNow, 3, key);
+
+ while (HeapTupleIsValid(tup = systable_getnext(scan)))
+ {
+ Form_pg_depend deprec = (Form_pg_depend) GETSTRUCT(tup);
+
+ if (deprec-refclassid == NamespaceRelationId
+ deprec-refobjsubid == 0
+ deprec-deptype == DEPENDENCY_NORMAL)
+ {
+ nspId = deprec-refobjid;
+ break;
+ }
+ }
+
+ systable_endscan(scan);
+ heap_close(depRel, AccessShareLock);
+
+ return nspId;
+ }
*** a/src/backend/catalog/pg_namespace.c
--- b/src/backend/catalog/pg_namespace.c
***
*** 76,80 NamespaceCreate(const char *nspName, Oid ownerId)
--- 76,92
/* Record dependency on owner */
recordDependencyOnOwner(NamespaceRelationId, nspoid, ownerId);
+ /* Record dependency on extension, if we're in a CREATE EXTENSION */
+ if (create_extension)
+ {
+ ObjectAddress myself;
+
+ myself.classId = NamespaceRelationId;
+ myself.objectId = nspoid;
+ myself.objectSubId = 0;
+
+ recordDependencyOn(myself, extension, DEPENDENCY_INTERNAL);
+ }
+
return nspoid;
}
*** a/src/backend/commands/alter.c
--- b/src/backend/commands/alter.c
***
*** 22,27
--- 22,28
#include commands/conversioncmds.h
#include commands/dbcommands.h
#include commands/defrem.h
+ #include commands/extension.h
#include commands/proclang.h
#include commands/schemacmds.h
#include commands/tablecmds.h
***
*** 189,194 ExecAlterObjectSchemaStmt(AlterObjectSchemaStmt *stmt)
--- 190,199
AlterConversionNamespace(stmt-object, stmt-newschema);
break;
+ case OBJECT_EXTENSION:
+ AlterExtensionNamespace(stmt-object, stmt-newschema);
+ break;
+
case OBJECT_FUNCTION:
AlterFunctionNamespace(stmt-object, stmt-objarg, false,
stmt-newschema);
***
*** 334,339 AlterObjectNamespace(Relation rel, int cacheId,
--- 339,436
NamespaceRelationId, oldNspOid, nspOid);
}
+ /*
+ * Do the SET SCHEMA depending on the object class.
+ *
+ * We only consider objects that have a namespace and that can exist
+ * without depending on another object (like a table) which will
+ * have its dependencies follow the SET SCHEMA operation.
+ */
+ void
+ AlterObjectNamespace_internal(ObjectAddress *thisobj, Oid nspOid)
+ {
+ switch (getObjectClass(thisobj))
+ {
+ case OCLASS_CLASS:
+ {
+ Relation classRel;
+ Relation rel = relation_open(thisobj-objectId, RowExclusiveLock);
+
+ switch (rel-rd_rel-relkind)
+ {
+ case RELKIND_COMPOSITE_TYPE:
+ /*
+ * just skip the pg_class entry, we have a pg_type
+ * entry too
+ */
+ break;
+
+ default:
+ classRel = heap_open(RelationRelationId, RowExclusiveLock);
+ AlterRelationNamespaceInternal(classRel,
+ RelationGetRelid(rel),
+ RelationGetNamespace(rel),
+ nspOid,
+ true);
+ heap_close(classRel, RowExclusiveLock);
+ break;
+ }
+ relation_close(rel, RowExclusiveLock);
+ break;
+ }
+
+ case OCLASS_PROC:
+ AlterFunctionNamespace_oid(thisobj-objectId, nspOid);
+
Re: [HACKERS] ALTER OBJECT any_name SET SCHEMA name
Excerpts from Dimitri Fontaine's message of jue nov 04 16:39:31 -0300 2010: Tom Lane t...@sss.pgh.pa.us writes: Not having read the patch, but ... the idea that was in the back of my mind was to have a generic AlterObjectNamespace function that would take parameters approximately like the following: Please find attached what I came up with, that's the set_schema patch version 6. Neat. The has_privs_of_role() call has the wrong ACL_KIND argument in the error report. (Nitpick: don't use e.g. at the end of the phrase. It seems strange to me.) -- Álvaro Herrera alvhe...@commandprompt.com The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] ALTER OBJECT any_name SET SCHEMA name
Excerpts from Dimitri Fontaine's message of jue nov 04 16:42:53 -0300 2010: Alvaro Herrera alvhe...@commandprompt.com writes: 2. I think the guts of AlterExtensionNamespace (the large switch block) should be elsewhere, probably in alter.c That's implemented in the alter_extension patch v2, and that's much better, thanks for your continued input. Please note that it depends on the new set_schema.6.patch. Hmm, seeing the amount of new includes in extension.c, I wonder if it'd be better to move AlterExtensionNamespace to alter.c. (The problem with smaller patches is indeed the dependencies) You can't please them all, I guess ... -- Álvaro Herrera alvhe...@commandprompt.com The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Fwd: Re: [HACKERS] ALTER OBJECT any_name SET SCHEMA name
Sorry, I messed up and emailed this only to Dimitri. --- Begin forwarded message from Alvaro Herrera --- From: Alvaro Herrera alvhe...@commandprompt.com To: Dimitri Fontaine dimi...@2ndquadrant.fr Date: Wed, 03 Nov 2010 14:13:58 -0300 Subject: Re: [HACKERS] ALTER OBJECT any_name SET SCHEMA name Excerpts from Dimitri Fontaine's message of mié nov 03 13:10:12 -0300 2010: Then, I think the ALTER EXTENSION foo SET SCHEMA name still has a use case, so I've prepared a simple patch to show the API usage before we get to refactor it all following Tom's asking. So there's a initial patch to see that in action. I had to rework AlterFunctionNamespace() API so that I can call it from elsewhere in the backend where I have Oids, so here's an updated set_schema.4.patch. We will have to extend the APIs for relations and types the same way, but it's already possible to test the patch with some extensions this way. Before I noticed that you were going to rework this patch completely, I cleaned it up a bit; attached (probably just for your amusement) I was looking at this patch 'cause someone asked for the ability to do CREATE TEMP OBJECT for objects that currently don't support it. I thought this might be related. --- End forwarded message --- -- Álvaro Herrera alvhe...@alvh.no-ip.org 0010-Clean-up-ALTER-OBJECT-SET-SCHEMA-patch.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] ALTER OBJECT any_name SET SCHEMA name
Excerpts from Dimitri Fontaine's message of mié nov 03 13:10:12 -0300 2010: Then, I think the ALTER EXTENSION foo SET SCHEMA name still has a use case, so I've prepared a simple patch to show the API usage before we get to refactor it all following Tom's asking. So there's a initial patch to see that in action. FWIW I think you should use getObjectDescription, as in the attached patch. (Note the patch is incomplete and does not compile because only one caller to CheckSetNamespace has been fixed). -- Álvaro Herrera alvhe...@commandprompt.com The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support 0001-Use-getObjectDescription.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] ALTER OBJECT any_name SET SCHEMA name
Alvaro Herrera alvhe...@commandprompt.com writes: FWIW I think you should use getObjectDescription, as in the attached patch. (Note the patch is incomplete and does not compile because only one caller to CheckSetNamespace has been fixed). That a very good idea, will apply (cherry-pick -n) and finish it tomorrow, thanks! Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] ALTER OBJECT any_name SET SCHEMA name
Hi, Thanks for your review! Tom Lane t...@sss.pgh.pa.us writes: Dimitri Fontaine dimi...@2ndquadrant.fr writes: ALTER EXTENSION ext SET SCHEMA name; ALTER EXTENSION ext SET SCHEMA foo TO bar; I think that would end the open debate about search_path vs extension, because each user would be able to relocate his local extensions easily, wherever the main script has installed them (often enough, public). I'm not sure whether that really fixes anything, or just provides people with a larger-caliber foot-gun. See for example recent complaints about citext misbehaving if it's not in the public schema (or more generally, any schema not in the search path). I think we'd need to think a bit harder about the behavior of objects that aren't in the search path before creating a facility like this, since it seems to be tantamount to promising that extensions won't break when pushed around to different schemas. Well AFAIK we have only two choices here. Either we impose the schema where to find any extension's object or we offer more facilities for users to support their choices. In the former case, we have problems with upgrades and hosting several versions of the same extension at the same time (think PostGIS 1.4 and 1.5 e.g.). Not being able to choose a schema where to host an extension's objects only makes sense when the user can't set the search_path, which is what we do with pg_catalog. In the latter case, following your example, all it would take the user to fix his setup would be a SET SCHEMA command on the citext extension. The only goal of this proposal is not to have to rethink object visibility, by offering the tools users need to manage the situation. All in all, I don't think it'll be ever possible to both support search_path flexibility and extension robustness when search_path changes. What we could do is offer extension's author a way to find their operator or functions or whatever dynamically in SQL, so that writing robust pure-SQL functions is possible. What comes to mind now would be a way to call a function/operator/... by OID at the SQL level. Not pretty but with the pg_extension_objects() SRF and maybe a layer atop that, that would do the trick. Brain dumping still. I'm also a bit less than enthused about the implementation approach. If we're going to have a policy that every object type must support ALTER SET SCHEMA, I think it might be time to refactor, rather than copying-and-pasting similar boilerplate code for every one. I've begun the effort with the CheckSetNamespace() function, about half of the rest of the code could receive some abstraction too (syscache searches, tuple access and editing, dependency changing, but ACL checks looks harder. How much easier would it be with defmacro... I'll have a try at it soon, happy to hear what you have in mind here before I start, so that I follow your guidance. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] ALTER OBJECT any_name SET SCHEMA name
--On 30. Oktober 2010 18:59:30 -0400 Tom Lane t...@sss.pgh.pa.us wrote: I'm not sure whether that really fixes anything, or just provides people with a larger-caliber foot-gun. See for example recent complaints about citext misbehaving if it's not in the public schema (or more generally, any schema not in the search path). I think we'd need to think a bit harder about the behavior of objects that aren't in the search path before creating a facility like this, since it seems to be tantamount to promising that extensions won't break when pushed around to different schemas. I'm also a bit less than enthused about the implementation approach. If we're going to have a policy that every object type must support ALTER SET SCHEMA, I think it might be time to refactor, rather than copying-and-pasting similar boilerplate code for every one. This reminds me of a small discussion we had some years ago when i targeted this for the sake of completeness of ASS (see http://archives.postgresql.org/pgsql-patches/2006-06/msg00021.php). I didn't follow the previous discussions about EXTENSION very closely, but to amend the idea made in the mentioned thread, couldn't we just invent a facility to move classes of objects belonging to an extension, only? -- Thanks Bernd -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] ALTER OBJECT any_name SET SCHEMA name
Bernd Helmle maili...@oopsware.de writes: This reminds me of a small discussion we had some years ago when i targeted this for the sake of completeness of ASS (see http://archives.postgresql.org/pgsql-patches/2006-06/msg00021.php). Discovered it, thanks for the pointer. I didn't follow the previous discussions about EXTENSION very closely, but to amend the idea made in the mentioned thread, couldn't we just invent a facility to move classes of objects belonging to an extension, only? Well under the hood you still need about the same code. In the current patch version I have, that would mean forgetting about a ~20 lines long function and keeping the other two, 10+30 lines about. Now if someone has a idea on how to have a single routine that knows how to deal with any object type and change its namespace, I'm all ears, and willing to have a try at it. Coding in C is for me a twice a year exercise, so whatever I come up with will be... improvable, I fear. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] ALTER OBJECT any_name SET SCHEMA name
On 31.10.2010 14:46, Dimitri Fontaine wrote: What we could do is offer extension's author a way to find their operator or functions or whatever dynamically in SQL, so that writing robust pure-SQL functions is possible. What comes to mind now would be a way to call a function/operator/... by OID at the SQL level. Not pretty but with the pg_extension_objects() SRF and maybe a layer atop that, that would do the trick. Brain dumping still. How about something like: CREATE EXTENSION myextension ... SCHEMA myschema; And in the .sql file in the extension you could have special markers for the schema, something like: CREATE FUNCTION otherfunction() AS ...; CREATE FUNCTION foo() AS $$ SELECT 'foo', @extsch...@.otherfunction() $$; @extschema@ would be searchreplaced at CREATE EXTENSION time with the schema specified by the user. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] ALTER OBJECT any_name SET SCHEMA name
Heikki Linnakangas heikki.linnakan...@enterprisedb.com writes: CREATE EXTENSION myextension ... SCHEMA myschema; And in the .sql file in the extension you could have special markers for the schema, something like: That's exactly the road I want to avoid, because of the script parsing issues. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] ALTER OBJECT any_name SET SCHEMA name
Sorry for the interruption, our program now continues... Dimitri Fontaine dimi...@2ndquadrant.fr writes: That's exactly the road I want to avoid, because of the script parsing issues. In particular, embedded and/or dynamic calls in PLs will get hairy if not turing complete and outright impossible to solve. -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] ALTER OBJECT any_name SET SCHEMA name
On 31.10.2010 19:38, Dimitri Fontaine wrote: Sorry for the interruption, our program now continues... Dimitri Fontainedimi...@2ndquadrant.fr writes: That's exactly the road I want to avoid, because of the script parsing issues. In particular, embedded and/or dynamic calls in PLs will get hairy if not turing complete and outright impossible to solve. Sorry, I don't follow. Got an example? -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] ALTER OBJECT any_name SET SCHEMA name
Heikki Linnakangas heikki.linnakan...@enterprisedb.com writes: In particular, embedded and/or dynamic calls in PLs will get hairy if not turing complete and outright impossible to solve. Sorry, I don't follow. Got an example? Well, who's to say the following hypothetical plpgsql example should be forgiven only in an exception's script? v_sql := 'SELECT * FROM ' || p_fun || '()'; FOR rec in EXECUTE v_sql LOOP … END LOOP; Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] ALTER OBJECT any_name SET SCHEMA name
On Sun, Oct 31, 2010 at 12:45 PM, Dimitri Fontaine dimi...@2ndquadrant.fr wrote: Bernd Helmle maili...@oopsware.de writes: This reminds me of a small discussion we had some years ago when i targeted this for the sake of completeness of ASS (see http://archives.postgresql.org/pgsql-patches/2006-06/msg00021.php). Discovered it, thanks for the pointer. I didn't follow the previous discussions about EXTENSION very closely, but to amend the idea made in the mentioned thread, couldn't we just invent a facility to move classes of objects belonging to an extension, only? Well under the hood you still need about the same code. In the current patch version I have, that would mean forgetting about a ~20 lines long function and keeping the other two, 10+30 lines about. Now if someone has a idea on how to have a single routine that knows how to deal with any object type and change its namespace, I'm all ears, and willing to have a try at it. Coding in C is for me a twice a year exercise, so whatever I come up with will be... improvable, I fear. I guess it's a question of how much special case code there is. The new objectaddress.c code knows how to deal with lots of different object types, but for example in the case of ALTER TABLE .. SET SCHEMA, what's there now is enmeshed in a complex structure so as to allow multiple ALTER TABLE subcommands in a single statement, and it has further special-case logic to handle moving the rowtype and related indexes, sequences, and constraints. It seems hard to fit that into a general framework, but maybe it could be done for other object types. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] ALTER OBJECT any_name SET SCHEMA name
On 31.10.2010 20:19, Dimitri Fontaine wrote: Heikki Linnakangasheikki.linnakan...@enterprisedb.com writes: In particular, embedded and/or dynamic calls in PLs will get hairy if not turing complete and outright impossible to solve. Sorry, I don't follow. Got an example? Well, who's to say the following hypothetical plpgsql example should be forgiven only in an exception's script? v_sql := 'SELECT * FROM ' || p_fun || '()'; FOR rec in EXECUTE v_sql LOOP … END LOOP; If I understand that correctly, the idea is that p_fun holds the name of a function that's in the same schema as the extension? You would write that as v_sql := 'SELECT * FROM @extsch...@.' || p_fun || '()'; FOR rec in EXECUTE v_sql LOOP … END LOOP; -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] ALTER OBJECT any_name SET SCHEMA name
Robert Haas robertmh...@gmail.com writes: ... related indexes, sequences, and constraints. It seems hard to fit that into a general framework, but maybe it could be done for other object types. My guess is that we're talking about having a generic code that would get exercised directly from src/backend/commands/alter.c in the function ExecAlterObjectSchemaStmt(AlterObjectSchemaStmt *stmt), rather than having a switch() statement and very similar functions implemented all over the place to do the actual bit editing. Let's see some details, if we're getting there. This line of code is repeated in lots of places, but always a little different: + conversionOid = get_conversion_oid(name, false); + operOid = LookupOperNameTypeNames(NULL, operatorName, + typeName1, typeName2, + false, -1); + prsId = get_ts_parser_oid(name, false); + dictId = get_ts_dict_oid(name, false); And for operator class and operator family, we're dealing with HeapTuple objects directly, rather than Oids, because the lookup ain't so easy. But all in all that code could get moved into another layer in the generic function, in a object type switch, if we get to HeapTuple based internal API, because next example shows that in most cases we get the tuple from the Oid in the same way. In the following code part, I can see how to get generic code with some parameters for the first 3 lines, but not so much for the other 2 lines: + tup = SearchSysCacheCopy1(CONVOID, ObjectIdGetDatum(conversionOid)); + if (!HeapTupleIsValid(tup)) /* should not happen */ + elog(ERROR, cache lookup failed for conversion %u, conversionOid); + + convForm = (Form_pg_conversion) GETSTRUCT(tup); + oldNspOid = convForm-connamespace; Then there's this code which looks even harder to get generic: + /* Otherwise, must be owner of the existing object */ + if (!pg_conversion_ownercheck(HeapTupleGetOid(tup), GetUserId())) + aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_CONVERSION, + NameStr(convForm-conname)); Now the following part could get moved easily to the CheckSetNamespace() function introduced in the patch, with a boolean to trigger the check, because as it is not all objects are open to non-superuser changes. + /* New owner must have CREATE privilege on namespace */ + aclresult = pg_namespace_aclcheck(nspOid, GetUserId(), ACL_CREATE); + if (aclresult != ACLCHECK_OK) + aclcheck_error(aclresult, ACL_KIND_NAMESPACE, + get_namespace_name(nspOid)); Then in the end of the function, we have the real work, where I can see a generic function for the heap update and the dependency tracking, but not for the tuple editing itself: + /* tup is a copy, so we can scribble directly on it */ + opfForm-opfnamespace = nspOid; + + simple_heap_update(rel, tup-t_self, tup); + CatalogUpdateIndexes(rel, tup); + + /* update dependencies to point to the new schema */ + changeDependencyFor(OperatorFamilyRelationId, HeapTupleGetOid(tup), + NamespaceRelationId, oldNspOid, nspOid); So it seems to get down to C'fu related to handling Form_pg_* pointers to get to values and change them then simple_heap_update the tuple. The other parts are all about the same code with different RelationId and such. Well of course you think you could just have a pointer as argument, but how to get the pointer is something we'd prefer generic too. Maybe we need a 3-step code here, in aforementioned ExecAlterObjectSchemaStmt: 1. call a generic function that switch on stmt-objectType and returns an HeapTuple 2. switch on stmt-objectType to get oldNspOid from the tuple and to check for permissions 3. call another generic function to do the namespace related checks then the heap update etc This way the only specific code we need to maintain are the second step here, object dependent, and in the first function too, but that's not an entire API like in current patch. Well of course there's the question of how to do that in a way that allows some other backend's code to get the action done with Oids as input rather than a statement, but that looks workable: get from classid to objectype (aren't they equal?) then call the step 1. function to get the HeapTyple, and proceed. So in fact the 3 steps are a separate function that is called either from ExecAlterObjectSchemaStmt or from the extension's code. And there's also the question whether that has anything to do with what Tom would want to see happening, too (or I'll be talking about those ideas with gcc rather than with the list) :)
Re: [HACKERS] ALTER OBJECT any_name SET SCHEMA name
Heikki Linnakangas heikki.linnakan...@enterprisedb.com writes: If I understand that correctly, the idea is that p_fun holds the name of a function that's in the same schema as the extension? You would write that as v_sql := 'SELECT * FROM @extsch...@.' || p_fun || '()'; Fair enough. Now what about the citext example, where IIRC the failure is not on function names but operators and opclass not found, etc. Forcing extension's author to get to always use the following notation seems to me like pushing it: - WHERE foo = bar + WHERE foo operator(@extsch...@.=) bar Also, those examples are plpgsql but extensions are free to depend on plperl or plpython, or even some pljs or plscheme out there. The alternative is to give DBAs the tools to move their local extensions in some schema that is part of their edited search_path, should they remove public from there. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] ALTER OBJECT any_name SET SCHEMA name
On 31.10.2010 21:42, Dimitri Fontaine wrote: Heikki Linnakangasheikki.linnakan...@enterprisedb.com writes: If I understand that correctly, the idea is that p_fun holds the name of a function that's in the same schema as the extension? You would write that as v_sql := 'SELECT * FROM @extsch...@.' || p_fun || '()'; Fair enough. Now what about the citext example, where IIRC the failure is not on function names but operators and opclass not found, etc. Just do SET search_pa...@extschema@ at the beginning of the install script, just like we have SET search_path=public there now. Forcing extension's author to get to always use the following notation seems to me like pushing it: - WHERE foo = bar + WHERE foo operator(@extsch...@.=) bar Also, those examples are plpgsql but extensions are free to depend on plperl or plpython, or even some pljs or plscheme out there. Well, in case of functions you can always do CREATE FUNCTION ... AS $$ ... $$ SET search_pa...@extschema. ALTER ... SET SCHEMA wouldn't do anything for SQL statements embedded in plperl or plpython anyway. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] ALTER OBJECT any_name SET SCHEMA name
Heikki Linnakangas heikki.linnakan...@enterprisedb.com writes: Just do SET search_pa...@extschema@ at the beginning of the install script, just like we have SET search_path=public there now. Well there's the installation itself then the runtime, as you say later... Well, in case of functions you can always do CREATE FUNCTION ... AS $$ ... $$ SET search_pa...@extschema. Fair enough. ALTER ... SET SCHEMA wouldn't do anything for SQL statements embedded in plperl or plpython anyway. That's why I was thinking about adding the possibility to: - easily find your function's etc OID, that's already mainly done - be able to call/use those objects per OID Ok that sucks somehow. I think it's better than @extschema@ replacing in the extension's script parsing, though. Maybe we should just shut down this attempt at working on search_path and extensions together, again. I though it was a simple and good enough solution though, and that it would avoid the usual rat holes. But we're deep in them already. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] ALTER OBJECT any_name SET SCHEMA name
On Sun, Oct 31, 2010 at 5:46 PM, Dimitri Fontaine dimi...@2ndquadrant.fr wrote: Heikki Linnakangas heikki.linnakan...@enterprisedb.com writes: Just do SET search_pa...@extschema@ at the beginning of the install script, just like we have SET search_path=public there now. Well there's the installation itself then the runtime, as you say later... Well, in case of functions you can always do CREATE FUNCTION ... AS $$ ... $$ SET search_pa...@extschema. Fair enough. ALTER ... SET SCHEMA wouldn't do anything for SQL statements embedded in plperl or plpython anyway. That's why I was thinking about adding the possibility to: - easily find your function's etc OID, that's already mainly done - be able to call/use those objects per OID Ok that sucks somehow. Yeah, I think that sucks a lot. I don't see what's wrong with Heikki's solution, actually. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] ALTER OBJECT any_name SET SCHEMA name
Robert Haas robertmh...@gmail.com writes: Yeah, I think that sucks a lot. I don't see what's wrong with Heikki's solution, actually. Coding the parser and replace. If all it takes is calling our replace function on the all-in-memory query string that we have in pg_execute_from_file() function, I can have a try at it. Main wrong point is that it puts the burden on the extension's authors rather than on the one who manages the search_path for its applications... -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] ALTER OBJECT any_name SET SCHEMA name
Hi,
In the road to the extension patch, we already found some parts that
have to be separated into their own patch. Here's another one. It
occurred to me while implementing the pg_extension_objects() SRF that if
we can list all objects that belong to an extension, certainly we also
are able to move them to another schema.
As soon as we have that ability, we are able to provide for relocatable
extensions with the following command:
ALTER EXTENSION ext SET SCHEMA name;
ALTER EXTENSION ext SET SCHEMA foo TO bar;
I think that would end the open debate about search_path vs extension,
because each user would be able to relocate his local extensions easily,
wherever the main script has installed them (often enough, public).
Please find attached a work-in-progress patch (it's missing
documentation) implementing support for setting a new schema to SQL
objects of types conversion, operator, operator class, operator family,
text search parser, dictionary, template and configuration.
If there's will to apply such a patch, I'll finish it by writing the
necessary documentation for the 8 new SQL commands.
Note: CreateCommandTag() already has support for tags for ALTER TEXT
SEARCH OBJECT … SET SCHEMA …, but the implementation I've not
found, in the grammar nor in tsearchcmds.c. It's in the patch.
As usual, you can also get to the development version by using git:
http://git.postgresql.org/gitweb?p=postgresql-extension.git;a=shortlog;h=refs/heads/set_schema
git --no-pager diff master..|diffstat
backend/catalog/pg_namespace.c| 38
backend/commands/alter.c | 32
backend/commands/conversioncmds.c | 84 ++
backend/commands/opclasscmds.c| 215 +++
backend/commands/operatorcmds.c | 90 +++
backend/commands/tsearchcmds.c| 295 ++
backend/parser/gram.y | 67
backend/tcop/utility.c| 12 +
include/catalog/pg_namespace.h|2
include/commands/conversioncmds.h |5
include/commands/defrem.h | 23 ++
11 files changed, 863 insertions(+)
Regards,
--
Dimitri Fontaine
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
*** a/src/backend/catalog/pg_namespace.c
--- b/src/backend/catalog/pg_namespace.c
***
*** 17,24
--- 17,26
#include access/heapam.h
#include catalog/dependency.h
#include catalog/indexing.h
+ #include catalog/namespace.h
#include catalog/pg_namespace.h
#include utils/builtins.h
+ #include utils/lsyscache.h
#include utils/rel.h
#include utils/syscache.h
***
*** 77,79 NamespaceCreate(const char *nspName, Oid ownerId)
--- 79,117
return nspoid;
}
+
+ /*
+ * Check new namespace validity in ALTER OBJECT ... SET SCHEMA ... and
+ * ereport(ERROR, ...) in case of any problem.
+ */
+ void
+ CheckSetNamespace(Oid oldNspOid, Oid nspOid,
+ const char *name, const char *objtype)
+ {
+ if (oldNspOid == nspOid)
+ ereport(ERROR,
+ (errcode(ERRCODE_DUPLICATE_OBJECT),
+ errmsg(%s \%s\ already exists in schema \%s\,
+ objtype, name, get_namespace_name(nspOid;
+
+ /* disallow renaming into or out of temp schemas */
+ if (isAnyTempNamespace(nspOid) || isAnyTempNamespace(oldNspOid))
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg(cannot move objects into or out of temporary schemas)));
+
+ /* same for TOAST schema */
+ if (nspOid == PG_TOAST_NAMESPACE || oldNspOid == PG_TOAST_NAMESPACE)
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg(cannot move objects into or out of TOAST schema)));
+
+ /* check for duplicate name (more friendly than unique-index failure) */
+ if (SearchSysCacheExists2(TYPENAMENSP,
+ CStringGetDatum(name),
+ ObjectIdGetDatum(nspOid)))
+ ereport(ERROR,
+ (errcode(ERRCODE_DUPLICATE_OBJECT),
+ errmsg(%s \%s\ already exists in schema \%s\,
+ objtype, name, get_namespace_name(nspOid;
+ }
*** a/src/backend/commands/alter.c
--- b/src/backend/commands/alter.c
***
*** 182,192 ExecAlterObjectSchemaStmt(AlterObjectSchemaStmt *stmt)
--- 182,208
stmt-newschema);
break;
+ case OBJECT_CONVERSION:
+ AlterConversionNamespace(stmt-object, stmt-newschema);
+ break;
+
case OBJECT_FUNCTION:
AlterFunctionNamespace(stmt-object, stmt-objarg, false,
stmt-newschema);
break;
+ case OBJECT_OPERATOR:
+ AlterOperatorNamespace(stmt-object, stmt-objarg, stmt-newschema);
+ break;
+
+ case OBJECT_OPCLASS:
+ AlterOpClassNamespace(stmt-object, stmt-objarg, stmt-newschema);
+ break;
+
+ case OBJECT_OPFAMILY:
+ AlterOpFamilyNamespace(stmt-object, stmt-objarg, stmt-newschema);
+ break;
+
case OBJECT_SEQUENCE:
case OBJECT_TABLE:
case OBJECT_VIEW:
***
*** 195,200
Re: [HACKERS] ALTER OBJECT any_name SET SCHEMA name
Dimitri Fontaine dimi...@2ndquadrant.fr writes: As soon as we have that ability, we are able to provide for relocatable extensions with the following command: ALTER EXTENSION ext SET SCHEMA name; ALTER EXTENSION ext SET SCHEMA foo TO bar; I think that would end the open debate about search_path vs extension, because each user would be able to relocate his local extensions easily, wherever the main script has installed them (often enough, public). I'm not sure whether that really fixes anything, or just provides people with a larger-caliber foot-gun. See for example recent complaints about citext misbehaving if it's not in the public schema (or more generally, any schema not in the search path). I think we'd need to think a bit harder about the behavior of objects that aren't in the search path before creating a facility like this, since it seems to be tantamount to promising that extensions won't break when pushed around to different schemas. I'm also a bit less than enthused about the implementation approach. If we're going to have a policy that every object type must support ALTER SET SCHEMA, I think it might be time to refactor, rather than copying-and-pasting similar boilerplate code for every one. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
