Re: [HACKERS] [RFC] Common object property boards
2011/8/7 Tom Lane t...@sss.pgh.pa.us:
Kohei KaiGai kai...@kaigai.gr.jp writes:
I'm under implementation of this code according to the suggestion.
However, I'm not sure whether it is really portable way (at least, GCC
accepts),
and whether the interface is simpler than as Robert suggested at first.
#define get_object_property_attnum_name(objtype) \
({ AttrNumber temp; \
get_object_property((objtype), NULL, NULL, NULL, NULL, \
temp, NULL, NULL); \
temp; })
Blocks within expressions are a gcc-ism and will fail on any other
compiler, so you can't do it that way.
Thanks for your suggestion.
So, it seems to me the interface should return a pointer to the entry
of array being specified, rather than above approach.
E.g, the above macro could be probably rewritten as follows:
#define get_object_property_attnum_name(objtype) \
(get_object_property(objtype)-attnum_name)
--
KaiGai Kohei kai...@kaigai.gr.jp
--
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] [RFC] Common object property boards
Excerpts from Kohei KaiGai's message of lun ago 08 03:12:20 -0400 2011: Thanks for your suggestion. So, it seems to me the interface should return a pointer to the entry of array being specified, rather than above approach. E.g, the above macro could be probably rewritten as follows: #define get_object_property_attnum_name(objtype) \ (get_object_property(objtype)-attnum_name) I don't understand why don't you just do something like #define get_object_property_attnum_name(objtype, attnum_name_value) \ (get_object_property((objtype), NULL, NULL, (attnum_name_value), NULL, NULL))) and the caller does AttrNumber attnum_name; get_object_property_attnum_name(OBJTYPE_TABLE, attnum_name); i.e. the caller must still pass pointers, instead of expecting the values to be returned. -- Á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] [RFC] Common object property boards
On Mon, Aug 8, 2011 at 11:57 AM, Alvaro Herrera alvhe...@commandprompt.com wrote: Excerpts from Kohei KaiGai's message of lun ago 08 03:12:20 -0400 2011: Thanks for your suggestion. So, it seems to me the interface should return a pointer to the entry of array being specified, rather than above approach. E.g, the above macro could be probably rewritten as follows: #define get_object_property_attnum_name(objtype) \ (get_object_property(objtype)-attnum_name) I don't understand why don't you just do something like #define get_object_property_attnum_name(objtype, attnum_name_value) \ (get_object_property((objtype), NULL, NULL, (attnum_name_value), NULL, NULL))) and the caller does AttrNumber attnum_name; get_object_property_attnum_name(OBJTYPE_TABLE, attnum_name); i.e. the caller must still pass pointers, instead of expecting the values to be returned. We could do that, but what the heck is the point? What benefit are we trying to get by not returning a pointer to the structure? I feel like we're making this ludicrously complicated with no real justification of why all of this complexity is adding any value. -- 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] [RFC] Common object property boards
Excerpts from Robert Haas's message of lun ago 08 12:05:05 -0400 2011: We could do that, but what the heck is the point? What benefit are we trying to get by not returning a pointer to the structure? I feel like we're making this ludicrously complicated with no real justification of why all of this complexity is adding any value. I don't see that it's complicated in any way. Seems pretty simple to me. The justification is simple too: don't export struct definitions without need. -- Á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] [RFC] Common object property boards
On Mon, Aug 8, 2011 at 12:22 PM, Alvaro Herrera alvhe...@commandprompt.com wrote: Excerpts from Robert Haas's message of lun ago 08 12:05:05 -0400 2011: We could do that, but what the heck is the point? What benefit are we trying to get by not returning a pointer to the structure? I feel like we're making this ludicrously complicated with no real justification of why all of this complexity is adding any value. I don't see that it's complicated in any way. Seems pretty simple to me. It's quite complicated. Every time someone adds a new member to the struct, they need to adjust the code all over the place. The call sequence of the accessor function will constantly be changing. We generally try to structure our code base so that patches don't need to touch more call sites than necessary, and we regularly get very nervous about patches that touch too many call sites and ask them to be changed so that they don't. Here, you're proposing a design that is practically guaranteed to have that problem every time someone makes a change in that area. The justification is simple too: don't export struct definitions without need. That's a reasonable guiding principle, but I believe that the underlying reason to do that is information hiding. In other words, if the details of what's in the struct don't matter to other modules, then they shouldn't peek into it. That way, when you want to change something, you don't have to worry about whether anyone else cares about the encapsulated data, because you already know they don't. In this case, however, the struct - by design - isn't going to contain any private data. If callers are going to potentially need access to every member of the struct, then you may as well export it and be done with it. -- 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] [RFC] Common object property boards
Excerpts from Kohei KaiGai's message of lun ago 08 12:18:47 -0400 2011: 2011/8/8 Robert Haas robertmh...@gmail.com: We could do that, but what the heck is the point? What benefit are we trying to get by not returning a pointer to the structure? I feel like we're making this ludicrously complicated with no real justification of why all of this complexity is adding any value. I agree with Robert's opinion. It seems to me we have little benefit to keep the structure condidential to other components. So you coded my suggestion in an extremely awkward way just to be able to dismiss it? We use that pattern in a lot of places. See get_op_opfamily_properties for the first example I found (took my 15 secs to find it). I don't understand why you think it's so complicated or horrible. Please tell me, why don't we just return Form_pg_amop in that function? -- Á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] [RFC] Common object property boards
2011/8/8 Robert Haas robertmh...@gmail.com: On Mon, Aug 8, 2011 at 11:57 AM, Alvaro Herrera alvhe...@commandprompt.com wrote: Excerpts from Kohei KaiGai's message of lun ago 08 03:12:20 -0400 2011: Thanks for your suggestion. So, it seems to me the interface should return a pointer to the entry of array being specified, rather than above approach. E.g, the above macro could be probably rewritten as follows: #define get_object_property_attnum_name(objtype) \ (get_object_property(objtype)-attnum_name) I don't understand why don't you just do something like #define get_object_property_attnum_name(objtype, attnum_name_value) \ (get_object_property((objtype), NULL, NULL, (attnum_name_value), NULL, NULL))) and the caller does AttrNumber attnum_name; get_object_property_attnum_name(OBJTYPE_TABLE, attnum_name); i.e. the caller must still pass pointers, instead of expecting the values to be returned. We could do that, but what the heck is the point? What benefit are we trying to get by not returning a pointer to the structure? I feel like we're making this ludicrously complicated with no real justification of why all of this complexity is adding any value. I agree with Robert's opinion. It seems to me we have little benefit to keep the structure condidential to other components. Thanks, -- KaiGai Kohei kai...@kaigai.gr.jp -- 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] [RFC] Common object property boards
Excerpts from Robert Haas's message of lun ago 08 12:33:45 -0400 2011: On Mon, Aug 8, 2011 at 12:22 PM, Alvaro Herrera alvhe...@commandprompt.com wrote: Excerpts from Robert Haas's message of lun ago 08 12:05:05 -0400 2011: We could do that, but what the heck is the point? What benefit are we trying to get by not returning a pointer to the structure? I feel like we're making this ludicrously complicated with no real justification of why all of this complexity is adding any value. I don't see that it's complicated in any way. Seems pretty simple to me. It's quite complicated. Every time someone adds a new member to the struct, they need to adjust the code all over the place. The call sequence of the accessor function will constantly be changing. You are rehashing an argument that I already rebutted: the way to solve this problem is with the macros, the format of which we were discussing in this sub-thread. I am not sure why you bring it up again. You say the reason for not exporting the struct definition is that you can later change it without having to touch the callers. That seems a good argument, and it seems to me to work equally for pg_amop as for the new header KaiGai is creating here. In any case, at the start of the thread KaiGai was asking for opinions, and I gave mine. I will now shut up. -- Á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] [RFC] Common object property boards
Robert Haas robertmh...@gmail.com writes:
We could do that, but what the heck is the point? What benefit are
we trying to get by not returning a pointer to the structure?
Not having an ABI break if we find it necessary to add members to the
struct ... which I grant is unlikely to happen in a minor version
update, but it could happen.
Having said that, the proposed coding with a single function returning N
pointer parameters seems like it's been written in the ugliest, least
efficient possible way, and it fails to resolve the ABI-break objection
anyway. (If you did need another struct member, you'd also need another
return parameter, thus forcing recompiles.) The form I had in mind was
N actual functions (not macros) that internally look like
sometype
get_object_property_foo(...)
{
structptr *p = get_object_property_struct(...);
return p-foo;
}
The only objection I can see to this is that somebody who needs several
different attributes of the same object would have to pay the lookup
cost several times. That may or may not be an adequate reason to allow
people to fetch the struct directly; without seeing a list of places
where this would happen, it's impossible to evaluate the performance
costs.
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] [RFC] Common object property boards
On Mon, Aug 8, 2011 at 1:16 PM, Tom Lane t...@sss.pgh.pa.us wrote:
Robert Haas robertmh...@gmail.com writes:
We could do that, but what the heck is the point? What benefit are
we trying to get by not returning a pointer to the structure?
Not having an ABI break if we find it necessary to add members to the
struct ... which I grant is unlikely to happen in a minor version
update, but it could happen.
I think that would only be a problem if we added members to the middle
of the struct, rather than at the end.
However...
Having said that, the proposed coding with a single function returning N
pointer parameters seems like it's been written in the ugliest, least
efficient possible way, and it fails to resolve the ABI-break objection
anyway. (If you did need another struct member, you'd also need another
return parameter, thus forcing recompiles.) The form I had in mind was
N actual functions (not macros) that internally look like
sometype
get_object_property_foo(...)
{
structptr *p = get_object_property_struct(...);
return p-foo;
}
The only objection I can see to this is that somebody who needs several
different attributes of the same object would have to pay the lookup
cost several times. That may or may not be an adequate reason to allow
people to fetch the struct directly; without seeing a list of places
where this would happen, it's impossible to evaluate the performance
costs.
...doing it this way seems fine, because IIRC this is all only being
used to support DDL operations, and the overhead of a few extra
function calls isn't going to matter.
--
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] [RFC] Common object property boards
On Mon, Aug 8, 2011 at 12:49 PM, Alvaro Herrera alvhe...@commandprompt.com wrote: Excerpts from Robert Haas's message of lun ago 08 12:33:45 -0400 2011: On Mon, Aug 8, 2011 at 12:22 PM, Alvaro Herrera alvhe...@commandprompt.com wrote: Excerpts from Robert Haas's message of lun ago 08 12:05:05 -0400 2011: We could do that, but what the heck is the point? What benefit are we trying to get by not returning a pointer to the structure? I feel like we're making this ludicrously complicated with no real justification of why all of this complexity is adding any value. I don't see that it's complicated in any way. Seems pretty simple to me. It's quite complicated. Every time someone adds a new member to the struct, they need to adjust the code all over the place. The call sequence of the accessor function will constantly be changing. You are rehashing an argument that I already rebutted: the way to solve this problem is with the macros, the format of which we were discussing in this sub-thread. I am not sure why you bring it up again. Well, the proposed macro design seems like it doesn't go as far in that direction as I would have hoped. I thought you were proposing to have one function that returns a pointer to the struct, and then a bunch of macros that return particular elements out of the returned struct. If we did that, the macro definitions wouldn't need to change when you added new members to the struct. On the other hand, if you have one function with N out parameters, and the macros are just calling that function with mostly NULL arguments, then the calls have to be updated every time you add a new struct member. IMHO, that's a pain. YMMV. You say the reason for not exporting the struct definition is that you can later change it without having to touch the callers. That seems a good argument, and it seems to me to work equally for pg_amop as for the new header KaiGai is creating here. In the case of pg_amop, there's an additional thing to worry about, which is that the result is allocated in some memory context, and you now have to worry about how long it sticks around in that context vs. how long the caller needs it for. Copying the data into out parameters (or a pg_amop struct in the caller's memory context, if we had preferred to go that route) addresses that problem. However, here we're just dealing with static data, so that's not a problem. Also, if get_op_opfamily_properties() had as many out parameters as this function would likely need to, I'd favor changing that, too. Three is reasonable. Ten is pushing it, and any more than that is kind of nuts. I think we're going to end up with quite a few in this case, because we're rapidly accreting a set of operations that work on any old SQL object (COMMENT, SECURITY LABEL, ALTER EXTENSION .. ADD/DROP, ALTER FOO .. SET SCHEMA, etc.). I think that's a good thing, but I don't want to assume that the number of struct members is going to remain small. In any case, at the start of the thread KaiGai was asking for opinions, and I gave mine. I will now shut up. Sorry to have given offense. My goal was to understand why we were having this argument, not to piss you off. However, I seem to have failed. -- 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] [RFC] Common object property boards
So add a bunch of macros on top for the two or three (five?) most common
cases -- say those that occur 3 times or more.
I could go for that.
OK, I'll try to implement according to the idea.
I'm under implementation of this code according to the suggestion.
However, I'm not sure whether it is really portable way (at least, GCC accepts),
and whether the interface is simpler than as Robert suggested at first.
extern void get_object_property(ObjectType objtype,
const char **objtype_text,
Oid *relationId,
int *catid_oidlookup,
int *catid_namelookup,
AttrNumber *attnum_name,
AttrNumber *attnum_namespace,
AttrNumber *attnum_owner);
#define get_object_property_attnum_name(objtype)\
({ AttrNumber temp;\
get_object_property((objtype), NULL, NULL, NULL, NULL, \
temp, NULL, NULL); \
temp; })
#define get_object_property_attnum_namespace(objtype) \
({ AttrNumber temp;\
get_object_property((objtype), NULL, NULL, NULL, NULL, \
NULL, temp, NULL); \
temp; })
#define get_object_property_attnum_ownership(objtype) \
({ AttrNumber temp;\
get_object_property((objtype), NULL, NULL, NULL, NULL, \
NULL, NULL, temp); \
temp; })
If get_object_property() returns an entry within the big property table,
the bunch of macros will become simple, as follows:
extern ObjectProperty get_object_property(ObjectType objtype);
#define get_object_property_attnum_name(objtype) \
(get_object_property(objtype)-attnum_name)
2011/8/2 Kohei KaiGai kai...@kaigai.gr.jp:
2011/8/2 Robert Haas robertmh...@gmail.com:
On Mon, Aug 1, 2011 at 4:27 PM, Alvaro Herrera
alvhe...@commandprompt.com wrote:
Excerpts from Robert Haas's message of lun ago 01 16:12:56 -0400 2011:
On Mon, Aug 1, 2011 at 4:02 PM, Alvaro Herrera
alvhe...@commandprompt.com wrote:
Excerpts from Kohei KaiGai's message of dom jul 31 02:21:55 -0400 2011:
2011/7/29 Tom Lane t...@sss.pgh.pa.us:
It would likely be better to not expose the struct type, just
individual
lookup functions.
If so, individual functions to expose a certain property of the supplied
object type should be provided.
int get_object_property_catid_oidlookup(ObjectType);
int get_object_property_catid_namelookup(ObjectType);
Oid get_object_property_relation_id(ObjectType);
AttrNumber get_object_property_nameattnum(ObjectType);
AttrNumber get_object_property_namespacenum(ObjectType);
AttrNumber get_object_property_ownershipnum(ObjectType);
Maybe a single lookup function that receives pointers that the lookup
routine can fill with the appropriate information; allowing for a NULL
pointer in each, meaning caller is not interested in that property.
That seems like a lot of extra notational complexity for no particular
benefit. Every time someone wants to add a new property to this
array, they're going to have to touch every caller, and all
third-party code using this interface will have to be rejiggered.
So add a bunch of macros on top for the two or three (five?) most common
cases -- say those that occur 3 times or more.
I could go for that.
OK, I'll try to implement according to the idea.
Thanks,
--
KaiGai Kohei kai...@kaigai.gr.jp
--
KaiGai Kohei kai...@kaigai.gr.jp
--
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] [RFC] Common object property boards
Kohei KaiGai kai...@kaigai.gr.jp writes:
I'm under implementation of this code according to the suggestion.
However, I'm not sure whether it is really portable way (at least, GCC
accepts),
and whether the interface is simpler than as Robert suggested at first.
#define get_object_property_attnum_name(objtype)\
({ AttrNumber temp;\
get_object_property((objtype), NULL, NULL, NULL, NULL, \
temp, NULL, NULL); \
temp; })
Blocks within expressions are a gcc-ism and will fail on any other
compiler, so you can't do it that way.
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] [RFC] Common object property boards
2011/8/2 Robert Haas robertmh...@gmail.com: On Mon, Aug 1, 2011 at 4:27 PM, Alvaro Herrera alvhe...@commandprompt.com wrote: Excerpts from Robert Haas's message of lun ago 01 16:12:56 -0400 2011: On Mon, Aug 1, 2011 at 4:02 PM, Alvaro Herrera alvhe...@commandprompt.com wrote: Excerpts from Kohei KaiGai's message of dom jul 31 02:21:55 -0400 2011: 2011/7/29 Tom Lane t...@sss.pgh.pa.us: It would likely be better to not expose the struct type, just individual lookup functions. If so, individual functions to expose a certain property of the supplied object type should be provided. int get_object_property_catid_oidlookup(ObjectType); int get_object_property_catid_namelookup(ObjectType); Oid get_object_property_relation_id(ObjectType); AttrNumber get_object_property_nameattnum(ObjectType); AttrNumber get_object_property_namespacenum(ObjectType); AttrNumber get_object_property_ownershipnum(ObjectType); Maybe a single lookup function that receives pointers that the lookup routine can fill with the appropriate information; allowing for a NULL pointer in each, meaning caller is not interested in that property. That seems like a lot of extra notational complexity for no particular benefit. Every time someone wants to add a new property to this array, they're going to have to touch every caller, and all third-party code using this interface will have to be rejiggered. So add a bunch of macros on top for the two or three (five?) most common cases -- say those that occur 3 times or more. I could go for that. OK, I'll try to implement according to the idea. Thanks, -- KaiGai Kohei kai...@kaigai.gr.jp -- 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] [RFC] Common object property boards
Excerpts from Kohei KaiGai's message of dom jul 31 02:21:55 -0400 2011: 2011/7/29 Tom Lane t...@sss.pgh.pa.us: It would likely be better to not expose the struct type, just individual lookup functions. If so, individual functions to expose a certain property of the supplied object type should be provided. int get_object_property_catid_oidlookup(ObjectType); int get_object_property_catid_namelookup(ObjectType); Oid get_object_property_relation_id(ObjectType); AttrNumber get_object_property_nameattnum(ObjectType); AttrNumber get_object_property_namespacenum(ObjectType); AttrNumber get_object_property_ownershipnum(ObjectType); Maybe a single lookup function that receives pointers that the lookup routine can fill with the appropriate information; allowing for a NULL pointer in each, meaning caller is not interested in that property. -- Á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] [RFC] Common object property boards
On Mon, Aug 1, 2011 at 4:02 PM, Alvaro Herrera alvhe...@commandprompt.com wrote: Excerpts from Kohei KaiGai's message of dom jul 31 02:21:55 -0400 2011: 2011/7/29 Tom Lane t...@sss.pgh.pa.us: It would likely be better to not expose the struct type, just individual lookup functions. If so, individual functions to expose a certain property of the supplied object type should be provided. int get_object_property_catid_oidlookup(ObjectType); int get_object_property_catid_namelookup(ObjectType); Oid get_object_property_relation_id(ObjectType); AttrNumber get_object_property_nameattnum(ObjectType); AttrNumber get_object_property_namespacenum(ObjectType); AttrNumber get_object_property_ownershipnum(ObjectType); Maybe a single lookup function that receives pointers that the lookup routine can fill with the appropriate information; allowing for a NULL pointer in each, meaning caller is not interested in that property. That seems like a lot of extra notational complexity for no particular benefit. Every time someone wants to add a new property to this array, they're going to have to touch every caller, and all third-party code using this interface will have to be rejiggered. I still think that just returning a pointer to the struct is a perfectly sensible API... -- 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] [RFC] Common object property boards
Excerpts from Robert Haas's message of lun ago 01 16:12:56 -0400 2011: On Mon, Aug 1, 2011 at 4:02 PM, Alvaro Herrera alvhe...@commandprompt.com wrote: Excerpts from Kohei KaiGai's message of dom jul 31 02:21:55 -0400 2011: 2011/7/29 Tom Lane t...@sss.pgh.pa.us: It would likely be better to not expose the struct type, just individual lookup functions. If so, individual functions to expose a certain property of the supplied object type should be provided. int get_object_property_catid_oidlookup(ObjectType); int get_object_property_catid_namelookup(ObjectType); Oid get_object_property_relation_id(ObjectType); AttrNumber get_object_property_nameattnum(ObjectType); AttrNumber get_object_property_namespacenum(ObjectType); AttrNumber get_object_property_ownershipnum(ObjectType); Maybe a single lookup function that receives pointers that the lookup routine can fill with the appropriate information; allowing for a NULL pointer in each, meaning caller is not interested in that property. That seems like a lot of extra notational complexity for no particular benefit. Every time someone wants to add a new property to this array, they're going to have to touch every caller, and all third-party code using this interface will have to be rejiggered. So add a bunch of macros on top for the two or three (five?) most common cases -- say those that occur 3 times or more. -- Á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] [RFC] Common object property boards
On Mon, Aug 1, 2011 at 4:27 PM, Alvaro Herrera alvhe...@commandprompt.com wrote: Excerpts from Robert Haas's message of lun ago 01 16:12:56 -0400 2011: On Mon, Aug 1, 2011 at 4:02 PM, Alvaro Herrera alvhe...@commandprompt.com wrote: Excerpts from Kohei KaiGai's message of dom jul 31 02:21:55 -0400 2011: 2011/7/29 Tom Lane t...@sss.pgh.pa.us: It would likely be better to not expose the struct type, just individual lookup functions. If so, individual functions to expose a certain property of the supplied object type should be provided. int get_object_property_catid_oidlookup(ObjectType); int get_object_property_catid_namelookup(ObjectType); Oid get_object_property_relation_id(ObjectType); AttrNumber get_object_property_nameattnum(ObjectType); AttrNumber get_object_property_namespacenum(ObjectType); AttrNumber get_object_property_ownershipnum(ObjectType); Maybe a single lookup function that receives pointers that the lookup routine can fill with the appropriate information; allowing for a NULL pointer in each, meaning caller is not interested in that property. That seems like a lot of extra notational complexity for no particular benefit. Every time someone wants to add a new property to this array, they're going to have to touch every caller, and all third-party code using this interface will have to be rejiggered. So add a bunch of macros on top for the two or three (five?) most common cases -- say those that occur 3 times or more. I could go for that. -- 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] [RFC] Common object property boards
2011/7/29 Tom Lane t...@sss.pgh.pa.us: Kohei Kaigai kohei.kai...@emea.nec.com writes: In addition to this suggestion, I think the big static array also contains the following items: - Text form of the object type (e.g, table, function, ...) What will you do with that that wouldn't be better done by calling getObjectDescription? The latter's output is somewhat localizable, but individual words would be hard to translate. The getObjectDescription() is good for existing object, but we cannot use this interface to generate error messages of not exist object. Does the main lookup function ought to return an entry of the big array? If so, the definition of structure should be declared in objectaddress.h, as follows: It would likely be better to not expose the struct type, just individual lookup functions. If so, individual functions to expose a certain property of the supplied object type should be provided. int get_object_property_catid_oidlookup(ObjectType); int get_object_property_catid_namelookup(ObjectType); Oid get_object_property_relation_id(ObjectType); AttrNumber get_object_property_nameattnum(ObjectType); AttrNumber get_object_property_namespacenum(ObjectType); AttrNumber get_object_property_ownershipnum(ObjectType); I'm not a fun to invoke these functions more than once in a certain step. For example, AlterObjectNamespace() wants to know attribute number of name, namespace and ownership. In addition, it also want cache-id of oid-loopup and name-lookup. Thus, it takes 5 times invocation of these function, rather than one lookup for array. Which is more preferable for programmer? And, a translation from ObjectType to type name (e.g table, type, ...) is helpful to generate error messages. const char *get_object_type_name(ObjectType objtype); Again, I think this is too low level because of message translation considerations. OK. Thanks, -- KaiGai Kohei kai...@kaigai.gr.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] [RFC] Common object property boards
Robert Haas wrote: | I think that get_object_namespace() needs to be rethought. If you | take a look at AlterObjectNamespace() and its callers, you'll notice | that we already have, encoded in those call sites, the knowledge of | which object types can be looked up in which system caches, and which | columns within the resulting heap tuples will contain the schema OIDs. | Here, we're essentially duplicating that information in another | place, which doesn't seem good. I think perhaps we should create a | big static array, each row of which would contain: | | - ObjectType | - system cache ID for OID lookups | - system catalog table OID for scans | - attribute number for the name attribute, where applicable (see | AlterObjectNamespace) | - attribute number for the namespace attribute | - attribute number for the owner attribute | - ...and maybe some other properties | | We could then create a function (in objectaddress.c, probably) that | you call with an ObjectType argument, and it returns a pointer to the | appropriate struct entry, or calls elog() if none is found. This | function could be used by the existing object_exists() functions in | lieu of having its own giant switch statement, by | AlterObjectNamespace(), and by this new consolidated DROP code. If | you agree with this approach, we should probably make it a separate | patch. | According to the suggestion from Robert, I'd like to see opinions about this feature prior to its implementation. In addition to this suggestion, I think the big static array also contains the following items: - Text form of the object type (e.g, table, function, ...) - Function pointer of validator. It shall raise an error when relkind is not RELKIND_RELATION for ObjectType being OBJECT_TABLE, for example. Does the main lookup function ought to return an entry of the big array? If so, the definition of structure should be declared in objectaddress.h, as follows: ObjectProperty get_object_property(ObjectType objtype); In addition to the big array indexed by ObjectType, I'd like to have a utility function that translate ObjectAddress to ObjectType. (A part of them need syscache lookup, because pg_class contains several objcts types: table, view, ...) ObjectType get_object_type(const ObjectAddress *address); If and when we have both of the function to reference ObjectProperty and to translate ObjectAddress to ObjectType, it is quite easy to implement service routines to lookup namespaceId, ownerId and others for the supplied ObjectAddress, as follows: Oid get_object_namespace(const ObjectAddress *address); Oid get_object_ownership(const ObjectAddress *address); char *get_object_name(const ObjectAddress *address); HeapTuple get_object_tuple(const ObjectAddress *address); And, a translation from ObjectType to type name (e.g table, type, ...) is helpful to generate error messages. const char *get_object_type_name(ObjectType objtype); Any comments please. Thanks, -- NEC Europe Ltd, SAP Global Competence Center KaiGai Kohei kohei.kai...@emea.nec.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] [RFC] Common object property boards
Kohei Kaigai kohei.kai...@emea.nec.com writes: In addition to this suggestion, I think the big static array also contains the following items: - Text form of the object type (e.g, table, function, ...) What will you do with that that wouldn't be better done by calling getObjectDescription? The latter's output is somewhat localizable, but individual words would be hard to translate. Does the main lookup function ought to return an entry of the big array? If so, the definition of structure should be declared in objectaddress.h, as follows: It would likely be better to not expose the struct type, just individual lookup functions. And, a translation from ObjectType to type name (e.g table, type, ...) is helpful to generate error messages. const char *get_object_type_name(ObjectType objtype); Again, I think this is too low level because of message translation considerations. 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] [RFC] Common object property boards
On Fri, Jul 29, 2011 at 1:49 PM, Tom Lane t...@sss.pgh.pa.us wrote: It would likely be better to not expose the struct type, just individual lookup functions. I'm not sure about that... I think that's just going to introduce a lot of excess notation. And, a translation from ObjectType to type name (e.g table, type, ...) is helpful to generate error messages. const char *get_object_type_name(ObjectType objtype); Again, I think this is too low level because of message translation considerations. On this point I agree. -- 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
