Re: [HACKERS] get_object_address support for additional object types
I pushed a fix for this. -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training Services -- 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] get_object_address support for additional object types
* Alvaro Herrera (alvhe...@2ndquadrant.com) wrote: Actually, there was a bug in the changes of the rule for ALTER EXTENSION ADD OPERATOR CLASS. I noticed by chance only, and upon testing it manually I realized I had made a mistake. I then remembered I made the same bug previously, fixed by 5c5ffee80f35, and I'm not wondering why do we not have any test for ALTER EXTENSION ADD other than pg_upgrading some database that contains an extension which uses each command. This seems pretty dangerous to me, generally speaking ... we should definitely be testing all these ALTER EXTENSION commands. It'd certainly be great to have more testing in general, but we're not going to be able to include complete code coverage tests in the normal set of regression tests which are run.. What I've been thinking for a while (probably influenced by other discussion) is that we should have the buildfarm running tests for code coverage as those are async from the development process. That isn't to say we shouldn't add more tests to the main regression suite when it makes sense to, we certainly should, but we really need to be looking at code coverage tools and adding tests to improve our test coverage which can be run by the buildfarm animals (or even just a subset of them, if we feel that having all the animals running them would be excessive). Thanks! Stephen signature.asc Description: Digital signature
Re: [HACKERS] get_object_address support for additional object types
* Andres Freund (and...@2ndquadrant.com) wrote: On 2015-03-16 12:50:13 -0300, Alvaro Herrera wrote: Well, we already have make targets for gcov and friends; you get some HTML charts and marked-up source lines with coverage counts, etc. I don't think we've made any use of that. It'd be neat to have something similar to our doxygen service, running some test suite and publishing the reports on the web. I remember trying to convince someone to set that up for the community, but that seems to have yield no results. Actually I think Peter E. has: http://pgci.eisentraut.org/jenkins/job/postgresql_master_coverage/Coverage/ Neat! Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] get_object_address support for additional object types
Stephen Frost wrote: Alvaro, * Alvaro Herrera (alvhe...@2ndquadrant.com) wrote: Well, we already have make targets for gcov and friends; you get some HTML charts and marked-up source lines with coverage counts, etc. I don't think we've made any use of that. It'd be neat to have something similar to our doxygen service, running some test suite and publishing the reports on the web. I remember trying to convince someone to set that up for the community, but that seems to have yield no results. I don't think we've made use of it either. If the targets/code are already there to make it happen and it's just a matter of having a system running which is generating the website then I can probably get that going. I was under the impression that there was more to it than that though. configure --enable-coverage; install the resulting tree; then run whatever tests you want, then make coverage. That's enough to get the HTML reports. We had someone else trying to submit patches to improve coverage of the regression tests, but (probably due to wrong stars alignment) they started with CREATE DATABASE which made the tests a lot slower, which got the patches turned down -- the submitter disappeared after that IIRC, probably discouraged by the lack of results. Agreed, and I think that's unfortunate. It's an area which we could really improve in and would be a good place for someone new to the community to be able to contribute- but we need to provide the right way for those tests to be added and that way isn't to include them in the main suite of tests which are run during development. Well, I don't disagree that we could do with some tests that are run additionally to the ones we currently have, but some basic stuff that takes almost no time to run would be adequate to have in the basic regression tests. The one I'm thinking is something like generate a VALUES of all the supported object types, then running pg_get_object_address on them to make sure we support all object types (or at least that we're aware which object types are not supported.) For instance, Petr Jelinek's patch to add the TABLESAMPLE clause adds a new object type which needs support in get_object_address, but I'm not sure it's added to the stuff in the object_address test. It's a very easy omission to make. -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training Services -- 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] get_object_address support for additional object types
On 2015-03-16 12:50:13 -0300, Alvaro Herrera wrote: Well, we already have make targets for gcov and friends; you get some HTML charts and marked-up source lines with coverage counts, etc. I don't think we've made any use of that. It'd be neat to have something similar to our doxygen service, running some test suite and publishing the reports on the web. I remember trying to convince someone to set that up for the community, but that seems to have yield no results. Actually I think Peter E. has: http://pgci.eisentraut.org/jenkins/job/postgresql_master_coverage/Coverage/ We had someone else trying to submit patches to improve coverage of the regression tests, but (probably due to wrong stars alignment) they started with CREATE DATABASE which made the tests a lot slower, which got the patches turned down -- the submitter disappeared after that IIRC, probably discouraged by the lack of results. I seem to recall that he'd also submitted a bunch of other things, and that some of it was applied? Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- 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] get_object_address support for additional object types
Alvaro, * Alvaro Herrera (alvhe...@2ndquadrant.com) wrote: Stephen Frost wrote: I don't think we've made use of it either. If the targets/code are already there to make it happen and it's just a matter of having a system running which is generating the website then I can probably get that going. I was under the impression that there was more to it than that though. configure --enable-coverage; install the resulting tree; then run whatever tests you want, then make coverage. That's enough to get the HTML reports. Ok, cool, I'll take a look at that. We had someone else trying to submit patches to improve coverage of the regression tests, but (probably due to wrong stars alignment) they started with CREATE DATABASE which made the tests a lot slower, which got the patches turned down -- the submitter disappeared after that IIRC, probably discouraged by the lack of results. Agreed, and I think that's unfortunate. It's an area which we could really improve in and would be a good place for someone new to the community to be able to contribute- but we need to provide the right way for those tests to be added and that way isn't to include them in the main suite of tests which are run during development. Well, I don't disagree that we could do with some tests that are run additionally to the ones we currently have, but some basic stuff that takes almost no time to run would be adequate to have in the basic regression tests. The one I'm thinking is something like generate a VALUES of all the supported object types, then running pg_get_object_address on them to make sure we support all object types (or at least that we're aware which object types are not supported.) Agreed, that sounds perfectly reasonable to me. I didn't mean to imply that we shouldn't add tests where they make sense, including to the core regression suites (particularly coverage tests like that one you're suggesting here), just pointing out that we need a way to address code coverage based tested also which is done outside of the main regression suite. For instance, Petr Jelinek's patch to add the TABLESAMPLE clause adds a new object type which needs support in get_object_address, but I'm not sure it's added to the stuff in the object_address test. It's a very easy omission to make. Agreed. Thanks! Stephen signature.asc Description: Digital signature
Re: [HACKERS] get_object_address support for additional object types
Alvaro, * Alvaro Herrera (alvhe...@2ndquadrant.com) wrote: Stephen Frost wrote: It'd certainly be great to have more testing in general, but we're not going to be able to include complete code coverage tests in the normal set of regression tests which are run.. What I've been thinking for a while (probably influenced by other discussion) is that we should have the buildfarm running tests for code coverage as those are async from the development process. That isn't to say we shouldn't add more tests to the main regression suite when it makes sense to, we certainly should, but we really need to be looking at code coverage tools and adding tests to improve our test coverage which can be run by the buildfarm animals (or even just a subset of them, if we feel that having all the animals running them would be excessive). Well, we already have make targets for gcov and friends; you get some HTML charts and marked-up source lines with coverage counts, etc. I don't think we've made any use of that. It'd be neat to have something similar to our doxygen service, running some test suite and publishing the reports on the web. I remember trying to convince someone to set that up for the community, but that seems to have yield no results. I don't think we've made use of it either. If the targets/code are already there to make it happen and it's just a matter of having a system running which is generating the website then I can probably get that going. I was under the impression that there was more to it than that though. We had someone else trying to submit patches to improve coverage of the regression tests, but (probably due to wrong stars alignment) they started with CREATE DATABASE which made the tests a lot slower, which got the patches turned down -- the submitter disappeared after that IIRC, probably discouraged by the lack of results. Agreed, and I think that's unfortunate. It's an area which we could really improve in and would be a good place for someone new to the community to be able to contribute- but we need to provide the right way for those tests to be added and that way isn't to include them in the main suite of tests which are run during development. Thanks! Stephen signature.asc Description: Digital signature
Re: [HACKERS] get_object_address support for additional object types
Stephen Frost wrote: Alvaro, * Alvaro Herrera (alvhe...@2ndquadrant.com) wrote: Stephen Frost wrote: I thought the rest of it looked alright. I agree it's a bit odd how the opfamily is handled but I agree with your assessment that there's not much better we can do with this object representation. Actually, on second thought I revisited this and changed the representation for opfamilies and opclasses: instead of putting the AM name in objargs, we can put it as the first element of objname instead. That way, objargs is unused for opfamilies and opclasses, and we're free to use it for the type arguments in amops and amprocs. This makes the lists consistent for the four cases: in objname, amname first, then qualified opclass/opfamily name. For amop/amproc, the member number follows. Objargs is unused in opclass/opfamily, and it's a two-element list of types in amop/amproc. Agreed, that makes more sense to me also. Great, thanks for checking -- pushed that way. The attached patch changes the grammar to comply with the above, and adds the necessary get_object_address and getObjectIdentityParts support code for amop/amproc objects. I took a quick look through and it looked fine to me. Actually, there was a bug in the changes of the rule for ALTER EXTENSION ADD OPERATOR CLASS. I noticed by chance only, and upon testing it manually I realized I had made a mistake. I then remembered I made the same bug previously, fixed by 5c5ffee80f35, and I'm not wondering why do we not have any test for ALTER EXTENSION ADD other than pg_upgrading some database that contains an extension which uses each command. This seems pretty dangerous to me, generally speaking ... we should definitely be testing all these ALTER EXTENSION commands. -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training Services -- 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] get_object_address support for additional object types
Stephen Frost wrote: It'd certainly be great to have more testing in general, but we're not going to be able to include complete code coverage tests in the normal set of regression tests which are run.. What I've been thinking for a while (probably influenced by other discussion) is that we should have the buildfarm running tests for code coverage as those are async from the development process. That isn't to say we shouldn't add more tests to the main regression suite when it makes sense to, we certainly should, but we really need to be looking at code coverage tools and adding tests to improve our test coverage which can be run by the buildfarm animals (or even just a subset of them, if we feel that having all the animals running them would be excessive). Well, we already have make targets for gcov and friends; you get some HTML charts and marked-up source lines with coverage counts, etc. I don't think we've made any use of that. It'd be neat to have something similar to our doxygen service, running some test suite and publishing the reports on the web. I remember trying to convince someone to set that up for the community, but that seems to have yield no results. We had someone else trying to submit patches to improve coverage of the regression tests, but (probably due to wrong stars alignment) they started with CREATE DATABASE which made the tests a lot slower, which got the patches turned down -- the submitter disappeared after that IIRC, probably discouraged by the lack of results. -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training Services -- 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] get_object_address support for additional object types
On 16 March 2015 at 15:11, Alvaro Herrera alvhe...@2ndquadrant.com wrote: Actually, on second thought I revisited this and changed the representation for opfamilies and opclasses: instead of putting the AM name in objargs, we can put it as the first element of objname instead. That way, objargs is unused for opfamilies and opclasses, and we're free to use it for the type arguments in amops and amprocs. This makes the lists consistent for the four cases: in objname, amname first, then qualified opclass/opfamily name. For amop/amproc, the member number follows. Objargs is unused in opclass/opfamily, and it's a two-element list of types in amop/amproc. Agreed, that makes more sense to me also. Great, thanks for checking -- pushed that way. I'm getting a couple of compiler warnings that I think are coming from this commit: objectaddress.c: In function ‘get_object_address’: objectaddress.c:1428:12: warning: array subscript is above array bounds objectaddress.c:1430:11: warning: array subscript is above array bounds I think because the compiler doesn't know the list size, so i could be 0,1,2. Regards, Dean -- 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] get_object_address support for additional object types
Alvaro, * Alvaro Herrera (alvhe...@2ndquadrant.com) wrote: Stephen Frost wrote: I thought the rest of it looked alright. I agree it's a bit odd how the opfamily is handled but I agree with your assessment that there's not much better we can do with this object representation. Actually, on second thought I revisited this and changed the representation for opfamilies and opclasses: instead of putting the AM name in objargs, we can put it as the first element of objname instead. That way, objargs is unused for opfamilies and opclasses, and we're free to use it for the type arguments in amops and amprocs. This makes the lists consistent for the four cases: in objname, amname first, then qualified opclass/opfamily name. For amop/amproc, the member number follows. Objargs is unused in opclass/opfamily, and it's a two-element list of types in amop/amproc. Agreed, that makes more sense to me also. The attached patch changes the grammar to comply with the above, and adds the necessary get_object_address and getObjectIdentityParts support code for amop/amproc objects. I took a quick look through and it looked fine to me. The only thing a bit unusual is that in does_not_exist_skipping() we need to do an list_copy_tail() to strip out the amname before reporting the skipping message, for DROP OPERATOR CLASS/FAMILY IF NOT EXISTS. I don't think this is a problem. In return, the code to deconstruct amop and amproc addresses is more sensible. Works for me. Thanks! Stephen signature.asc Description: Digital signature
Re: [HACKERS] get_object_address support for additional object types
Stephen Frost wrote:
I thought the rest of it looked alright. I agree it's a bit odd how the
opfamily is handled but I agree with your assessment that there's not
much better we can do with this object representation.
Actually, on second thought I revisited this and changed the
representation for opfamilies and opclasses: instead of putting the AM
name in objargs, we can put it as the first element of objname instead.
That way, objargs is unused for opfamilies and opclasses, and we're free
to use it for the type arguments in amops and amprocs. This makes the
lists consistent for the four cases: in objname, amname first, then
qualified opclass/opfamily name. For amop/amproc, the member number
follows. Objargs is unused in opclass/opfamily, and it's a two-element
list of types in amop/amproc.
The attached patch changes the grammar to comply with the above, and
adds the necessary get_object_address and getObjectIdentityParts support
code for amop/amproc objects.
The only thing a bit unusual is that in does_not_exist_skipping() we
need to do an list_copy_tail() to strip out the amname before reporting
the skipping message, for DROP OPERATOR CLASS/FAMILY IF NOT EXISTS.
I don't think this is a problem. In return, the code to deconstruct
amop and amproc addresses is more sensible.
--
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training Services
From 35336c997b8be18d890a3a8bdf2822f757f70faf Mon Sep 17 00:00:00 2001
From: Alvaro Herrera alvhe...@alvh.no-ip.org
Date: Fri, 6 Mar 2015 12:39:50 -0300
Subject: [PATCH] Support opfamily members in get_object_address
In the spirit of 890192e99af and 4464303405f, have get_object_address
understand pg_amop and pg_amproc objects. There is no way to refer to
such objects directly in the grammar, but in event triggers and
pg_get_object_address it becomes possible to become involved with them.
Reviewed by: Stephen Frost
---
src/backend/catalog/objectaddress.c | 234 +--
src/backend/commands/dropcmds.c | 24 ++-
src/backend/commands/event_trigger.c | 2 +
src/backend/parser/gram.y| 43 ++---
src/include/nodes/parsenodes.h | 2 +
src/test/regress/expected/object_address.out | 60 ---
src/test/regress/sql/object_address.sql | 16 +-
7 files changed, 264 insertions(+), 117 deletions(-)
diff --git a/src/backend/catalog/objectaddress.c b/src/backend/catalog/objectaddress.c
index 142bc68..46ea09a 100644
--- a/src/backend/catalog/objectaddress.c
+++ b/src/backend/catalog/objectaddress.c
@@ -492,9 +492,9 @@ ObjectTypeMap[] =
/* OCLASS_OPFAMILY */
{ operator family, OBJECT_OPFAMILY },
/* OCLASS_AMOP */
- { operator of access method, -1 }, /* unmapped */
+ { operator of access method, OBJECT_AMOP },
/* OCLASS_AMPROC */
- { function of access method, -1 }, /* unmapped */
+ { function of access method, OBJECT_AMPROC },
/* OCLASS_REWRITE */
{ rule, OBJECT_RULE },
/* OCLASS_TRIGGER */
@@ -552,9 +552,12 @@ static ObjectAddress get_object_address_attrdef(ObjectType objtype,
List *objname, Relation *relp, LOCKMODE lockmode,
bool missing_ok);
static ObjectAddress get_object_address_type(ObjectType objtype,
- List *objname, bool missing_ok);
+ ListCell *typecell, bool missing_ok);
static ObjectAddress get_object_address_opcf(ObjectType objtype, List *objname,
- List *objargs, bool missing_ok);
+ bool missing_ok);
+static ObjectAddress get_object_address_opf_member(ObjectType objtype,
+ List *objname, List *objargs, bool missing_ok);
+
static ObjectAddress get_object_address_usermapping(List *objname,
List *objargs, bool missing_ok);
static ObjectAddress get_object_address_defacl(List *objname, List *objargs,
@@ -567,8 +570,7 @@ static void getRelationTypeDescription(StringInfo buffer, Oid relid,
int32 objectSubId);
static void getProcedureTypeDescription(StringInfo buffer, Oid procid);
static void getConstraintTypeDescription(StringInfo buffer, Oid constroid);
-static void getOpFamilyIdentity(StringInfo buffer, Oid opfid, List **objname,
- List **objargs);
+static void getOpFamilyIdentity(StringInfo buffer, Oid opfid, List **objname);
static void getRelationIdentity(StringInfo buffer, Oid relid, List **objname);
/*
@@ -661,7 +663,8 @@ get_object_address(ObjectType objtype, List *objname, List *objargs,
ObjectAddress domaddr;
char *constrname;
- domaddr = get_object_address_type(OBJECT_DOMAIN, objname, missing_ok);
+ domaddr = get_object_address_type(OBJECT_DOMAIN,
+ list_head(objname), missing_ok);
constrname = strVal(linitial(objargs));
address.classId = ConstraintRelationId;
@@ -685,7 +688,7 @@ get_object_address(ObjectType objtype, List *objname, List *objargs,
break;
case OBJECT_TYPE:
case OBJECT_DOMAIN:
-address =
Re: [HACKERS] get_object_address support for additional object types
Stephen, thanks for the comments. Stephen Frost wrote: I don't really care for how all the get_object_address stuff uses lists for arguments instead of using straight-forward arguments, but it's how it's been done and I can kind of see the reasoning behind it. I'm not following why you're switching this case (get_object_address_type) to using a ListCell though..? The reason for changing get_object_address_type is that it's a helper soubroutine to get_object_address and we can do whatever is more convenient. It turns out that it's more convenient, for the amop/amproc cases, to pass a ListCell instead of a List. get_object_address gets its stuff directly from the parser in some cases, or from pg_get_object_address otherwise, which constructs things to mimick exactly what the parser does. We can change what the parser emits and as long as we keep get_object_address in agreement, there is no issue. There might be code churn of course (because some of those object representations travel from the parser through other parts of the backend), but that doesn't really matter much. We have now exposed that interface through the SQL-callable pg_get_object_address function, but I'm pretty sure we could change that easily and it wouldn't be a tremendous problem -- as long as we do it before 9.5 is released. For instance we might want to decide that we want to have three lists instead of two; or two lists and a numeric quantity (which would also help the large object case, currently a bit ugly), or that we want something akin to what the parser does to types using struct TypeName for opclass-related objects. Anyway I'm not hot on changing anything here. It's a bit cumbersome an interface to use, but it's not excessively exposed to the user unless they use event triggers, and even then it is perfectly reliable anyhow. I thought the rest of it looked alright. I agree it's a bit odd how the opfamily is handled but I agree with your assessment that there's not much better we can do with this object representation. Great, thanks. -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training Services -- 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] get_object_address support for additional object types
* Alvaro Herrera (alvhe...@2ndquadrant.com) wrote: Anyway I'm not hot on changing anything here. It's a bit cumbersome an interface to use, but it's not excessively exposed to the user unless they use event triggers, and even then it is perfectly reliable anyhow. Works for me. Thanks! Stephen signature.asc Description: Digital signature
Re: [HACKERS] get_object_address support for additional object types
Alvaro, * Alvaro Herrera (alvhe...@2ndquadrant.com) wrote: This is extracted from the DDL deparse series. These patches add get_object_address support for the following object types: - user mappings - default ACLs - operators and functions of operator families I took a (relatively quick) look through these patches. Subject: [PATCH 1/3] deparse/core: get_object_address support user mappings [...] I thought this looked fine. One minor nit-pick is that the function added doesn't have a single comment, but it's a pretty short too. Subject: [PATCH 2/3] deparse/core: get_object_address support default ACLs [...] + char *stuff; Nit-pick, but 'stuff' isn't really a great variable name. :) Perhaps 'defacltype_name'? It's longer, sure, but it's not used a lot.. Subject: [PATCH 3/3] deparse/core: get_object_address support opfamily members @@ -661,7 +664,8 @@ get_object_address(ObjectType objtype, List *objname, List *objargs, ObjectAddress domaddr; char *constrname; - domaddr = get_object_address_type(OBJECT_DOMAIN, objname, missing_ok); + domaddr = get_object_address_type(OBJECT_DOMAIN, + list_head(objname), missing_ok); constrname = strVal(linitial(objargs)); address.classId = ConstraintRelationId; I don't really care for how all the get_object_address stuff uses lists for arguments instead of using straight-forward arguments, but it's how it's been done and I can kind of see the reasoning behind it. I'm not following why you're switching this case (get_object_address_type) to using a ListCell though..? I thought the rest of it looked alright. I agree it's a bit odd how the opfamily is handled but I agree with your assessment that there's not much better we can do with this object representation. Thanks! Stephen signature.asc Description: Digital signature
[HACKERS] get_object_address support for additional object types
This is extracted from the DDL deparse series. These patches add
get_object_address support for the following object types:
- user mappings
- default ACLs
- operators and functions of operator families
In each case I had to create a new value in ObjectType. These object
types can not be created from the parser, which is why they don't exist
yet. But if we want to be able to process DDL for them, then we need to
cope at this level. This is the kind of fix we need to handle the
failures related to commit a486841eb11517e.
There is one strange thing in the last one, which is that an opfamily
member is represented in two arrays like this (objname, objargs):
{opfamily identity, access method identity, number} , {left type, right type}
This is a bit odd considering that operator families themselves are
addressed like this instead:
{opfamily identity} , {access method identity}
Note that the AM name is originally in objargs and moves to objnames.
The reason I did it this way is that the objargs elements can be
processed completely as an array of TypeName, and therefore there's no
need for an extra strange case in pg_get_object_address. But it does
mean that there is some code that knows to search the strategy or
function number in a specific position in the objname array.
If we had more freedom on general object representation I'm sure we
could do better, but it's what we have. I don't think it's a tremendous
problem, considering that get_object_address gets a fairly ad-hoc
representation for each object type anyway, as each gets constructed by
the grammar.
--
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training Services
From fad598488c0c15e0962808ad13825374e8a3640e Mon Sep 17 00:00:00 2001
From: Alvaro Herrera alvhe...@alvh.no-ip.org
Date: Thu, 5 Mar 2015 12:04:39 -0300
Subject: [PATCH 1/3] deparse/core: get_object_address support user mappings
---
src/backend/catalog/objectaddress.c | 67 +++-
src/backend/commands/event_trigger.c | 1 +
src/include/nodes/parsenodes.h | 1 +
src/test/regress/expected/event_trigger.out | 12 -
src/test/regress/expected/object_address.out | 19 +---
src/test/regress/sql/event_trigger.sql | 11 -
src/test/regress/sql/object_address.sql | 9 ++--
7 files changed, 107 insertions(+), 13 deletions(-)
diff --git a/src/backend/catalog/objectaddress.c b/src/backend/catalog/objectaddress.c
index 541912b..5553ec7 100644
--- a/src/backend/catalog/objectaddress.c
+++ b/src/backend/catalog/objectaddress.c
@@ -520,7 +520,7 @@ ObjectTypeMap[] =
/* OCLASS_FOREIGN_SERVER */
{ server, OBJECT_FOREIGN_SERVER },
/* OCLASS_USER_MAPPING */
- { user mapping, -1 }, /* unmapped */
+ { user mapping, OBJECT_USER_MAPPING },
/* OCLASS_DEFACL */
{ default acl, -1 }, /* unmapped */
/* OCLASS_EXTENSION */
@@ -555,6 +555,8 @@ static ObjectAddress get_object_address_type(ObjectType objtype,
List *objname, bool missing_ok);
static ObjectAddress get_object_address_opcf(ObjectType objtype, List *objname,
List *objargs, bool missing_ok);
+static ObjectAddress get_object_address_usermapping(List *objname,
+ List *objargs, bool missing_ok);
static const ObjectPropertyType *get_object_property_data(Oid class_id);
static void getRelationDescription(StringInfo buffer, Oid relid);
@@ -769,6 +771,10 @@ get_object_address(ObjectType objtype, List *objname, List *objargs,
address.objectId = get_ts_config_oid(objname, missing_ok);
address.objectSubId = 0;
break;
+ case OBJECT_USER_MAPPING:
+address = get_object_address_usermapping(objname, objargs,
+ missing_ok);
+break;
default:
elog(ERROR, unrecognized objtype: %d, (int) objtype);
/* placate compiler, in case it thinks elog might return */
@@ -1373,6 +1379,64 @@ get_object_address_opcf(ObjectType objtype,
}
/*
+ * Find the ObjectAddress for a user mapping.
+ */
+static ObjectAddress
+get_object_address_usermapping(List *objname, List *objargs, bool missing_ok)
+{
+ ObjectAddress address;
+ Oid userid;
+ char *username;
+ char *servername;
+ ForeignServer *server;
+ HeapTuple tp;
+
+ ObjectAddressSet(address, UserMappingRelationId, InvalidOid);
+
+ username = strVal(linitial(objname));
+ servername = strVal(linitial(objargs));
+ server = GetForeignServerByName(servername, false);
+
+ if (strcmp(username, public) == 0)
+ userid = InvalidOid;
+ else
+ {
+ tp = SearchSysCache1(AUTHNAME,
+ CStringGetDatum(username));
+ if (!HeapTupleIsValid(tp))
+ {
+ if (!missing_ok)
+ereport(ERROR,
+ (errcode(ERRCODE_UNDEFINED_OBJECT),
+ errmsg(user mapping for user \%s\ in server \%s\ does not exist,
+username, servername)));
+ return address;
+ }
+ userid = HeapTupleGetOid(tp);
+ ReleaseSysCache(tp);
+ }
+
+ tp = SearchSysCache2(USERMAPPINGUSERSERVER,
+ ObjectIdGetDatum(userid),
+
