Re: [HACKERS] [PATCH] Store Extension Options
On Thu, Mar 13, 2014 at 5:35 PM, Robert Haas robertmh...@gmail.com wrote: Well, I'm not sure that's really any big deal, but I'm not wedded to the label idea. My principal concern is: I'm opposed to allowing unvalidated options into the database. I think it should be a requirement that if the validator can't be found and called, then the reloption is no good and you just reject it. So, if we go with the reloptions route, I'd want to see pg_register_option_namespace go away in favor of some solution that preserves that property. One thing I kind of like about the LABEL approach is that it applies to virtually every object type, meaning that we might not have to repeat this discussion when (as seems inevitable) people want to be able to do things to schemas or tablespaces or roles. But I don't hold that position so strongly as to be unwilling to entertain any other options. During the last days I thought about this discussion and to use SECLABELs sounds weird to me. Here in Brazil we call this kind of thing 'gambiarra'. Because we'll try to use something that born with a very well defined purpose to another purpose. Personally I don't like that. If we think more about SECLABELs, in a more abstract way, it is just a 'property' about database objects. And the same is COMMENTs. Both SECLABEL and COMMENT provide a way to store something about objects. Maybe we can think about a new object on top of COMMENT and SECLABELs. An object called 'PROPERTY' or 'OPTION'. And COMMENTs and SECLABELs can be just a kind of this new object, and we must introduce a new kind callled 'CUSTOM'. I thought about this because representation (syntax) of SECLABELs and COMMENTs are similar. They describe something about objects, they have local and shared catalog. This is just something that occurred to me. Maybe I'm completely wrong. Or not! Comments? -- Fabrízio de Royes Mello Consultoria/Coaching PostgreSQL Timbira: http://www.timbira.com.br Blog sobre TI: http://fabriziomello.blogspot.com Perfil Linkedin: http://br.linkedin.com/in/fabriziomello Twitter: http://twitter.com/fabriziomello
Re: [HACKERS] [PATCH] Store Extension Options
* Tom Lane (t...@sss.pgh.pa.us) wrote: I don't really think partial validation makes sense. We could just remove the whole topic, and tell extension authors that it's up to them to defend themselves against bizarre values stored for their table options. But I'm wondering if there's really so much use-case for a feature like that. While I agree that validation would be a good thing to have, if we can figure out a way to make it work, I don't really see why that has a huge bearing on the use-cases for this feature overall. There's clearly a bunch of use-cases for I need to add a bit of meta-data, for my own needs, about this table. Nasby is doing what Robert was originally advocating (having an independent table-of-tables) and rightfully pointed out that it basically sucks. I feel like a lot of this has to do with reloptions being in some way/shape/form viewed as ours (as in, belongs to -core). I can get behind that idea, but it doesn't solve the use-case. The whole discussion around validation is interesting but it would also eliminate a bunch of natural use-cases as not everyone will want to build an extension or write C code just to have a place to store this extra meta-data (and indeed- we'd probably just end up with someone implementing a custom_reloptions extension which just allowed anything). In the end, perhaps we should just add another field which is called 'custom_reloptions' and allow that to be the wild west? With a few recommendations that extension authors use a prefix of some kind and that individual DBAs use either no-namespace, or one which isn't likely to conflict with real extensions. That would also avoid any possible conflict with what we want to do in core later on. As for dealing with extensions which migrate to core, we might be able to teach pg_dump's binary upgrade about that, and be able to migrate any custom_reloptions which were for the independent extension into the 'core' reloptions, or we could just punt on it and tell people they'll need to re-set the options or use whatever the new DDL is, or perhaps we'll update the extension to just pass through the options. In any case, that strikes me as a solveable problem, particularly if they're independent fields. Perhaps one other option would be to add a new field which is the 'wild west' but then allow extensions to add to reloptions w/ appropriate validation, but I'm not sure that it's really necessary. Extensions should be able to validate the value when they go to use it for whatever they need it for and complain if they don't understand it. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] [PATCH] Store Extension Options
On Thu, Mar 13, 2014 at 12:47 AM, Simon Riggs si...@2ndquadrant.com wrote: On 13 March 2014 02:14, Robert Haas robertmh...@gmail.com wrote: I'm not sure why this is being blocked. This is a community contribution that seeks to improve everybody's options. Blocking it does *nothing* to prevent individual extensions from providing table-level options - we give them freedom to do whatever the hell they want. Validation is a pipe dream, not *ever* an achievable reality. Blocking is just exercise of a veto for nobody's gain. Unsurprisingly, I don't agree with any of that. The point is that execising a veto here is irrelevant. Blocking this patch does *nothing* to prevent extensions from adopting per-table options. All that is happening is that a single, coherent mechanism for such options is being blocked. Blocking this is like trying to block rain. We can all pretend the blocking viewpoint has succeeded, but all it does is to bring Postgres core into disrepute. I have often heard that from others that this is a business opportunity, not a problem. If that is true, its not because we didn't try to act for the good of all. It is very true that there are other ways for extensions to manage per-table options. In my mind, that's another reason NOT to throw open the door to unrestricted use of reloptions to store whatever anyone wants to throw in there, but rather to wait until we have a sound and well-thought-out design that we're comfortable with our ability to support and extend into the indefinite future. The bottom line here is that, as in previous years, there are a certain number of people who show up near the end of CF4 and are unhappy that some patch didn't get committed. Generally, they allege that (1) there's nothing wrong with the patch, (2) if there is something wrong with the patch, then it's the fault of the people objecting for not volunteering to fix it, and (3) that if the patch isn't committed despite the objections raised, it's going to be hideously bad for PostgreSQL. Josh Berkus chose to put his version of this rant on his blog: http://www.databasesoup.com/2014/02/why-hstore2jsonb-is-most-important.html But the reality is that most of the patches we reject are in my opinion rejected for good reasons (though some are rejected for bad reasons); that most of the ones that really matter emerge for a later release in greatly improved form; and that the product is better overall of for those delays. Because on projects where people are quick to commit irrevocably to insufficiently-scrutinized design decisions, huge amounts of time and energy get spent digging out from under those bad decisions; or else nobody fixes it and the product just stinks. So, in my opinion, the time and care that we take to get things right is a feature, not a bug. Your mileage may, of course, vary. -- 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] [PATCH] Store Extension Options
On 2014-03-13 09:17:36 -0400, Robert Haas wrote: It is very true that there are other ways for extensions to manage per-table options. You previously said that, but I really don't see any. Which way out there exists that a) doesn't leave garbage after the relation is dropped or renamed b) is properly dumped by pg_dump c) is properly integratable with cache invalidations. c) is hackable by manually sending cache invalidations from C code when changing the associated information, and by using a relcache callback for cache invalidation, but the others really aren't solveable right now afaics. The bottom line here is that, as in previous years, there are a certain number of people who show up near the end of CF4 and are unhappy that some patch didn't get committed. Generally, they allege that (1) there's nothing wrong with the patch, (2) if there is something wrong with the patch, then it's the fault of the people objecting for not volunteering to fix it, and (3) that if the patch isn't committed despite the objections raised, it's going to be hideously bad for PostgreSQL. I agree that this happens occasionally, but I don't really see evidence of it in this case. We seem to be discussing the merit of the patch itself, not the scheduling of a eventual commit. 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] [PATCH] Store Extension Options
On Thu, Mar 13, 2014 at 9:26 AM, Andres Freund and...@2ndquadrant.com wrote: On 2014-03-13 09:17:36 -0400, Robert Haas wrote: It is very true that there are other ways for extensions to manage per-table options. You previously said that, but I really don't see any. Which way out there exists that a) doesn't leave garbage after the relation is dropped or renamed b) is properly dumped by pg_dump c) is properly integratable with cache invalidations. c) is hackable by manually sending cache invalidations from C code when changing the associated information, and by using a relcache callback for cache invalidation, but the others really aren't solveable right now afaics. Well, I'm not going to claim that the methods that exist today are perfect. Things you can do include: (1) the table of tables approach, (2) abusing comments, and perhaps (3) abusing the security label machinery. SECURITY LABEL FOR bdr ON TABLE copy_me IS 'yes, please'? Only the first of those fails prong (a) of your proposed requirements, and they all pass prong (b). I'm not totally sure how well comments and security labels integrate with cache invalidation. An interesting point here is that the SECURITY LABEL functionality is arguably exactly what is wanted here, except for the name of the command. Tables (and almost every other type of object in the system, including columns, functions, etc.) can have an arbitrary number of security labels, each of which must be managed by a separate provider, which gets to validate those options at the time they're applied. Of course, the provider can simply choose to accept everything, if it wants. Dump-and-reload is handled by assuming that you need to have the applicable providers present at reload time (or ignore the errors you get when restoring the dump, or edit the dump). And an interesting point is that the SECURITY LABEL feature has been around since 9.1 and we've had zero complaints about the design. This either means that the design is excellent, or very few people have tried to use it for anything. But I think it would be worth considering to what extent that design (modulo the name) also meets the requirements here. Because it works on all object types, it's actually quite a bit more general than this proposal. And it wouldn't be very hard to drop the word SECURITY from the command and just let objects have labels. (We could even introduce introduce alternate syntax, like ALTER object-type object-name SET LABEL FOR provider TO value, if that makes things nicer, though the confusion of having two completely different syntaxes might not be worth it.) On the other hand, if that design *doesn't* meet the requirements here, then it would be good to know why. What I think we certainly don't want to do is invent a very similar mechanism to what already exists, but with a slightly different set of warts. -- 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] [PATCH] Store Extension Options
On 2014-03-13 10:03:03 -0400, Robert Haas wrote:
On Thu, Mar 13, 2014 at 9:26 AM, Andres Freund and...@2ndquadrant.com wrote:
On 2014-03-13 09:17:36 -0400, Robert Haas wrote:
It is very true that there are other ways for extensions to manage
per-table options.
You previously said that, but I really don't see any. Which way out
there exists that a) doesn't leave garbage after the relation is dropped
or renamed b) is properly dumped by pg_dump c) is properly integratable
with cache invalidations.
c) is hackable by manually sending cache invalidations from C code when
changing the associated information, and by using a relcache callback
for cache invalidation, but the others really aren't solveable right now
afaics.
Well, I'm not going to claim that the methods that exist today are
perfect. Things you can do include: (1) the table of tables approach,
(2) abusing comments, and perhaps (3) abusing the security label
machinery. SECURITY LABEL FOR bdr ON TABLE copy_me IS 'yes, please'?
Only the first of those fails prong (a) of your proposed requirements,
and they all pass prong (b). I'm not totally sure how well comments
and security labels integrate with cache invalidation.
The table of table fall short of all of those, so it's pretty much
unusable. Comments aren't usable because there's no way to coordinate
between various users of the facility and it breaks their original
usage. They also don't produce cache invalidations.
But security labels are a nice idea, will think about it. AFAICs there's
no builtin subdivision within the label for one provider which is a bit
of a shame but solvable. The biggest issue I see is that it essentially
seems to require that the provider is in
{shared,local}_preload_libraries? You can't restore into a server
otherwise afaics?
They currently don't seem to create invalidations on the objects they
are set upon, maybe we should change that? There seems to be pretty
little reason to avoid that, the frequence of change really should never
be high enough for it to be problematic.
And an interesting point is that the SECURITY LABEL feature has been
around since 9.1 and we've had zero complaints about the design. This
either means that the design is excellent, or very few people have
tried to use it for anything.
Without saying that its design is bad, I am pretty sure it's because
it's basically unused.
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] [PATCH] Store Extension Options
On 13 March 2014 13:17, Robert Haas robertmh...@gmail.com wrote: The bottom line here is that, as in previous years, there are a certain number of people who show up near the end of CF4 and are unhappy that some patch didn't get committed. Generally, they allege that (1) there's nothing wrong with the patch, (2) if there is something wrong with the patch, then it's the fault of the people objecting for not volunteering to fix it, and (3) that if the patch isn't committed despite the objections raised, it's going to be hideously bad for PostgreSQL. Josh Berkus chose to put his version of this rant on his blog: An interesting twist. 1) It's a simple patch and could be committed. Claiming otherwise would not be accurate. 2) Nobody has said it's the fault of the people objecting for not volunteering to fix it 3) As I explained twice already, *not* committing the patch does *nothing* to prevent extension writers from making up their own mechanism, so blocking the patch does nothing. Writing the extra code required takes a while, but frankly its quicker than pointless arguing. PostgreSQL will not explode if this patch is blocked, nor will it explode if we allow unvalidated options. Hmm, so actually none of those points stick. Perhaps we're talking about another patch that you think should be rejected? Not sure. -- Simon Riggs 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] [PATCH] Store Extension Options
[ forgot to respond to this part ] Andres Freund and...@2ndquadrant.com writes: They currently don't seem to create invalidations on the objects they are set upon, maybe we should change that? No, because relcache doesn't store security labels to start with. There's a separate catalog cache for security labels, I believe, and invalidating entries in that ought to be sufficient. 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] [PATCH] Store Extension Options
On 2014-03-13 10:24:09 -0400, Tom Lane wrote:
Andres Freund and...@2ndquadrant.com writes:
But security labels are a nice idea, will think about it. AFAICs there's
no builtin subdivision within the label for one provider which is a bit
of a shame but solvable. The biggest issue I see is that it essentially
seems to require that the provider is in
{shared,local}_preload_libraries? You can't restore into a server
otherwise afaics?
Well, if you want to validate the settings then you pretty much have to
require that in some form.
If there were a CREATE SECURITY LABEL PROVIDER or something, with the
catalog pointing to a validator function, we wouldn't necessarily...
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] [PATCH] Store Extension Options
On Thu, Mar 13, 2014 at 10:20 AM, Andres Freund and...@2ndquadrant.com wrote:
Well, I'm not going to claim that the methods that exist today are
perfect. Things you can do include: (1) the table of tables approach,
(2) abusing comments, and perhaps (3) abusing the security label
machinery. SECURITY LABEL FOR bdr ON TABLE copy_me IS 'yes, please'?
Only the first of those fails prong (a) of your proposed requirements,
and they all pass prong (b). I'm not totally sure how well comments
and security labels integrate with cache invalidation.
The table of table fall short of all of those, so it's pretty much
unusable. Comments aren't usable because there's no way to coordinate
between various users of the facility and it breaks their original
usage. They also don't produce cache invalidations.
But security labels are a nice idea, will think about it. AFAICs there's
no builtin subdivision within the label for one provider which is a bit
of a shame but solvable.
Why do we need that? Are we really going to have so many names here
that a simple convention that an extension providing multiple names
should prefix each one with $EXTENSION_NAME + _ is insufficient?
The biggest issue I see is that it essentially
seems to require that the provider is in
{shared,local}_preload_libraries? You can't restore into a server
otherwise afaics?
Correct.
They currently don't seem to create invalidations on the objects they
are set upon, maybe we should change that? There seems to be pretty
little reason to avoid that, the frequence of change really should never
be high enough for it to be problematic.
No objection.
And an interesting point is that the SECURITY LABEL feature has been
around since 9.1 and we've had zero complaints about the design. This
either means that the design is excellent, or very few people have
tried to use it for anything.
Without saying that its design is bad, I am pretty sure it's because
it's basically unused.
Sure, that's my bet as well. I think the really interesting question
here is how the dump-and-reload issue ought to be handled. As Tom
says, it seems on the surface as though you can either require that
the provider be loaded for that, or you can accept unvalidated
settings. Between those, my vote is for the first, because I think
that extensions are not likely to want to have to deal at runtime with
the possibility of having arbitrary values where they expect values
from a fixed list.
Basically, my feeling is that if you install an extension that adds
new table-level options, that's effectively a new version of the
database, and expecting a dump from that version to restore into a
vanilla database is about as reasonable as expecting 9.4 dumps to
restore flawlessly on 8.4.
--
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] [PATCH] Store Extension Options
On 13 March 2014 13:17, Stephen Frost sfr...@snowman.net wrote: In the end, perhaps we should just add another field which is called 'custom_reloptions' and allow that to be the wild west? That makes sense. ... and allow that to be the wild west? but that would be an emotive phrase that doesn't help acceptance. As you say, this is just metadata. We have no reason to believe that a DBA would be less careful with metadata than they are with their data. We trust them to design their own tables and fill them with data. I figure we can trust them with options metadata too. -- Simon Riggs 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] [PATCH] Store Extension Options
Andres Freund and...@2ndquadrant.com writes:
But security labels are a nice idea, will think about it. AFAICs there's
no builtin subdivision within the label for one provider which is a bit
of a shame but solvable. The biggest issue I see is that it essentially
seems to require that the provider is in
{shared,local}_preload_libraries? You can't restore into a server
otherwise afaics?
Well, if you want to validate the settings then you pretty much have to
require that in some form.
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] [PATCH] Store Extension Options
On 13 March 2014 14:03, Robert Haas robertmh...@gmail.com wrote: On Thu, Mar 13, 2014 at 9:26 AM, Andres Freund and...@2ndquadrant.com wrote: On 2014-03-13 09:17:36 -0400, Robert Haas wrote: It is very true that there are other ways for extensions to manage per-table options. You previously said that, but I really don't see any. Which way out there exists that a) doesn't leave garbage after the relation is dropped or renamed b) is properly dumped by pg_dump c) is properly integratable with cache invalidations. c) is hackable by manually sending cache invalidations from C code when changing the associated information, and by using a relcache callback for cache invalidation, but the others really aren't solveable right now afaics. Well, I'm not going to claim that the methods that exist today are perfect. Things you can do include: (1) the table of tables approach, (2) abusing comments, and perhaps (3) abusing the security label machinery. SECURITY LABEL FOR bdr ON TABLE copy_me IS 'yes, please'? Only the first of those fails prong (a) of your proposed requirements, and they all pass prong (b). I'm not totally sure how well comments and security labels integrate with cache invalidation. An interesting point here is that the SECURITY LABEL functionality is arguably exactly what is wanted here, except for the name of the command. Tables (and almost every other type of object in the system, including columns, functions, etc.) can have an arbitrary number of security labels, each of which must be managed by a separate provider, which gets to validate those options at the time they're applied. Of course, the provider can simply choose to accept everything, if it wants. Dump-and-reload is handled by assuming that you need to have the applicable providers present at reload time (or ignore the errors you get when restoring the dump, or edit the dump). And an interesting point is that the SECURITY LABEL feature has been around since 9.1 and we've had zero complaints about the design. This either means that the design is excellent, or very few people have tried to use it for anything. But I think it would be worth considering to what extent that design (modulo the name) also meets the requirements here. Because it works on all object types, it's actually quite a bit more general than this proposal. And it wouldn't be very hard to drop the word SECURITY from the command and just let objects have labels. (We could even introduce introduce alternate syntax, like ALTER object-type object-name SET LABEL FOR provider TO value, if that makes things nicer, though the confusion of having two completely different syntaxes might not be worth it.) I like that suggestion, all of it. Perhaps change it to METADATA LABEL ? On the other hand, if that design *doesn't* meet the requirements here, then it would be good to know why. What I think we certainly don't want to do is invent a very similar mechanism to what already exists, but with a slightly different set of warts. -- Simon Riggs 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] [PATCH] Store Extension Options
On Thu, Mar 13, 2014 at 10:22 AM, Simon Riggs si...@2ndquadrant.com wrote: On 13 March 2014 13:17, Robert Haas robertmh...@gmail.com wrote: The bottom line here is that, as in previous years, there are a certain number of people who show up near the end of CF4 and are unhappy that some patch didn't get committed. Generally, they allege that (1) there's nothing wrong with the patch, (2) if there is something wrong with the patch, then it's the fault of the people objecting for not volunteering to fix it, and (3) that if the patch isn't committed despite the objections raised, it's going to be hideously bad for PostgreSQL. Josh Berkus chose to put his version of this rant on his blog: An interesting twist. 1) It's a simple patch and could be committed. Claiming otherwise would not be accurate. 2) Nobody has said it's the fault of the people objecting for not volunteering to fix it 3) As I explained twice already, *not* committing the patch does *nothing* to prevent extension writers from making up their own mechanism, so blocking the patch does nothing. Writing the extra code required takes a while, but frankly its quicker than pointless arguing. PostgreSQL will not explode if this patch is blocked, nor will it explode if we allow unvalidated options. Hmm, so actually none of those points stick. Perhaps we're talking about another patch that you think should be rejected? Not sure. Well, I'm *trying* to talk about the fact that I think that any machinery that allows custom reloptions (or their equivalent) should also support mandatory validation. I think this subthread is somehow getting sidetracked from the meat of that conversation. -- 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] [PATCH] Store Extension Options
Robert Haas escribió: Basically, my feeling is that if you install an extension that adds new table-level options, that's effectively a new version of the database, and expecting a dump from that version to restore into a vanilla database is about as reasonable as expecting 9.4 dumps to restore flawlessly on 8.4. This seems a very reasonable principle to me. -- Álvaro Herrerahttp://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] [PATCH] Store Extension Options
On 2014-03-13 10:26:11 -0400, Tom Lane wrote: [ forgot to respond to this part ] Andres Freund and...@2ndquadrant.com writes: They currently don't seem to create invalidations on the objects they are set upon, maybe we should change that? No, because relcache doesn't store security labels to start with. There's a separate catalog cache for security labels, I believe, and invalidating entries in that ought to be sufficient. There doesn't seem to be any form of system managed cache for security labels afaics. Every lookup does a index scan. I currently don't see how I could build a cache in userland that'd invalidate if either a) the underlying object changes b) the label changes. I don't have a better idea than triggering invalidations on the respective underlying object. If you have one... 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] [PATCH] Store Extension Options
On 13 March 2014 14:36, Simon Riggs si...@2ndquadrant.com wrote: I like that suggestion, all of it. Perhaps change it to METADATA LABEL ? Damn. It works, apart from the fact that we don't get parameter=value. That may not be critical, since most use cases I can think of are booleans. -- Simon Riggs 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] [PATCH] Store Extension Options
On 2014-03-13 10:31:12 -0400, Robert Haas wrote: I think the really interesting question here is how the dump-and-reload issue ought to be handled. As Tom says, it seems on the surface as though you can either require that the provider be loaded for that, or you can accept unvalidated settings. Between those, my vote is for the first, because I think that extensions are not likely to want to have to deal at runtime with the possibility of having arbitrary values where they expect values from a fixed list. Basically, my feeling is that if you install an extension that adds new table-level options, that's effectively a new version of the database, and expecting a dump from that version to restore into a vanilla database is about as reasonable as expecting 9.4 dumps to restore flawlessly on 8.4. Pft. I don't expect a restore to succeed without the library present, but I think any such infrastructure should work with a CREATE EXTENSION installing the provider. Especially if we're trying to make this into something more generic than just for pure security labels. It might make sense to always require the library be always loaded for selinux or whatnot, but much less so if it's for a schema management tool or something. Relying on shared_preload_library seems to run counter the route pg's extensibility has taken. 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] [PATCH] Store Extension Options
Andres Freund and...@2ndquadrant.com writes: On 2014-03-13 10:26:11 -0400, Tom Lane wrote: No, because relcache doesn't store security labels to start with. There's a separate catalog cache for security labels, I believe, and invalidating entries in that ought to be sufficient. There doesn't seem to be any form of system managed cache for security labels afaics. Every lookup does a index scan. I currently don't see how I could build a cache in userland that'd invalidate if either a) the underlying object changes b) the label changes. If there's not a catcache for pg_seclabels, I'd have no objection to adding one. As for your userland cache objection, you certainly could build such a thing using the existing inval callbacks (if we had a catcache on pg_seclabels), and in any case what have userland caches got to do with relcache? 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] [PATCH] Store Extension Options
On Thu, Mar 13, 2014 at 10:27 AM, Andres Freund and...@2ndquadrant.com wrote:
On 2014-03-13 10:24:09 -0400, Tom Lane wrote:
Andres Freund and...@2ndquadrant.com writes:
But security labels are a nice idea, will think about it. AFAICs there's
no builtin subdivision within the label for one provider which is a bit
of a shame but solvable. The biggest issue I see is that it essentially
seems to require that the provider is in
{shared,local}_preload_libraries? You can't restore into a server
otherwise afaics?
Well, if you want to validate the settings then you pretty much have to
require that in some form.
If there were a CREATE SECURITY LABEL PROVIDER or something, with the
catalog pointing to a validator function, we wouldn't necessarily...
I seriously doubt that's going to work nicely. Now you've implicitly
introduced a dependency from every object that has a label to the
label provider. pg_dump is going to have to restore the validator
function before it restores anything that has a label, and before that
it's going to have to restore the languages used to create those
validator functions, and those languages might themselves be labeled,
either by that provider or by other providers.
Perhaps you could untangle that mess, but I'm disinclined to try
because I can't see what real problem we're solving here. Extension
that just provide particular functions or datatypes can be loaded on
demand, but those that change underlying system behavior need to be
loaded by the postmaster, or at least at backend startup. We've tried
to patch around that fact with GUCs and it seems to me that we've
thoroughly destroyed validation in the process but without really
buying ourselves much. There are five contrib modules that define
custom variables: auth_delay, auto_explain, pg_stat_statements,
sepgsql, and worker_spi. auth_delay, worker_spi and
pg_stat_statements have to be loaded at postmaster startup time, and
you have to decide whether you want sepgsql at *initdb* time. The
only one of those that you can possibly load on the fly into an
individual session is auto_explain, and that's probably not very
useful: if you have control of the interactive session, you might as
well just use EXPLAIN. Maybe there are better examples outside the
core distribution, but to me it's looking like the idea that you can
add GUCs on the fly into individual sessions is a big fizz.
--
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] [PATCH] Store Extension Options
On Thu, Mar 13, 2014 at 10:45 AM, Andres Freund and...@2ndquadrant.com wrote: On 2014-03-13 10:31:12 -0400, Robert Haas wrote: I think the really interesting question here is how the dump-and-reload issue ought to be handled. As Tom says, it seems on the surface as though you can either require that the provider be loaded for that, or you can accept unvalidated settings. Between those, my vote is for the first, because I think that extensions are not likely to want to have to deal at runtime with the possibility of having arbitrary values where they expect values from a fixed list. Basically, my feeling is that if you install an extension that adds new table-level options, that's effectively a new version of the database, and expecting a dump from that version to restore into a vanilla database is about as reasonable as expecting 9.4 dumps to restore flawlessly on 8.4. Pft. I don't expect a restore to succeed without the library present, but I think any such infrastructure should work with a CREATE EXTENSION installing the provider. Especially if we're trying to make this into something more generic than just for pure security labels. It might make sense to always require the library be always loaded for selinux or whatnot, but much less so if it's for a schema management tool or something. Relying on shared_preload_library seems to run counter the route pg's extensibility has taken. Well, I don't have a big problem with the idea that some sessions might not have a certain extension loaded. For some extensions, that might not lead to very coherent behavior, but I guess it's the extension developer's job to tell the user whether or not that extension needs shared_preload_libraries, needs either shared or local preload_libraries, or can be installed however. At the same time, I don't feel compelled to provide an autoload mechanism to cover the case where a user tries to set a label in a session which does not have the label provider preloaded. Such a mechanism will be complex and introduce many problems of its own for what is in my mind a pretty darn narrow benefit; and we sure as heck do not have time to engineer it for 9.4. -- 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] [PATCH] Store Extension Options
On 2014-03-13 11:11:51 -0400, Tom Lane wrote: Andres Freund and...@2ndquadrant.com writes: On 2014-03-13 10:26:11 -0400, Tom Lane wrote: No, because relcache doesn't store security labels to start with. There's a separate catalog cache for security labels, I believe, and invalidating entries in that ought to be sufficient. There doesn't seem to be any form of system managed cache for security labels afaics. Every lookup does a index scan. I currently don't see how I could build a cache in userland that'd invalidate if either a) the underlying object changes b) the label changes. If there's not a catcache for pg_seclabels, I'd have no objection to adding one. Ok. That's an easy enough patch, would anyone object to adding that now? As for your userland cache objection, you certainly could build such a thing using the existing inval callbacks (if we had a catcache on pg_seclabels), and in any case what have userland caches got to do with relcache? I don't think I've said anything about relcaches being required for this. It came up in 20140313132604.gg8...@awork2.anarazel.de, but that was because we were just talking table level there, and it's a tad easier to hook into relcache invalidation callbacks than catcache ones. That said, for a relation level cache that refer's to the table's definition, you really *do* need a relcache invalidation callback, not just a catcache callback. There's a fair number of places that do a CacheInvalidateRelcache() to trigger invals. 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] [PATCH] Store Extension Options
Robert Haas escribió: Well, I don't have a big problem with the idea that some sessions might not have a certain extension loaded. For some extensions, that might not lead to very coherent behavior, but I guess it's the extension developer's job to tell the user whether or not that extension needs shared_preload_libraries, needs either shared or local preload_libraries, or can be installed however. At the same time, I don't feel compelled to provide an autoload mechanism to cover the case where a user tries to set a label in a session which does not have the label provider preloaded. Such a mechanism will be complex and introduce many problems of its own for what is in my mind a pretty darn narrow benefit; and we sure as heck do not have time to engineer it for 9.4. Eh? Why do we need an autoload mechanism? As far as I know, we already have one --- you call a function that's in an installed module, and if the module is not loaded, it will then be loaded. So if we have a registry of validator functions, it will just be a matter of calling the validator function, and the autoloader will load the module. Of course, the module needs to have been installed previously, but at least for extensions surely this is going to be the case. I don't really like the LABEL idea being proposed in this subthread to store options. The nice thing about reloptions is that the code to parse input, validate the option names and values, and put the values in a struct is all already there. All a module has to do is call a few appropriate functions and provide a parsing table that goes alongside a struct definition. With LABELs, each module is going to have to provide code to do this all over, unless you're all thinking of something that I'm missing. -- Álvaro Herrerahttp://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] [PATCH] Store Extension Options
On Thu, Mar 13, 2014 at 11:11 AM, Tom Lane t...@sss.pgh.pa.us wrote: Andres Freund and...@2ndquadrant.com writes: On 2014-03-13 10:26:11 -0400, Tom Lane wrote: No, because relcache doesn't store security labels to start with. There's a separate catalog cache for security labels, I believe, and invalidating entries in that ought to be sufficient. There doesn't seem to be any form of system managed cache for security labels afaics. Every lookup does a index scan. I currently don't see how I could build a cache in userland that'd invalidate if either a) the underlying object changes b) the label changes. If there's not a catcache for pg_seclabels, I'd have no objection to adding one. As for your userland cache objection, you certainly could build such a thing using the existing inval callbacks (if we had a catcache on pg_seclabels), and in any case what have userland caches got to do with relcache? I avoided doing that for the same reasons that we've been careful to add no such cache to pg_largeobject_metadata: the number of large objects could be big enough to cause problems with backend memory consumption. Note that large objects are one of the object types to which security labels can be applied, so any concern that applies there also applies here. I have however had the thought before that it would be nice to allow for callbacks of invalidation functions of some kind even on catalogs that don't have catcaches. -- 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] [PATCH] Store Extension Options
On 2014-03-13 11:15:56 -0400, Robert Haas wrote:
On Thu, Mar 13, 2014 at 10:27 AM, Andres Freund and...@2ndquadrant.com
wrote:
On 2014-03-13 10:24:09 -0400, Tom Lane wrote:
Andres Freund and...@2ndquadrant.com writes:
But security labels are a nice idea, will think about it. AFAICs there's
no builtin subdivision within the label for one provider which is a bit
of a shame but solvable. The biggest issue I see is that it essentially
seems to require that the provider is in
{shared,local}_preload_libraries? You can't restore into a server
otherwise afaics?
Well, if you want to validate the settings then you pretty much have to
require that in some form.
If there were a CREATE SECURITY LABEL PROVIDER or something, with the
catalog pointing to a validator function, we wouldn't necessarily...
I seriously doubt that's going to work nicely. Now you've implicitly
introduced a dependency from every object that has a label to the
label provider. pg_dump is going to have to restore the validator
function before it restores anything that has a label, and before that
it's going to have to restore the languages used to create those
validator functions, and those languages might themselves be labeled,
either by that provider or by other providers.
Aren't pretty much all of those problems already solved because already
need to be able to order all this to dump the extension for a datatype
before the relation with a column of that type?
Perhaps you could untangle that mess, but I'm disinclined to try
because I can't see what real problem we're solving here. Extension
that just provide particular functions or datatypes can be loaded on
demand, but those that change underlying system behavior need to be
loaded by the postmaster, or at least at backend startup.
Why is adding an annotation to a table changing the underlying system
behaviour? There might be cases where it is, and those can easily
require having been loaded via s_p_l.
We've tried to patch around that fact with GUCs and it seems to me that we've
thoroughly destroyed validation in the process but without really
buying ourselves much.
I think you're making a much bigger issue of GUC validation problems
than there is. It's perfectly possible to assign datatypes, check
functions et all to custom GUCs? And there's
EmitWarningsOnPlaceholders() to warn about unknown GUCs inside a
namespace.
There are five contrib modules that define
custom variables: auth_delay, auto_explain, pg_stat_statements,
sepgsql, and worker_spi. auth_delay, worker_spi and
pg_stat_statements have to be loaded at postmaster startup time, and
you have to decide whether you want sepgsql at *initdb* time.
You forgot at least plpgsql. Which is already a good showcase why we
want this to be per database, not per cluster, i.e. not preloaded.
There's also pretty good reasons to use auto_explain at the session
level, because you otherwise can't look inside a function's plans.
Maybe there are better examples outside the
core distribution, but to me it's looking like the idea that you can
add GUCs on the fly into individual sessions is a big fizz.
It seems to be on a somewhat odd warpath against against custom gucs ;)
. I've used the capability to do so *dozens* of times. What problems
have they actually caused?
Note that postgresql.conf is parsed long before we initiate
shared_preload_libraries et al are taking effect, so even if we'd
require libraries to be loaded before custom GUCs can be defined, we'd
need to create a entirely new mechanism of loading libraries for
it. With a very odd circularity, because to parse postgresql.conf you'd
need to have it parsed to load the libraries.
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] [PATCH] Store Extension Options
On 2014-03-13 11:20:02 -0400, Robert Haas wrote: At the same time, I don't feel compelled to provide an autoload mechanism to cover the case where a user tries to set a label in a session which does not have the label provider preloaded. I don't think there's that much need for that to be supported for user initiated setting, but pg_dump imo is a different case. Such a mechanism will be complex and introduce many problems of its own for what is in my mind a pretty darn narrow benefit; and we sure as heck do not have time to engineer it for 9.4. Yea, I think that's pretty clearly out of scope for 9.4, independent of the solution we can come up with. 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] [PATCH] Store Extension Options
On 2014-03-13 11:26:10 -0400, Robert Haas wrote: On Thu, Mar 13, 2014 at 11:11 AM, Tom Lane t...@sss.pgh.pa.us wrote: If there's not a catcache for pg_seclabels, I'd have no objection to adding one. As for your userland cache objection, you certainly could build such a thing using the existing inval callbacks (if we had a catcache on pg_seclabels), and in any case what have userland caches got to do with relcache? I avoided doing that for the same reasons that we've been careful to add no such cache to pg_largeobject_metadata: the number of large objects could be big enough to cause problems with backend memory consumption. Note that large objects are one of the object types to which security labels can be applied, so any concern that applies there also applies here. Good point. Are you primarily worried about the size of the cache, or about the size of the queued invaldations? I guess if it's the former we could just have the cache, but not use it when looking up values. But yuck. I think it'd be cleaner to trigger invalidations on the underlying objects... I have however had the thought before that it would be nice to allow for callbacks of invalidation functions of some kind even on catalogs that don't have catcaches. Unfortunately the format catcache invalidations have is pretty tightly tied to the hash function catcaches use internally. And we need something that can be included in the WAL, otherwise it won't work on HS nodes. 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] [PATCH] Store Extension Options
Andres Freund and...@2ndquadrant.com writes: On 2014-03-13 11:26:10 -0400, Robert Haas wrote: I have however had the thought before that it would be nice to allow for callbacks of invalidation functions of some kind even on catalogs that don't have catcaches. Unfortunately the format catcache invalidations have is pretty tightly tied to the hash function catcaches use internally. And we need something that can be included in the WAL, otherwise it won't work on HS nodes. Note that the existence of a cache doesn't mean it's necessarily populated. In this example, a catcache on pg_seclabels could be used just fine, as long as it wasn't used to load labels for large objects. Even if it were never used at all, it would still provide a usable conduit for invalidation events. 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] [PATCH] Store Extension Options
On Thu, Mar 13, 2014 at 12:11 PM, Tom Lane t...@sss.pgh.pa.us wrote: Andres Freund and...@2ndquadrant.com writes: On 2014-03-13 11:26:10 -0400, Robert Haas wrote: I have however had the thought before that it would be nice to allow for callbacks of invalidation functions of some kind even on catalogs that don't have catcaches. Unfortunately the format catcache invalidations have is pretty tightly tied to the hash function catcaches use internally. And we need something that can be included in the WAL, otherwise it won't work on HS nodes. Note that the existence of a cache doesn't mean it's necessarily populated. In this example, a catcache on pg_seclabels could be used just fine, as long as it wasn't used to load labels for large objects. Even if it were never used at all, it would still provide a usable conduit for invalidation events. That'd need some commenting, but it seems like a possibly workable approach that wouldn't require changing much. -- 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] [PATCH] Store Extension Options
On Thu, Mar 13, 2014 at 11:42 AM, Andres Freund and...@2ndquadrant.com wrote: On 2014-03-13 11:26:10 -0400, Robert Haas wrote: On Thu, Mar 13, 2014 at 11:11 AM, Tom Lane t...@sss.pgh.pa.us wrote: If there's not a catcache for pg_seclabels, I'd have no objection to adding one. As for your userland cache objection, you certainly could build such a thing using the existing inval callbacks (if we had a catcache on pg_seclabels), and in any case what have userland caches got to do with relcache? I avoided doing that for the same reasons that we've been careful to add no such cache to pg_largeobject_metadata: the number of large objects could be big enough to cause problems with backend memory consumption. Note that large objects are one of the object types to which security labels can be applied, so any concern that applies there also applies here. Good point. Are you primarily worried about the size of the cache, or about the size of the queued invaldations? Mostly the former. I can't really see the latter being a big deal. I mean, if you do a lot of DDL, you'll get more sinval resets, but oh well. We can't optimize away re-examining the data when it actually is changing underneath us. -- 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] [PATCH] Store Extension Options
On Thu, Mar 13, 2014 at 11:37 AM, Andres Freund and...@2ndquadrant.com wrote: I seriously doubt that's going to work nicely. Now you've implicitly introduced a dependency from every object that has a label to the label provider. pg_dump is going to have to restore the validator function before it restores anything that has a label, and before that it's going to have to restore the languages used to create those validator functions, and those languages might themselves be labeled, either by that provider or by other providers. Aren't pretty much all of those problems already solved because already need to be able to order all this to dump the extension for a datatype before the relation with a column of that type? Well, there's dependency tracking in general. But security label providers don't exist as SQL objects today, so they don't participate in it. You'd need to make them dumpable objects and then add dependencies in pg_(sh)depend and then figure out how to break cycles. We could do that, but I'm not finding it very compelling. Perhaps you could untangle that mess, but I'm disinclined to try because I can't see what real problem we're solving here. Extension that just provide particular functions or datatypes can be loaded on demand, but those that change underlying system behavior need to be loaded by the postmaster, or at least at backend startup. Why is adding an annotation to a table changing the underlying system behaviour? There might be cases where it is, and those can easily require having been loaded via s_p_l. I guess that's true. There are five contrib modules that define custom variables: auth_delay, auto_explain, pg_stat_statements, sepgsql, and worker_spi. auth_delay, worker_spi and pg_stat_statements have to be loaded at postmaster startup time, and you have to decide whether you want sepgsql at *initdb* time. You forgot at least plpgsql. Which is already a good showcase why we want this to be per database, not per cluster, i.e. not preloaded. OK, true. Maybe there are better examples outside the core distribution, but to me it's looking like the idea that you can add GUCs on the fly into individual sessions is a big fizz. It seems to be on a somewhat odd warpath against against custom gucs ;) . I've used the capability to do so *dozens* of times. What problems have they actually caused? Note that postgresql.conf is parsed long before we initiate shared_preload_libraries et al are taking effect, so even if we'd require libraries to be loaded before custom GUCs can be defined, we'd need to create a entirely new mechanism of loading libraries for it. With a very odd circularity, because to parse postgresql.conf you'd need to have it parsed to load the libraries. Yes, that's part of the problem there. -- 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] [PATCH] Store Extension Options
On Thu, Mar 13, 2014 at 11:30 AM, Alvaro Herrera alvhe...@2ndquadrant.com wrote: Robert Haas escribió: Well, I don't have a big problem with the idea that some sessions might not have a certain extension loaded. For some extensions, that might not lead to very coherent behavior, but I guess it's the extension developer's job to tell the user whether or not that extension needs shared_preload_libraries, needs either shared or local preload_libraries, or can be installed however. At the same time, I don't feel compelled to provide an autoload mechanism to cover the case where a user tries to set a label in a session which does not have the label provider preloaded. Such a mechanism will be complex and introduce many problems of its own for what is in my mind a pretty darn narrow benefit; and we sure as heck do not have time to engineer it for 9.4. Eh? Why do we need an autoload mechanism? As far as I know, we already have one --- you call a function that's in an installed module, and if the module is not loaded, it will then be loaded. So if we have a registry of validator functions, it will just be a matter of calling the validator function, and the autoloader will load the module. We have an autoload mechanism for functions, but not for label providers. To make label providers autoload, we'd have to store them in a catalog with pointers to pg_proc entries for their validators - which seems like a can of worms, at least at this point in the release cycle. Of course, the module needs to have been installed previously, but at least for extensions surely this is going to be the case. I don't really like the LABEL idea being proposed in this subthread to store options. The nice thing about reloptions is that the code to parse input, validate the option names and values, and put the values in a struct is all already there. All a module has to do is call a few appropriate functions and provide a parsing table that goes alongside a struct definition. With LABELs, each module is going to have to provide code to do this all over, unless you're all thinking of something that I'm missing. Well, I'm not sure that's really any big deal, but I'm not wedded to the label idea. My principal concern is: I'm opposed to allowing unvalidated options into the database. I think it should be a requirement that if the validator can't be found and called, then the reloption is no good and you just reject it. So, if we go with the reloptions route, I'd want to see pg_register_option_namespace go away in favor of some solution that preserves that property. One thing I kind of like about the LABEL approach is that it applies to virtually every object type, meaning that we might not have to repeat this discussion when (as seems inevitable) people want to be able to do things to schemas or tablespaces or roles. But I don't hold that position so strongly as to be unwilling to entertain any other options. -- 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] [PATCH] Store Extension Options
On Tue, Mar 11, 2014 at 8:42 PM, Simon Riggs si...@2ndquadrant.com wrote: On 11 March 2014 18:33, Tom Lane t...@sss.pgh.pa.us wrote: Simon Riggs si...@2ndquadrant.com writes: -1 to *requiring* validation for table-level options for exactly the same reasons we no longer validate custom GUCs. Well, that is an interesting analogy, but I'm not sure how much it applies here. In the case of a GUC, you can fairly easily validate it once the module does get loaded (and before the module actually tries to do anything with it). I don't see how that's going to work for table options. I trust nobody is seriously proposing that on module load, we're going to scan the whole of pg_class looking to see if there are incorrect settings. (Even if we did, what would we do about it? Not try to force a pg_class update, for sure. And what if the module is loading into the postmaster thanks to a preload spec?) Thank goodness for that. Strict validation does seem scary. I don't really think partial validation makes sense. We could just remove the whole topic, and tell extension authors that it's up to them to defend themselves against bizarre values stored for their table options. But I'm wondering if there's really so much use-case for a feature like that. DBAs are fairly used to the idea that if you put crap data in the database then bad things happen. We provide the table, they provide the data. Validation is possible, but not enforced as essential. (Except in terms of the datatype - but then we are also validating data to specific types here). So I think that DBAs will also cope rather well with table-level options without us nannying them. There is nothing more annoying that needing to run scripts in a specific sequence to make them work, or dumps that fail because certain modules aren't loaded yet (or cannot ever be so). And maybe the DBA wants to annotate tables based on a design and then later move to implement modules to take advantage of the annotation. Having an option be set and yet be unvalidated and/or unused is no more annoying than having a column in a table that is known incorrect and/or not accessed. Searching for badly set options needs to be possible, even easy, but hard validation can cause problems. And if we try and force it, whats to stop people from using a dummy validator just to circumvent the strictness? Then I think my patch is more adherent given these conclusions, except by the some adjustments suggested by Tom Lane and mentioned by Alvaro Herrera [1]. Am I correct? [1] http://www.postgresql.org/message-id/20140307205649.gf4...@eldon.alvh.no-ip.org -- Fabrízio de Royes Mello Consultoria/Coaching PostgreSQL Timbira: http://www.timbira.com.br Blog sobre TI: http://fabriziomello.blogspot.com Perfil Linkedin: http://br.linkedin.com/in/fabriziomello Twitter: http://twitter.com/fabriziomello
Re: [HACKERS] [PATCH] Store Extension Options
On Mon, Mar 10, 2014 at 9:33 PM, Alvaro Herrera alvhe...@2ndquadrant.com wrote: I haven't touched pg_dump yet, but if this proposed design sits well with everyone, my intention is that the dump output will contain the pg_register_option_namespace() calls necessary so that a table definition will be able to do the SET calls to set the values the original table has, and succeed. In other words, restoring a dump will preserve the values you had, without a need of having the module loaded in the new server. I think this is what was discussed. Robert, do you agree? No, I wasn't imagining anything like pg_register_option_namespace(). My thought was that you'd need to have any relevant modules loaded at restore time. In essence, patching in a new option via an extension module would work about like adding one by patching the core code: you need a server version that supports that option in order to set it. I don't like the idea of using reloptions to let people attach arbitrary unvalidated settings to tables. I consider the way things work with GUCs to be a bug, not a feature, and definitely not something I want to propagate into every other area of the system where the underlying storage format happens to allow it. I also kind of think that what you're going to find if you try to press forward with the pg_register_option_namespace() idea is that what you really want is CREATE RELOPTION NAMESPACE, ALTER RELOPTION NAMESPACE, DROP RELOPTION NAMESPACE. Short of that, you're going to end up with a bunch of kludges, I suspect. And some kind of real DDL syntax (with better naming) is OK with me, but as you observed elsewhere on the thread, now you're looking at a new catalog and a bunch more complexity. I kind of think that this is too half-baked for 9.4 and we ought to punt it to 9.5. -- 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] [PATCH] Store Extension Options
On 03/12/2014 03:58 PM, Robert Haas wrote: I don't like the idea of using reloptions to let people attach arbitrary unvalidated settings to tables. I consider the way things work with GUCs to be a bug, not a feature, and definitely not something I want to propagate into every other area of the system where the underlying storage format happens to allow it. +1. Relopts are one of the uglier warts we have. -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.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] [PATCH] Store Extension Options
Josh Berkus escribió: On 03/12/2014 03:58 PM, Robert Haas wrote: I don't like the idea of using reloptions to let people attach arbitrary unvalidated settings to tables. I consider the way things work with GUCs to be a bug, not a feature, and definitely not something I want to propagate into every other area of the system where the underlying storage format happens to allow it. +1. Relopts are one of the uglier warts we have. I'm not sure what you're plus-oneing here, but I hope it's not the ability to set custom reloptions altogether. As I interpret what Robert was saying, it was let's not have *unvalidated* reloptions, with which I'm fine --- it only means we need to make sure custom reloptions are validated, in some way yet to be agreed. I agree that it has gotten too late for this in 9.4, also. I don't see what's so ugly about reloptions as they currently exist, anyway. -- Álvaro Herrerahttp://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] [PATCH] Store Extension Options
On 12 March 2014 22:58, Robert Haas robertmh...@gmail.com wrote: I don't like the idea of using reloptions to let people attach arbitrary unvalidated settings to tables. I respect your opinion. If you disagree, don't use them. Same as is possible for RULEs etc. I consider the way things work with GUCs to be a bug, not a feature, and definitely not something I want to propagate into every other area of the system where the underlying storage format happens to allow it. Experience was that requiring validation made things more brittle, which is why we relaxed things a few releases ago. Opinions are one thing, experience is quite another. I kind of think that this is too half-baked for 9.4 and we ought to punt it to 9.5. No, its fully functional, apart from the requirement for validation which is imposed upon this patch. I'm not sure why this is being blocked. This is a community contribution that seeks to improve everybody's options. Blocking it does *nothing* to prevent individual extensions from providing table-level options - we give them freedom to do whatever the hell they want. Validation is a pipe dream, not *ever* an achievable reality. Blocking is just exercise of a veto for nobody's gain. -- Simon Riggs 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] [PATCH] Store Extension Options
On Wed, Mar 12, 2014 at 9:38 PM, Simon Riggs si...@2ndquadrant.com wrote: On 12 March 2014 22:58, Robert Haas robertmh...@gmail.com wrote: I don't like the idea of using reloptions to let people attach arbitrary unvalidated settings to tables. I respect your opinion. If you disagree, don't use them. Same as is possible for RULEs etc. That's not an answer. We don't let people put things in a date column that aren't actually dates, and we don't let people put things in an integer columns that aren't actually integers. Some other database have made different choices in those areas, and we've rightly chosen to more strict. Why is validation a good thing for the values that are stored in the tables but not a good idea for the metadata associated with those tables? Experience was that requiring validation made things more brittle, which is why we relaxed things a few releases ago. Opinions are one thing, experience is quite another. Sure. But I think the reason why requiring validation made things more brittle is because the validation mechanism we used to have wasn't very good, not because validating stuff is in general not a good thing to do. I'm not sure why this is being blocked. This is a community contribution that seeks to improve everybody's options. Blocking it does *nothing* to prevent individual extensions from providing table-level options - we give them freedom to do whatever the hell they want. Validation is a pipe dream, not *ever* an achievable reality. Blocking is just exercise of a veto for nobody's gain. Unsurprisingly, I don't agree with any of 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] [PATCH] Store Extension Options
On 13 March 2014 02:14, Robert Haas robertmh...@gmail.com wrote: I'm not sure why this is being blocked. This is a community contribution that seeks to improve everybody's options. Blocking it does *nothing* to prevent individual extensions from providing table-level options - we give them freedom to do whatever the hell they want. Validation is a pipe dream, not *ever* an achievable reality. Blocking is just exercise of a veto for nobody's gain. Unsurprisingly, I don't agree with any of that. The point is that execising a veto here is irrelevant. Blocking this patch does *nothing* to prevent extensions from adopting per-table options. All that is happening is that a single, coherent mechanism for such options is being blocked. Blocking this is like trying to block rain. We can all pretend the blocking viewpoint has succeeded, but all it does is to bring Postgres core into disrepute. I have often heard that from others that this is a business opportunity, not a problem. If that is true, its not because we didn't try to act for the good of all. -- Simon Riggs 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] [PATCH] Store Extension Options
Tom Lane escribió:
=?ISO-8859-1?Q?Fabr=EDzio_de_Royes_Mello?= fabriziome...@gmail.com writes:
You are correct. pg_dump export reloptions using WITH clause of CREATE
TABLE statement. I.e.:
CREATE TABLE foo (
)
WITH (autovacuum_enabled=false, bdr.do_replicate=false);
So if this statement checks for 'bdr' extension is loaded then in partial
restore it can be fail.
I see absolutely *nothing* wrong with failing that command if bdr is not
installed. For an analogy, if this table includes a column of type bar
defined by some extension baz, we are certainly going to fail the
CREATE TABLE if baz isn't installed.
Now, if bdr is installed but the validation doesn't happen unless bdr
is loaded in some sense, then that is an implementation deficiency
that I think we can insist be rectified before this feature is accepted.
So, I spent some time on this patch the last couple of days to add some
validation. But after submitting it, it seems to me that there wasn't
as much consensus on how to handle validation than at first I thought.
So, the first and simplest way to go about this, of course, is just not
validate anything. This is what Fabrízio's patch does. So the table
owner can execute
ALTER TABLE mary SET (supercalifragilisticexpialidocious.umbrella_flight =
'hell yeah')
and that would be it. Whether a module makes use of this later or not,
is not that guy's problem. This is mostly what we do for GUCs, note, so
it's not exactly groundbreaking.
As a second possibility, my patch as posted allows one to register a
namespace. So pg_dump can do this:
SELECT pg_register_option_namespace('supercalifragilisticexpialidocious');
and then create the table just like the above ALTER TABLE. Note that
the variable name, and the value, are not checked until later. If a
module comes later and says hey, I own that super- option namespace,
and I have option umbrella_flight but it's a boolean (by calling
add_bool_reloption), that will raise an error. Note that in my patch as
posted, if you set the parameter umbrella_flight='better not' to an
index, but the parameter has only been defined for tables
(RELOPT_KIND_HEAP), it will be silently accepted. Also note that we can
add a function equivalent to EmitWarningOnPlaceholders (Andres' idea),
so that any unrecognized option will cause some noise and it can be
identified right away. Since only table owners can set options, this
seems more than good to me; it's not like table owners are going to mess
up by adding pointless options just for the heck of it.
A further possibility requires modules to also register options (not
only namespaces), and to validate each and every option as soon as it's
created. So if you try to set an option that has not been previously
registered by a module, that will raise an error right there. This
seems to be what Tom, Robert and Peter want. However, getting there
seems very laborious; apparently, we will need a new system catalog to
register option validators, for starters. We'll also need a way to load
the module whenever a table gets an option in a not-loaded module. (I
think this will fall off automatically if the validator is registered,
because when the validator is called, the module is loaded by the
system).
One slight problem with this is what to do with extensions that don't
provide any C code. Some use cases require options that can be set and
accessed only from SQL code.
My question here for the hackers community at large is this: are we okay
with implementing the second option I propose above? If we are, then I
will see about finalizing the patch and getting it committed. If we are
not, and we're determined that only the third option is acceptable, I
will jump out of this thread and stop wasting everyone's time.
--
Álvaro Herrerahttp://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] [PATCH] Store Extension Options
On 11 March 2014 17:26, Alvaro Herrera alvhe...@2ndquadrant.com wrote: Tom Lane escribió: =?ISO-8859-1?Q?Fabr=EDzio_de_Royes_Mello?= fabriziome...@gmail.com writes: You are correct. pg_dump export reloptions using WITH clause of CREATE TABLE statement. I.e.: CREATE TABLE foo ( ) WITH (autovacuum_enabled=false, bdr.do_replicate=false); So if this statement checks for 'bdr' extension is loaded then in partial restore it can be fail. I see absolutely *nothing* wrong with failing that command if bdr is not installed. For an analogy, if this table includes a column of type bar defined by some extension baz, we are certainly going to fail the CREATE TABLE if baz isn't installed. Now, if bdr is installed but the validation doesn't happen unless bdr is loaded in some sense, then that is an implementation deficiency that I think we can insist be rectified before this feature is accepted. So, I spent some time on this patch the last couple of days to add some validation. But after submitting it, it seems to me that there wasn't as much consensus on how to handle validation than at first I thought. So, the first and simplest way to go about this, of course, is just not validate anything. This is what Fabrízio's patch does. So the table owner can execute ALTER TABLE mary SET (supercalifragilisticexpialidocious.umbrella_flight = 'hell yeah') and that would be it. Whether a module makes use of this later or not, is not that guy's problem. This is mostly what we do for GUCs, note, so it's not exactly groundbreaking. If a module fails to use a parameter that may be a problem. But forcing us to validate this using user written code may not improve the situation. What happens if I have two extensions that both use the namespace foo? That means we would run two validation routines on it, and if they disagree on the set of options and values then we are hosed. -1 to *requiring* validation for table-level options for exactly the same reasons we no longer validate custom GUCs. -- Simon Riggs 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] [PATCH] Store Extension Options
On Tue, Mar 11, 2014 at 2:43 PM, Simon Riggs si...@2ndquadrant.com wrote: On 11 March 2014 17:26, Alvaro Herrera alvhe...@2ndquadrant.com wrote: Tom Lane escribió: =?ISO-8859-1?Q?Fabr=EDzio_de_Royes_Mello?= fabriziome...@gmail.com writes: You are correct. pg_dump export reloptions using WITH clause of CREATE TABLE statement. I.e.: CREATE TABLE foo ( ) WITH (autovacuum_enabled=false, bdr.do_replicate=false); So if this statement checks for 'bdr' extension is loaded then in partial restore it can be fail. I see absolutely *nothing* wrong with failing that command if bdr is not installed. For an analogy, if this table includes a column of type bar defined by some extension baz, we are certainly going to fail the CREATE TABLE if baz isn't installed. Now, if bdr is installed but the validation doesn't happen unless bdr is loaded in some sense, then that is an implementation deficiency that I think we can insist be rectified before this feature is accepted. So, I spent some time on this patch the last couple of days to add some validation. But after submitting it, it seems to me that there wasn't as much consensus on how to handle validation than at first I thought. So, the first and simplest way to go about this, of course, is just not validate anything. This is what Fabrízio's patch does. So the table owner can execute ALTER TABLE mary SET (supercalifragilisticexpialidocious.umbrella_flight = 'hell yeah') and that would be it. Whether a module makes use of this later or not, is not that guy's problem. This is mostly what we do for GUCs, note, so it's not exactly groundbreaking. If a module fails to use a parameter that may be a problem. But forcing us to validate this using user written code may not improve the situation. What happens if I have two extensions that both use the namespace foo? That means we would run two validation routines on it, and if they disagree on the set of options and values then we are hosed. -1 to *requiring* validation for table-level options for exactly the same reasons we no longer validate custom GUCs. In a previous email [1] I asked about alternatives to drive the work but unfortunately no one replied. So because we already do that to custom GUCs, and is the simpler way to implement this feature then I did that. Regards, [1] http://www.postgresql.org/message-id/cafcns+r1zxtruzleceuj1sf9qr6ciks7he-esmkzoznh4nx...@mail.gmail.com -- Fabrízio de Royes Mello Consultoria/Coaching PostgreSQL Timbira: http://www.timbira.com.br Blog sobre TI: http://fabriziomello.blogspot.com Perfil Linkedin: http://br.linkedin.com/in/fabriziomello Twitter: http://twitter.com/fabriziomello
Re: [HACKERS] [PATCH] Store Extension Options
Simon Riggs si...@2ndquadrant.com writes: -1 to *requiring* validation for table-level options for exactly the same reasons we no longer validate custom GUCs. Well, that is an interesting analogy, but I'm not sure how much it applies here. In the case of a GUC, you can fairly easily validate it once the module does get loaded (and before the module actually tries to do anything with it). I don't see how that's going to work for table options. I trust nobody is seriously proposing that on module load, we're going to scan the whole of pg_class looking to see if there are incorrect settings. (Even if we did, what would we do about it? Not try to force a pg_class update, for sure. And what if the module is loading into the postmaster thanks to a preload spec?) I don't really think partial validation makes sense. We could just remove the whole topic, and tell extension authors that it's up to them to defend themselves against bizarre values stored for their table options. But I'm wondering if there's really so much use-case for a feature like that. 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] [PATCH] Store Extension Options
On 11 March 2014 18:33, Tom Lane t...@sss.pgh.pa.us wrote: Simon Riggs si...@2ndquadrant.com writes: -1 to *requiring* validation for table-level options for exactly the same reasons we no longer validate custom GUCs. Well, that is an interesting analogy, but I'm not sure how much it applies here. In the case of a GUC, you can fairly easily validate it once the module does get loaded (and before the module actually tries to do anything with it). I don't see how that's going to work for table options. I trust nobody is seriously proposing that on module load, we're going to scan the whole of pg_class looking to see if there are incorrect settings. (Even if we did, what would we do about it? Not try to force a pg_class update, for sure. And what if the module is loading into the postmaster thanks to a preload spec?) Thank goodness for that. Strict validation does seem scary. I don't really think partial validation makes sense. We could just remove the whole topic, and tell extension authors that it's up to them to defend themselves against bizarre values stored for their table options. But I'm wondering if there's really so much use-case for a feature like that. DBAs are fairly used to the idea that if you put crap data in the database then bad things happen. We provide the table, they provide the data. Validation is possible, but not enforced as essential. (Except in terms of the datatype - but then we are also validating data to specific types here). So I think that DBAs will also cope rather well with table-level options without us nannying them. There is nothing more annoying that needing to run scripts in a specific sequence to make them work, or dumps that fail because certain modules aren't loaded yet (or cannot ever be so). And maybe the DBA wants to annotate tables based on a design and then later move to implement modules to take advantage of the annotation. Having an option be set and yet be unvalidated and/or unused is no more annoying than having a column in a table that is known incorrect and/or not accessed. Searching for badly set options needs to be possible, even easy, but hard validation can cause problems. And if we try and force it, whats to stop people from using a dummy validator just to circumvent the strictness? -- Simon Riggs 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] [PATCH] Store Extension Options
Fabrízio de Royes Mello escribió:
On Fri, Mar 7, 2014 at 5:56 PM, Alvaro Herrera
alvhe...@2ndquadrant.comwrote:
I am reworking this patch, both to accomodate earlier review comments
and to fix the multiple verify step of namespaces, as noted by Tom in
4530.1390023...@sss.pgh.pa.us
Alvaro,
Do you need some help?
Would you give this version of the patch a look? I reworked your patch
a bit, mainly to add a bit of checking to custom namespaces. In this
code, before you can add a namespaced option to an object, you need to
register the namespace. There are two interfaces for this: C code can
call registerOptionNamespace(). This patch adds a call to plpgsql that
does so (It's not my intention that plpgsql is modified in any way by
this patch; this part of the patch is here just for demonstration purposes.)
I expect most extension modules would do things that way.
The other interface for namespace registration is a SQL-callable
function. I expect Jim Nasby to do something like
SELECT pg_register_option_namespace('decibel');
ALTER TABLE nasby SET (decibel.seed = true);
which seems to cover what he wanted.
If you register a namespace, you can later do ALTER TABLE .. SET
(yournsp.foobar=blah) and your value will be stored in catalogs. Note
that if you have a C module, you can register the options themselves,
using add_bool_reloption() and so on; that way, your option will be
type-checked. If you don't add your options, they will not be
checked. This is in line with what we do for custom GUCs: if we know
about them, they are checked, otherwise we just pass them around
untouched.
Note one weird thing related to TOAST tables: we need to tell
transformRelOptions specifically whether we want custom namespaces to be
kept in its output or not. This is because we want such options in the
main table, but not in the toast table; and we call transformRelOptions
on both tables with the whole array of values. That's what the new
include_custom bit is for. For most callers that bit is true, but
when a table is being processed and the options are for the toast table,
that option is false.
Another thing to note is that I've separated the checking of the
namespaces from the transformation. There's actually very little
duplicated work that we're saving from doing things that way AFAICS, but
the new interface does make more sense than the old one. This is per
the thread I linked to previously. (Note there is surely a better way
to do the HEAP_RELOPT_NAMESPACES than a #define with the static const
char * const valid[] thingy sprinkled all over the place; I assume we
can just declare that once in the header. I will fix that later.)
I haven't touched pg_dump yet, but if this proposed design sits well
with everyone, my intention is that the dump output will contain the
pg_register_option_namespace() calls necessary so that a table
definition will be able to do the SET calls to set the values the
original table has, and succeed. In other words, restoring a dump will
preserve the values you had, without a need of having the module loaded
in the new server. I think this is what was discussed. Robert, do you
agree?
I think there is more work to do here, mainly to ensure that the
internal representation makes sense for C users (i.e. can extensions get
at the values they previously set). At this point I'm interested in
getting no objections to the SQL interface and the pg_dump bits.
--
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training Services
*** a/doc/src/sgml/ref/alter_index.sgml
--- b/doc/src/sgml/ref/alter_index.sgml
***
*** 82,87 ALTER INDEX [ IF EXISTS ] replaceable class=PARAMETERname/replaceable RESE
--- 82,93
xref linkend=SQL-REINDEX
to get the desired effects.
/para
+ note
+para
+ Custom storage parameters are of the form namespace.option. See
+ example below.
+/para
+ /note
/listitem
/varlistentry
***
*** 202,207 ALTER INDEX distributors SET (fillfactor = 75);
--- 208,219
REINDEX INDEX distributors;
/programlisting/para
+ para
+To set a custom storage parameter:
+ programlisting
+ ALTER INDEX distributors
+ SET (somenamespace.optionname=true);
+
/refsect1
refsect1
*** a/doc/src/sgml/ref/alter_table.sgml
--- b/doc/src/sgml/ref/alter_table.sgml
***
*** 213,218 ALTER TABLE [ IF EXISTS ] replaceable class=PARAMETERname/replaceable
--- 213,226
of statistics by the productnamePostgreSQL/productname query
planner, refer to xref linkend=planner-stats.
/para
+
+ note
+ para
+ Custom storage parameters are of the form namespace.option. See
+ example below.
+ /para
+ /note
+
/listitem
/varlistentry
***
*** 476,481 ALTER TABLE [ IF EXISTS ] replaceable
Re: [HACKERS] [PATCH] Store Extension Options
I am reworking this patch, both to accomodate earlier review comments and to fix the multiple verify step of namespaces, as noted by Tom in 4530.1390023...@sss.pgh.pa.us -- Álvaro Herrerahttp://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] [PATCH] Store Extension Options
On Fri, Mar 7, 2014 at 5:56 PM, Alvaro Herrera alvhe...@2ndquadrant.comwrote: I am reworking this patch, both to accomodate earlier review comments and to fix the multiple verify step of namespaces, as noted by Tom in 4530.1390023...@sss.pgh.pa.us This link is broken... -- Fabrízio de Royes Mello Consultoria/Coaching PostgreSQL Timbira: http://www.timbira.com.br Blog sobre TI: http://fabriziomello.blogspot.com Perfil Linkedin: http://br.linkedin.com/in/fabriziomello Twitter: http://twitter.com/fabriziomello
Re: [HACKERS] [PATCH] Store Extension Options
On Fri, Mar 7, 2014 at 7:21 PM, Andres Freund and...@2ndquadrant.comwrote: On 2014-03-07 19:14:48 -0300, Fabrízio de Royes Mello wrote: On Fri, Mar 7, 2014 at 5:56 PM, Alvaro Herrera alvhe...@2ndquadrant.com wrote: I am reworking this patch, both to accomodate earlier review comments and to fix the multiple verify step of namespaces, as noted by Tom in 4530.1390023...@sss.pgh.pa.us This link is broken... It is a message id, and it seemt o point to an appropriate thread? You can relatively easily lookup message ids using urls like http://www.postgresql.org/message-id/4530.1390023...@sss.pgh.pa.us Sorry... my fault!! Thanks! -- Fabrízio de Royes Mello Consultoria/Coaching PostgreSQL Timbira: http://www.timbira.com.br Blog sobre TI: http://fabriziomello.blogspot.com Perfil Linkedin: http://br.linkedin.com/in/fabriziomello Twitter: http://twitter.com/fabriziomello
Re: [HACKERS] [PATCH] Store Extension Options
On 2014-03-07 19:14:48 -0300, Fabrízio de Royes Mello wrote: On Fri, Mar 7, 2014 at 5:56 PM, Alvaro Herrera alvhe...@2ndquadrant.comwrote: I am reworking this patch, both to accomodate earlier review comments and to fix the multiple verify step of namespaces, as noted by Tom in 4530.1390023...@sss.pgh.pa.us This link is broken... It is a message id, and it seemt o point to an appropriate thread? You can relatively easily lookup message ids using urls like http://www.postgresql.org/message-id/4530.1390023...@sss.pgh.pa.us 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] [PATCH] Store Extension Options
On Fri, Mar 7, 2014 at 5:56 PM, Alvaro Herrera alvhe...@2ndquadrant.comwrote: I am reworking this patch, both to accomodate earlier review comments and to fix the multiple verify step of namespaces, as noted by Tom in 4530.1390023...@sss.pgh.pa.us Alvaro, Do you need some help? Regards, -- Fabrízio de Royes Mello Consultoria/Coaching PostgreSQL Timbira: http://www.timbira.com.br Blog sobre TI: http://fabriziomello.blogspot.com Perfil Linkedin: http://br.linkedin.com/in/fabriziomello Twitter: http://twitter.com/fabriziomello
Re: [HACKERS] [PATCH] Store Extension Options
Hi Fabrízio. Here are a few comments based on a quick look at your updated patch. At 2014-02-13 22:44:56 -0200, fabriziome...@gmail.com wrote: diff --git a/doc/src/sgml/ref/alter_index.sgml b/doc/src/sgml/ref/alter_index.sgml index d210077..5e9ee9d 100644 --- a/doc/src/sgml/ref/alter_index.sgml +++ b/doc/src/sgml/ref/alter_index.sgml @@ -82,6 +82,14 @@ ALTER INDEX [ IF EXISTS ] replaceable class=PARAMETERname/replaceable RESE xref linkend=SQL-REINDEX to get the desired effects. /para + note + para + A custom name can be used as namespace to define a storage parameter. + Storage option pattern: namespace.option=value + (namespace=custom name, option=option name and value=option value). + See example bellow. + /para + /note /listitem /varlistentry I was slightly confused by the wording here. I think it would be better to say something like Custom storage parameters are of the form namespace.option and leave it at that. (Aside: s/bellow/below/) @@ -202,6 +210,17 @@ ALTER INDEX distributors SET (fillfactor = 75); REINDEX INDEX distributors; /programlisting/para + para + To set a custom storage parameter: +programlisting +ALTER INDEX distributors + SET (bdr.do_replicate=true); +/programlisting + (bdr=custom name, do_replicate=option name and + true=option value) +/para + + /refsect1 It might be best to avoid using bdr.do_replicate in the example, since it's still a moving target. It might be better to use a generic example like somenamespace.optionname=true, in which case the explanation isn't needed either. The patch applies and builds fine, the tests pass, and the code looks OK to me. I don't have a strong opinion on validating custom reloption values through hooks as discussed earlier in the thread, but the simple version (i.e. your latest patch) seems at least a useful starting point. -- Abhijit -- 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] [PATCH] Store Extension Options
On Fri, Feb 28, 2014 at 5:08 AM, Abhijit Menon-Sen a...@2ndquadrant.com wrote: Hi Fabrízio. Here are a few comments based on a quick look at your updated patch. At 2014-02-13 22:44:56 -0200, fabriziome...@gmail.com wrote: diff --git a/doc/src/sgml/ref/alter_index.sgml b/doc/src/sgml/ref/alter_index.sgml index d210077..5e9ee9d 100644 --- a/doc/src/sgml/ref/alter_index.sgml +++ b/doc/src/sgml/ref/alter_index.sgml @@ -82,6 +82,14 @@ ALTER INDEX [ IF EXISTS ] replaceable class=PARAMETERname/replaceable RESE xref linkend=SQL-REINDEX to get the desired effects. /para + note + para + A custom name can be used as namespace to define a storage parameter. + Storage option pattern: namespace.option=value + (namespace=custom name, option=option name and value=option value). + See example bellow. + /para + /note /listitem /varlistentry I was slightly confused by the wording here. I think it would be better to say something like Custom storage parameters are of the form namespace.option and leave it at that. (Aside: s/bellow/below/) You are correct... my english isn't so good... sorry! Fixed. @@ -202,6 +210,17 @@ ALTER INDEX distributors SET (fillfactor = 75); REINDEX INDEX distributors; /programlisting/para + para + To set a custom storage parameter: +programlisting +ALTER INDEX distributors + SET (bdr.do_replicate=true); +/programlisting + (bdr=custom name, do_replicate=option name and + true=option value) +/para + + /refsect1 It might be best to avoid using bdr.do_replicate in the example, since it's still a moving target. It might be better to use a generic example like somenamespace.optionname=true, in which case the explanation isn't needed either. Fixed. The patch applies and builds fine, the tests pass, and the code looks OK to me. I don't have a strong opinion on validating custom reloption values through hooks as discussed earlier in the thread, but the simple version (i.e. your latest patch) seems at least a useful starting point. Thanks for your review. Regards, -- Fabrízio de Royes Mello Consultoria/Coaching PostgreSQL Timbira: http://www.timbira.com.br Blog sobre TI: http://fabriziomello.blogspot.com Perfil Linkedin: http://br.linkedin.com/in/fabriziomello Twitter: http://twitter.com/fabriziomello diff --git a/doc/src/sgml/ref/alter_index.sgml b/doc/src/sgml/ref/alter_index.sgml index 94a7af0..f0932cb 100644 --- a/doc/src/sgml/ref/alter_index.sgml +++ b/doc/src/sgml/ref/alter_index.sgml @@ -82,6 +82,12 @@ ALTER INDEX [ IF EXISTS ] replaceable class=PARAMETERname/replaceable RESE xref linkend=SQL-REINDEX to get the desired effects. /para + note + para + Custom storage parameters are of the form namespace.option. See + example below. + /para + /note /listitem /varlistentry @@ -202,6 +208,12 @@ ALTER INDEX distributors SET (fillfactor = 75); REINDEX INDEX distributors; /programlisting/para + para + To set a custom storage parameter: +programlisting +ALTER INDEX distributors + SET (somenamespace.optionname=true); + /refsect1 refsect1 diff --git a/doc/src/sgml/ref/alter_table.sgml b/doc/src/sgml/ref/alter_table.sgml index 2b02e66..1c36f4b 100644 --- a/doc/src/sgml/ref/alter_table.sgml +++ b/doc/src/sgml/ref/alter_table.sgml @@ -213,6 +213,14 @@ ALTER TABLE [ IF EXISTS ] replaceable class=PARAMETERname/replaceable of statistics by the productnamePostgreSQL/productname query planner, refer to xref linkend=planner-stats. /para + + note + para +Custom storage parameters are of the form namespace.option. See +example below. + /para + /note + /listitem /varlistentry @@ -476,6 +484,10 @@ ALTER TABLE [ IF EXISTS ] replaceable class=PARAMETERname/replaceable commandALTER TABLE/ does not treat literalOIDS/ as a storage parameter. Instead use the literalSET WITH OIDS/ and literalSET WITHOUT OIDS/ forms to change OID status. + A custom name can be used as namespace to define a storage parameter. + Storage option pattern: namespace.option=value + (namespace=custom name, option=option name and value=option value). + See example bellow. /para /note /listitem @@ -1112,6 +1124,20 @@ ALTER TABLE distributors DROP CONSTRAINT distributors_pkey, ADD CONSTRAINT distributors_pkey PRIMARY KEY USING INDEX dist_id_temp_idx; /programlisting/para + para + To set a custom per-attribute option: +programlisting +ALTER TABLE distributors + ALTER COLUMN dist_id SET (somenamespace.optionname=true); +/programlisting +/para + + para + To set a custom storage parameter: +programlisting +ALTER TABLE distributors + SET (somenamespace.optionname=true); + /refsect1 refsect1 diff --git
Re: [HACKERS] [PATCH] Store Extension Options
On Sun, Feb 9, 2014 at 2:22 PM, Fabrízio de Royes Mello fabriziome...@gmail.com wrote: On Sat, Jan 11, 2014 at 2:47 AM, Peter Eisentraut pete...@gmx.net wrote: On Sat, 2014-01-11 at 00:48 -0200, Fabrízio de Royes Mello wrote: Now, if bdr is installed but the validation doesn't happen unless bdr is loaded in some sense, then that is an implementation deficiency that I think we can insist be rectified before this feature is accepted. Check if extension is already installed is not enough for the first version of this feature? Elsewhere it was argued that tying this to extensions is not appropriate. I agree. It depends on how this feature is supposed to be used exactly. A replication plugin might very well be loaded via session_preload_libraries and not appear in SQL at all. In that case you need some C-level hook. In another case, an extension might want to inspect relation options from user-space triggers. So you'd need to register some SQL-level function for option validation. This could end up being two separate but overlapping features. Hi all, I taken this weekend to work on this patch and on monday or tuesday I'll send it. But I have some doubts: 1) I'm not convinced to tying this to extensions. I think this feature must enable us to just store a custom GUC. We can set custom GUCs in a backend session using SET class.variable = value, and this feature could just enable us to store it for relations/attributes. Without the complexity and overhead to register a function to validate them. That way we can use this feature to extensions and other needs too. 2) If we're implement the Robert's idea to have a function to validate the extension options then we must think about how a extension developer will register this function. Beacuse when we install a extension must have one way to get de pg_proc OID and store it in the pg_extension (or a different catalog). Or we'll implement some way to register this function at the SQL level, like ALTER EXTENSION bdr SET VALIDATE FUNCTION bdr_options_validate(); or another sintax of course. I don't know if you guys understood my concerns!! :-) Comments? Hi all, The attached patch implements the first option that I suggested before. Regards, -- Fabrízio de Royes Mello Consultoria/Coaching PostgreSQL Timbira: http://www.timbira.com.br Blog sobre TI: http://fabriziomello.blogspot.com Perfil Linkedin: http://br.linkedin.com/in/fabriziomello Twitter: http://twitter.com/fabriziomello diff --git a/doc/src/sgml/ref/alter_index.sgml b/doc/src/sgml/ref/alter_index.sgml index d210077..5e9ee9d 100644 --- a/doc/src/sgml/ref/alter_index.sgml +++ b/doc/src/sgml/ref/alter_index.sgml @@ -82,6 +82,14 @@ ALTER INDEX [ IF EXISTS ] replaceable class=PARAMETERname/replaceable RESE xref linkend=SQL-REINDEX to get the desired effects. /para + note + para + A custom name can be used as namespace to define a storage parameter. + Storage option pattern: namespace.option=value + (namespace=custom name, option=option name and value=option value). + See example bellow. + /para + /note /listitem /varlistentry @@ -202,6 +210,17 @@ ALTER INDEX distributors SET (fillfactor = 75); REINDEX INDEX distributors; /programlisting/para + para + To set a custom storage parameter: +programlisting +ALTER INDEX distributors + SET (bdr.do_replicate=true); +/programlisting + (bdr=custom name, do_replicate=option name and + true=option value) +/para + + /refsect1 refsect1 diff --git a/doc/src/sgml/ref/alter_table.sgml b/doc/src/sgml/ref/alter_table.sgml index 89649a2..6fd9d67 100644 --- a/doc/src/sgml/ref/alter_table.sgml +++ b/doc/src/sgml/ref/alter_table.sgml @@ -213,6 +213,16 @@ ALTER TABLE [ IF EXISTS ] replaceable class=PARAMETERname/replaceable of statistics by the productnamePostgreSQL/productname query planner, refer to xref linkend=planner-stats. /para + + note + para +A custom name can be used as namespace to define a storage parameter. +Storage option pattern: namespace.option=value +(namespace=custom name, option=option name and value=option value). +See example bellow. + /para + /note + /listitem /varlistentry @@ -476,6 +486,10 @@ ALTER TABLE [ IF EXISTS ] replaceable class=PARAMETERname/replaceable commandALTER TABLE/ does not treat literalOIDS/ as a storage parameter. Instead use the literalSET WITH OIDS/ and literalSET WITHOUT OIDS/ forms to change OID status. + A custom name can be used as namespace to define a storage parameter. + Storage option pattern: namespace.option=value + (namespace=custom name, option=option name and value=option value). + See example bellow. /para /note /listitem @@ -1112,6 +1126,26 @@ ALTER TABLE distributors DROP CONSTRAINT
Re: [HACKERS] [PATCH] Store Extension Options
On Sat, Jan 11, 2014 at 2:47 AM, Peter Eisentraut pete...@gmx.net wrote: On Sat, 2014-01-11 at 00:48 -0200, Fabrízio de Royes Mello wrote: Now, if bdr is installed but the validation doesn't happen unless bdr is loaded in some sense, then that is an implementation deficiency that I think we can insist be rectified before this feature is accepted. Check if extension is already installed is not enough for the first version of this feature? Elsewhere it was argued that tying this to extensions is not appropriate. I agree. It depends on how this feature is supposed to be used exactly. A replication plugin might very well be loaded via session_preload_libraries and not appear in SQL at all. In that case you need some C-level hook. In another case, an extension might want to inspect relation options from user-space triggers. So you'd need to register some SQL-level function for option validation. This could end up being two separate but overlapping features. Hi all, I taken this weekend to work on this patch and on monday or tuesday I'll send it. But I have some doubts: 1) I'm not convinced to tying this to extensions. I think this feature must enable us to just store a custom GUC. We can set custom GUCs in a backend session using SET class.variable = value, and this feature could just enable us to store it for relations/attributes. Without the complexity and overhead to register a function to validate them. That way we can use this feature to extensions and other needs too. 2) If we're implement the Robert's idea to have a function to validate the extension options then we must think about how a extension developer will register this function. Beacuse when we install a extension must have one way to get de pg_proc OID and store it in the pg_extension (or a different catalog). Or we'll implement some way to register this function at the SQL level, like ALTER EXTENSION bdr SET VALIDATE FUNCTION bdr_options_validate(); or another sintax of course. I don't know if you guys understood my concerns!! :-) Comments? Regards, -- Fabrízio de Royes Mello Consultoria/Coaching PostgreSQL Timbira: http://www.timbira.com.br Blog sobre TI: http://fabriziomello.blogspot.com Perfil Linkedin: http://br.linkedin.com/in/fabriziomello Twitter: http://twitter.com/fabriziomello
Re: [HACKERS] [PATCH] Store Extension Options
On Mon, Jan 6, 2014 at 1:50 AM, Tom Lane t...@sss.pgh.pa.us wrote: =?ISO-8859-1?Q?Fabr=EDzio_de_Royes_Mello?= fabriziome...@gmail.com writes: You are correct. pg_dump export reloptions using WITH clause of CREATE TABLE statement. I.e.: CREATE TABLE foo ( ) WITH (autovacuum_enabled=false, bdr.do_replicate=false); So if this statement checks for 'bdr' extension is loaded then in partial restore it can be fail. I see absolutely *nothing* wrong with failing that command if bdr is not installed. For an analogy, if this table includes a column of type bar defined by some extension baz, we are certainly going to fail the CREATE TABLE if baz isn't installed. Ok. Now, if bdr is installed but the validation doesn't happen unless bdr is loaded in some sense, then that is an implementation deficiency that I think we can insist be rectified before this feature is accepted. Check if extension is already installed is not enough for the first version of this feature? Regards, -- Fabrízio de Royes Mello Consultoria/Coaching PostgreSQL Timbira: http://www.timbira.com.br Blog sobre TI: http://fabriziomello.blogspot.com Perfil Linkedin: http://br.linkedin.com/in/fabriziomello Twitter: http://twitter.com/fabriziomello
Re: [HACKERS] [PATCH] Store Extension Options
On Sat, 2014-01-11 at 00:48 -0200, Fabrízio de Royes Mello wrote: Now, if bdr is installed but the validation doesn't happen unless bdr is loaded in some sense, then that is an implementation deficiency that I think we can insist be rectified before this feature is accepted. Check if extension is already installed is not enough for the first version of this feature? Elsewhere it was argued that tying this to extensions is not appropriate. I agree. It depends on how this feature is supposed to be used exactly. A replication plugin might very well be loaded via session_preload_libraries and not appear in SQL at all. In that case you need some C-level hook. In another case, an extension might want to inspect relation options from user-space triggers. So you'd need to register some SQL-level function for option validation. This could end up being two separate but overlapping features. -- 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] [PATCH] Store Extension Options
On Sun, Jan 5, 2014 at 10:50 PM, Tom Lane t...@sss.pgh.pa.us wrote: =?ISO-8859-1?Q?Fabr=EDzio_de_Royes_Mello?= fabriziome...@gmail.com writes: You are correct. pg_dump export reloptions using WITH clause of CREATE TABLE statement. I.e.: CREATE TABLE foo ( ) WITH (autovacuum_enabled=false, bdr.do_replicate=false); So if this statement checks for 'bdr' extension is loaded then in partial restore it can be fail. I see absolutely *nothing* wrong with failing that command if bdr is not installed. For an analogy, if this table includes a column of type bar defined by some extension baz, we are certainly going to fail the CREATE TABLE if baz isn't installed. Now, if bdr is installed but the validation doesn't happen unless bdr is loaded in some sense, then that is an implementation deficiency that I think we can insist be rectified before this feature is accepted. We could add a catalog pg_custom_reloption with a reloption namespace, a reloption name, and a pg_proc OID for a checker-function. This is a lot more overhead than just having a hook the way we do for GUCs, and I'm not sure how you'd handle invalidation, but in theory it solves the problem. -- 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] [PATCH] Store Extension Options
Robert Haas robertmh...@gmail.com writes: On Sun, Jan 5, 2014 at 10:50 PM, Tom Lane t...@sss.pgh.pa.us wrote: Now, if bdr is installed but the validation doesn't happen unless bdr is loaded in some sense, then that is an implementation deficiency that I think we can insist be rectified before this feature is accepted. We could add a catalog pg_custom_reloption with a reloption namespace, a reloption name, and a pg_proc OID for a checker-function. This is a lot more overhead than just having a hook the way we do for GUCs, and I'm not sure how you'd handle invalidation, but in theory it solves the problem. If we're willing to tie the reloption names to extension names, which seems reasonable to me, then we don't need a new catalog --- just add a checker-function column to pg_extension. I don't follow your point about invalidation. Once an extension has accepted a reloption value, it doesn't get to change its mind later; it has to deal with that value somehow forevermore. Using a hook, or failing to validate the value at all, certainly isn't going to make that requirement go away. 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] [PATCH] Store Extension Options
On Mon, Jan 6, 2014 at 2:58 PM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: On Sun, Jan 5, 2014 at 10:50 PM, Tom Lane t...@sss.pgh.pa.us wrote: Now, if bdr is installed but the validation doesn't happen unless bdr is loaded in some sense, then that is an implementation deficiency that I think we can insist be rectified before this feature is accepted. We could add a catalog pg_custom_reloption with a reloption namespace, a reloption name, and a pg_proc OID for a checker-function. This is a lot more overhead than just having a hook the way we do for GUCs, and I'm not sure how you'd handle invalidation, but in theory it solves the problem. If we're willing to tie the reloption names to extension names, which seems reasonable to me, then we don't need a new catalog --- just add a checker-function column to pg_extension. That seems sketchy to me. If somebody sets up a home-brew replication solution, I can't see why they should have to package it as an extension to register a reloption. So far, extensions are all about packaging, not functionality, and I'm not excited about changing that. I don't follow your point about invalidation. Once an extension has accepted a reloption value, it doesn't get to change its mind later; it has to deal with that value somehow forevermore. Using a hook, or failing to validate the value at all, certainly isn't going to make that requirement go away. Well... I'm assuming that the contents of the pg_custom_reloption catalog (or whatever catalog we use) would have to be loaded into some backend-local cache on first use, so it would need to be invalidated later as necessary. But come to think of it, that's actually not a problem at all; we can easily register a syscache callback and that should work fine. Not sure what I was worried about. -- 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] [PATCH] Store Extension Options
On 1/4/14, 12:00 PM, Tom Lane wrote: If you have some settings that need to be table-specific, then I agree that the reloptions infrastructure is a nice place to track them. What's actually missing here is some compelling examples of such settings for plausible extensions. I've got a real world example. At work we use limited dumps of production to support development. We do a schema dump and then grab data out of certain seed tables. To mark tables as being seed, we have to store that info in a table, but that gets us into another problem: what if a table gets renamed? In order to solve that problem, we created a tables table: CREATE TABLE tools.tables( id SERIAL PRIMARY KEY, table_schema text, table_name text ); That way if we need to rename a table we update one record in one place instead of a bunch of places (we have other code that makes use of tools.tables). (And no, we can't use OIDs because they're not immutable between dumps). This is obviously ugly and fragile. It'd be much better if we could just define a custom setting on the table itself that says hey, dump the data from here!. (FWIW, same applies to sequnces). Actually, I just checked and our seed object stuff doesn't use tools.tables, but our inheritance framework and a change notification system we wrote does. Those are other examples of where we need to store additional information about a table and had to create a system of our own to handle it. -- Jim C. Nasby, Data Architect j...@nasby.net 512.569.9461 (cell) http://jim.nasby.net -- 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] [PATCH] Store Extension Options
On Sat, Jan 4, 2014 at 1:00 PM, Tom Lane t...@sss.pgh.pa.us wrote: I would suggest addressing Robert's concern about lack of error checking by refusing to allow a custom reloption to be set unless the relevant extension is loaded and checks it. Unlike the postgresql.conf problem, I don't see any very good use-case for allowing an unchecked ALTER TABLE to occur. How do you plan to resolve the associated dump/restore hazard? AIUI, that's why we allow people define any old this.that GUC that they want without checking it - because the relevant shared library might not be loaded at the time of definition, but only by time of use. -- 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] [PATCH] Store Extension Options
Robert Haas robertmh...@gmail.com writes: On Sat, Jan 4, 2014 at 1:00 PM, Tom Lane t...@sss.pgh.pa.us wrote: I would suggest addressing Robert's concern about lack of error checking by refusing to allow a custom reloption to be set unless the relevant extension is loaded and checks it. Unlike the postgresql.conf problem, I don't see any very good use-case for allowing an unchecked ALTER TABLE to occur. How do you plan to resolve the associated dump/restore hazard? pg_dump creates extensions before tables, no? So what dump/restore hazard? AIUI, that's why we allow people define any old this.that GUC that they want without checking it - because the relevant shared library might not be loaded at the time of definition, but only by time of use. No, the reason we allow GUCs to be set before the relevant library is loaded is so that you can put a setting into postgresql.conf without thereby having to make the extension be load-into-postmaster. 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] [PATCH] Store Extension Options
On Sun, Jan 5, 2014 at 3:45 PM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: On Sat, Jan 4, 2014 at 1:00 PM, Tom Lane t...@sss.pgh.pa.us wrote: I would suggest addressing Robert's concern about lack of error checking by refusing to allow a custom reloption to be set unless the relevant extension is loaded and checks it. Unlike the postgresql.conf problem, I don't see any very good use-case for allowing an unchecked ALTER TABLE to occur. How do you plan to resolve the associated dump/restore hazard? pg_dump creates extensions before tables, no? So what dump/restore hazard? Creating the extension doesn't guarantee that the shared library will always be loaded. If nothing else, think about partial restores. -- 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] [PATCH] Store Extension Options
On Mon, Jan 6, 2014 at 1:08 AM, Robert Haas robertmh...@gmail.com wrote: On Sun, Jan 5, 2014 at 3:45 PM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: On Sat, Jan 4, 2014 at 1:00 PM, Tom Lane t...@sss.pgh.pa.us wrote: I would suggest addressing Robert's concern about lack of error checking by refusing to allow a custom reloption to be set unless the relevant extension is loaded and checks it. Unlike the postgresql.conf problem, I don't see any very good use-case for allowing an unchecked ALTER TABLE to occur. How do you plan to resolve the associated dump/restore hazard? pg_dump creates extensions before tables, no? So what dump/restore hazard? Creating the extension doesn't guarantee that the shared library will always be loaded. If nothing else, think about partial restores. You are correct. pg_dump export reloptions using WITH clause of CREATE TABLE statement. I.e.: CREATE TABLE foo ( ) WITH (autovacuum_enabled=false, bdr.do_replicate=false); So if this statement checks for 'bdr' extension is loaded then in partial restore it can be fail. At this point we have two choices: 1) do not check if extension already is loaded 2) hack the pg_dump to produce an ALTER TABLE ... SET (...) instead of CREATE TABLE ... WITH (...) to set reloptions Comments? -- Fabrízio de Royes Mello Consultoria/Coaching PostgreSQL Timbira: http://www.timbira.com.br Blog sobre TI: http://fabriziomello.blogspot.com Perfil Linkedin: http://br.linkedin.com/in/fabriziomello Twitter: http://twitter.com/fabriziomello
Re: [HACKERS] [PATCH] Store Extension Options
Robert Haas robertmh...@gmail.com writes: On Sun, Jan 5, 2014 at 3:45 PM, Tom Lane t...@sss.pgh.pa.us wrote: pg_dump creates extensions before tables, no? So what dump/restore hazard? Creating the extension doesn't guarantee that the shared library will always be loaded. No, but unless the plan is that no validation happens at all (which I gather is not your desire) then there must be some mechanism for figuring out which extension owns a given reloption and asking it to validate the value. This might be more complicated than a passive hook, but I don't feel bad about demanding that it work like that. 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] [PATCH] Store Extension Options
=?ISO-8859-1?Q?Fabr=EDzio_de_Royes_Mello?= fabriziome...@gmail.com writes: You are correct. pg_dump export reloptions using WITH clause of CREATE TABLE statement. I.e.: CREATE TABLE foo ( ) WITH (autovacuum_enabled=false, bdr.do_replicate=false); So if this statement checks for 'bdr' extension is loaded then in partial restore it can be fail. I see absolutely *nothing* wrong with failing that command if bdr is not installed. For an analogy, if this table includes a column of type bar defined by some extension baz, we are certainly going to fail the CREATE TABLE if baz isn't installed. Now, if bdr is installed but the validation doesn't happen unless bdr is loaded in some sense, then that is an implementation deficiency that I think we can insist be rectified before this feature is accepted. 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] [PATCH] Store Extension Options
Enviado via iPhone Em 02/01/2014, às 22:16, Robert Haas robertmh...@gmail.com escreveu: On Thu, Jan 2, 2014 at 4:19 AM, Andres Freund and...@2ndquadrant.com wrote: On 2013-12-31 13:37:59 +0100, Pavel Stehule wrote: We use the namespace ext to the internal code (src/backend/access/common/reloptions.c) skip some validations and store the custom GUC. Do you think we don't need to use the ext namespace? yes - there be same mechanism as we use for GUC There is no existing mechanism to handle conflicts for GUCs. The difference is that for GUCs nearly no namespaced GUCs exist (plperl, plpgsql have some), but postgres defines at least autovacuum. and toast. namespaces for relation options. I continue to think that the case for having this feature at all has not been well-made. We can use this feature to store any custom GUC for relations, attributes and functions also. Some use cases: * extension options * config for external apps (frameworks, third part software) Comments? Regards, Fabrízio Mello -- 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] [PATCH] Store Extension Options
On Sat, Jan 4, 2014 at 11:07 AM, Fabrizio Mello fabriziome...@gmail.com wrote: I continue to think that the case for having this feature at all has not been well-made. We can use this feature to store any custom GUC for relations, attributes and functions also. Some use cases: * extension options * config for external apps (frameworks, third part software) Comments? Well, as I said before, somebody can make their own configuration table and store stuff there, rather than using pg_class.reloptions. As I recall, the only response to that proposal was well, they might not want to do it that way, which does not strike me as a sufficient reason. What we've basically settled into for GUCs is that you can register a custom GUC, but unless the module is loaded we'll accept any value for that GUC without checking it. We'd presumably need a similar mechanism here, or maybe you're proposing that we accept any reloption at all with any associated value whatsoever, so long as the prefix is ext. The first seems like an extension of an existing kludge of which I'm not overly found, and the second is an even larger kludge. In my experience as a software developer, there are very few places where it's useful to accept and store user input without any validation whatsoever, and I doubt that this is one of them. -- 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] [PATCH] Store Extension Options
On 2014-01-04 11:54:46 -0500, Robert Haas wrote: On Sat, Jan 4, 2014 at 11:07 AM, Fabrizio Mello fabriziome...@gmail.com wrote: I continue to think that the case for having this feature at all has not been well-made. We can use this feature to store any custom GUC for relations, attributes and functions also. Some use cases: * extension options * config for external apps (frameworks, third part software) Comments? Well, as I said before, somebody can make their own configuration table and store stuff there, rather than using pg_class.reloptions. As I recall, the only response to that proposal was well, they might not want to do it that way, which does not strike me as a sufficient reason. Well, there's some things you get by that integration: * Proper dependency tracking when relations are dropped renamed * Sensible integration into pg_dump, with only the relevant options being dumped/restored on partial dump/restores * Caching of values, with proper cache invalidation Sure, you can implement both using event triggers and relcache invalidation callbacks, but that's not something we want several extensions to do independently. 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] [PATCH] Store Extension Options
Andres Freund and...@2ndquadrant.com writes: On 2014-01-04 11:54:46 -0500, Robert Haas wrote: Well, as I said before, somebody can make their own configuration table and store stuff there, rather than using pg_class.reloptions. As I recall, the only response to that proposal was well, they might not want to do it that way, which does not strike me as a sufficient reason. Well, there's some things you get by that integration: * Proper dependency tracking when relations are dropped renamed * Sensible integration into pg_dump, with only the relevant options being dumped/restored on partial dump/restores * Caching of values, with proper cache invalidation If you have some settings that need to be table-specific, then I agree that the reloptions infrastructure is a nice place to track them. What's actually missing here is some compelling examples of such settings for plausible extensions. (The original example was pure handwaving, as I don't believe it's possible to build a replication extension with no core-code changes. As long as you need some of those, patching in a few more standard reloptions is hardly a deal-breaker.) Assuming that such examples are forthcoming, though, I think my main objection to this proposal is the ext. prefix, which seems precisely 100% useless, not to mention inconsistent with the naming of custom GUCs, which the same extension might well have some of. I think that custom reloptions should just have names like extension_name.option_name, the same as custom GUCs do. We have enough experience now with custom GUCs that I don't think it's unreasonable to model the behavior of custom reloptions on them as closely as possible. I would suggest addressing Robert's concern about lack of error checking by refusing to allow a custom reloption to be set unless the relevant extension is loaded and checks it. Unlike the postgresql.conf problem, I don't see any very good use-case for allowing an unchecked ALTER TABLE to occur. 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] [PATCH] Store Extension Options
On 2014-01-04 13:00:03 -0500, Tom Lane wrote: Andres Freund and...@2ndquadrant.com writes: On 2014-01-04 11:54:46 -0500, Robert Haas wrote: Well, as I said before, somebody can make their own configuration table and store stuff there, rather than using pg_class.reloptions. As I recall, the only response to that proposal was well, they might not want to do it that way, which does not strike me as a sufficient reason. Well, there's some things you get by that integration: * Proper dependency tracking when relations are dropped renamed * Sensible integration into pg_dump, with only the relevant options being dumped/restored on partial dump/restores * Caching of values, with proper cache invalidation If you have some settings that need to be table-specific, then I agree that the reloptions infrastructure is a nice place to track them. What's actually missing here is some compelling examples of such settings for plausible extensions. I don't know about others, but I would like to use it for bdr to configure which table is replicated where. I.e. something like bdr.replication_set=a,b,c, (The original example was pure handwaving, as I don't believe it's possible to build a replication extension with no core-code changes. As long as you need some of those, patching in a few more standard reloptions is hardly a deal-breaker.) Well, slony et al exist, so it's certainly possible. And even if you want to talk about logical replication, there aren't that much core changes required - and all of them have been submitted. They might not make it into 9.4 but I certainly plan to pursue things further so it's possible to stuff without patching core. Assuming that such examples are forthcoming, though, I think my main objection to this proposal is the ext. prefix, which seems precisely 100% useless, not to mention inconsistent with the naming of custom GUCs, which the same extension might well have some of. Well, the argument is/was that it avoid conflicts with future core code adding more namespaces - like the already existing toast. prefix. If we say we can live with the possibility of such conflicts, it seems appropriate not to use ext. as a prefix. I would suggest addressing Robert's concern about lack of error checking by refusing to allow a custom reloption to be set unless the relevant extension is loaded and checks it. Unlike the postgresql.conf problem, I don't see any very good use-case for allowing an unchecked ALTER TABLE Fine with me. 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] [PATCH] Store Extension Options
Andres Freund and...@2ndquadrant.com writes: On 2014-01-04 13:00:03 -0500, Tom Lane wrote: Assuming that such examples are forthcoming, though, I think my main objection to this proposal is the ext. prefix, which seems precisely 100% useless, not to mention inconsistent with the naming of custom GUCs, which the same extension might well have some of. Well, the argument is/was that it avoid conflicts with future core code adding more namespaces - like the already existing toast. prefix. If we say we can live with the possibility of such conflicts, it seems appropriate not to use ext. as a prefix. And if we have ext. as a prefix, exactly what prevents conflicts in the second part of the name? Nothing, that's what. It's useless. 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] [PATCH] Store Extension Options
On 2014-01-04 14:06:19 -0500, Tom Lane wrote: Andres Freund and...@2ndquadrant.com writes: On 2014-01-04 13:00:03 -0500, Tom Lane wrote: Assuming that such examples are forthcoming, though, I think my main objection to this proposal is the ext. prefix, which seems precisely 100% useless, not to mention inconsistent with the naming of custom GUCs, which the same extension might well have some of. Well, the argument is/was that it avoid conflicts with future core code adding more namespaces - like the already existing toast. prefix. If we say we can live with the possibility of such conflicts, it seems appropriate not to use ext. as a prefix. And if we have ext. as a prefix, exactly what prevents conflicts in the second part of the name? Nothing, that's what. It's useless. Uh? We are certainly not going to add core code that defines relation options with ext. in the name like we've introduced toast.fillfactor et al? 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] [PATCH] Store Extension Options
Andres Freund and...@2ndquadrant.com writes: On 2014-01-04 14:06:19 -0500, Tom Lane wrote: And if we have ext. as a prefix, exactly what prevents conflicts in the second part of the name? Nothing, that's what. It's useless. Uh? We are certainly not going to add core code that defines relation options with ext. in the name like we've introduced toast.fillfactor et al? If this feature is of any use, surely we should assume that more than one extension will use it. If those extensions are separately developed, there's nothing preventing name conflicts. I would rank the odds of two people writing my_replication_extension a lot higher than the odds of the core code deciding to use such a prefix. What's more, what happens if we decide to migrate some such extension into core? A hard and fast division between names allowed to external and internal features is just going to bite us on the rear eventually. 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] [PATCH] Store Extension Options
On 2013-12-31 13:37:59 +0100, Pavel Stehule wrote: We use the namespace ext to the internal code (src/backend/access/common/reloptions.c) skip some validations and store the custom GUC. Do you think we don't need to use the ext namespace? yes - there be same mechanism as we use for GUC There is no existing mechanism to handle conflicts for GUCs. The difference is that for GUCs nearly no namespaced GUCs exist (plperl, plpgsql have some), but postgres defines at least autovacuum. and toast. namespaces for relation options. 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] [PATCH] Store Extension Options
On Thu, Jan 2, 2014 at 7:19 AM, Andres Freund and...@2ndquadrant.com wrote: On 2013-12-31 13:37:59 +0100, Pavel Stehule wrote: We use the namespace ext to the internal code (src/backend/access/common/reloptions.c) skip some validations and store the custom GUC. Do you think we don't need to use the ext namespace? yes - there be same mechanism as we use for GUC There is no existing mechanism to handle conflicts for GUCs. The difference is that for GUCs nearly no namespaced GUCs exist (plperl, plpgsql have some), but postgres defines at least autovacuum. and toast. namespaces for relation options. autovacuum. namespace ??? The HEAP_RELOPT_NAMESPACES (src/include/access/reloptions.h) constant define only toast and null as a valid relation option namespace. I missed something? Regards, -- Fabrízio de Royes Mello Consultoria/Coaching PostgreSQL Timbira: http://www.timbira.com.br Blog sobre TI: http://fabriziomello.blogspot.com Perfil Linkedin: http://br.linkedin.com/in/fabriziomello Twitter: http://twitter.com/fabriziomello
Re: [HACKERS] [PATCH] Store Extension Options
On 2014-01-02 08:26:20 -0200, Fabrízio de Royes Mello wrote: On Thu, Jan 2, 2014 at 7:19 AM, Andres Freund and...@2ndquadrant.com wrote: On 2013-12-31 13:37:59 +0100, Pavel Stehule wrote: We use the namespace ext to the internal code (src/backend/access/common/reloptions.c) skip some validations and store the custom GUC. Do you think we don't need to use the ext namespace? yes - there be same mechanism as we use for GUC There is no existing mechanism to handle conflicts for GUCs. The difference is that for GUCs nearly no namespaced GUCs exist (plperl, plpgsql have some), but postgres defines at least autovacuum. and toast. namespaces for relation options. autovacuum. namespace ??? Yea, right, it's autovacuum_... 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] [PATCH] Store Extension Options
On Thu, Jan 2, 2014 at 4:19 AM, Andres Freund and...@2ndquadrant.com wrote: On 2013-12-31 13:37:59 +0100, Pavel Stehule wrote: We use the namespace ext to the internal code (src/backend/access/common/reloptions.c) skip some validations and store the custom GUC. Do you think we don't need to use the ext namespace? yes - there be same mechanism as we use for GUC There is no existing mechanism to handle conflicts for GUCs. The difference is that for GUCs nearly no namespaced GUCs exist (plperl, plpgsql have some), but postgres defines at least autovacuum. and toast. namespaces for relation options. I continue to think that the case for having this feature at all has not been well-made. -- 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] [PATCH] Store Extension Options
On Tue, Dec 31, 2013 at 9:08 AM, Pavel Stehule pavel.steh...@gmail.com wrote: Hello I am looking on this patch ALTER TABLE foo SET (ext.somext.do_replicate=true); Why is there fixed prefix ext ? This feature is similar to attaching setting to function CREATE OR REPLACE FUNCTION ... SET var = ...; We can use someprefix.someguc without problems there. Hi, We use the prefix ext (aka namespace) to distinguish these options which are related to extensions. Have you seen the previous thread [1] ? Regards, [1] http://www.postgresql.org/message-id/CAFcNs+rqCq1H5eXW-cvdti6T-xo2STEkhjREx=odmakk5ti...@mail.gmail.com -- Fabrízio de Royes Mello Consultoria/Coaching PostgreSQL Timbira: http://www.timbira.com.br Blog sobre TI: http://fabriziomello.blogspot.com Perfil Linkedin: http://br.linkedin.com/in/fabriziomello Twitter: http://twitter.com/fabriziomello
Re: [HACKERS] [PATCH] Store Extension Options
2013/12/31 Fabrízio de Royes Mello fabriziome...@gmail.com On Tue, Dec 31, 2013 at 9:08 AM, Pavel Stehule pavel.steh...@gmail.com wrote: Hello I am looking on this patch ALTER TABLE foo SET (ext.somext.do_replicate=true); Why is there fixed prefix ext ? This feature is similar to attaching setting to function CREATE OR REPLACE FUNCTION ... SET var = ...; We can use someprefix.someguc without problems there. Hi, We use the prefix ext (aka namespace) to distinguish these options which are related to extensions. Have you seen the previous thread [1] ? yes, but I don't understand why it is necessary? I use a analogy with custom GUC - and there we don't use similar prefix. Only any prefix is required - and it can contain a dot. Regards Pavel Regards, [1] http://www.postgresql.org/message-id/CAFcNs+rqCq1H5eXW-cvdti6T-xo2STEkhjREx=odmakk5ti...@mail.gmail.com -- Fabrízio de Royes Mello Consultoria/Coaching PostgreSQL Timbira: http://www.timbira.com.br Blog sobre TI: http://fabriziomello.blogspot.com Perfil Linkedin: http://br.linkedin.com/in/fabriziomello Twitter: http://twitter.com/fabriziomello
Re: [HACKERS] [PATCH] Store Extension Options
On Tue, Dec 31, 2013 at 9:38 AM, Pavel Stehule pavel.steh...@gmail.com wrote: 2013/12/31 Fabrízio de Royes Mello fabriziome...@gmail.com On Tue, Dec 31, 2013 at 9:08 AM, Pavel Stehule pavel.steh...@gmail.com wrote: Hello I am looking on this patch ALTER TABLE foo SET (ext.somext.do_replicate=true); Why is there fixed prefix ext ? This feature is similar to attaching setting to function CREATE OR REPLACE FUNCTION ... SET var = ...; We can use someprefix.someguc without problems there. Hi, We use the prefix ext (aka namespace) to distinguish these options which are related to extensions. Have you seen the previous thread [1] ? yes, but I don't understand why it is necessary? I use a analogy with custom GUC - and there we don't use similar prefix. Only any prefix is required - and it can contain a dot. We use the namespace ext to the internal code (src/backend/access/common/reloptions.c) skip some validations and store the custom GUC. Do you think we don't need to use the ext namespace? Regards, -- Fabrízio de Royes Mello Consultoria/Coaching PostgreSQL Timbira: http://www.timbira.com.br Blog sobre TI: http://fabriziomello.blogspot.com Perfil Linkedin: http://br.linkedin.com/in/fabriziomello Twitter: http://twitter.com/fabriziomello
Re: [HACKERS] [PATCH] Store Extension Options
2013/12/31 Fabrízio de Royes Mello fabriziome...@gmail.com On Tue, Dec 31, 2013 at 9:38 AM, Pavel Stehule pavel.steh...@gmail.com wrote: 2013/12/31 Fabrízio de Royes Mello fabriziome...@gmail.com On Tue, Dec 31, 2013 at 9:08 AM, Pavel Stehule pavel.steh...@gmail.com wrote: Hello I am looking on this patch ALTER TABLE foo SET (ext.somext.do_replicate=true); Why is there fixed prefix ext ? This feature is similar to attaching setting to function CREATE OR REPLACE FUNCTION ... SET var = ...; We can use someprefix.someguc without problems there. Hi, We use the prefix ext (aka namespace) to distinguish these options which are related to extensions. Have you seen the previous thread [1] ? yes, but I don't understand why it is necessary? I use a analogy with custom GUC - and there we don't use similar prefix. Only any prefix is required - and it can contain a dot. We use the namespace ext to the internal code (src/backend/access/common/reloptions.c) skip some validations and store the custom GUC. Do you think we don't need to use the ext namespace? yes - there be same mechanism as we use for GUC Pavel Regards, -- Fabrízio de Royes Mello Consultoria/Coaching PostgreSQL Timbira: http://www.timbira.com.br Blog sobre TI: http://fabriziomello.blogspot.com Perfil Linkedin: http://br.linkedin.com/in/fabriziomello Twitter: http://twitter.com/fabriziomello
Re: [HACKERS] [PATCH] Store Extension Options
On Tue, Dec 31, 2013 at 10:37 AM, Pavel Stehule pavel.steh...@gmail.com
wrote:
2013/12/31 Fabrízio de Royes Mello fabriziome...@gmail.com
On Tue, Dec 31, 2013 at 9:38 AM, Pavel Stehule pavel.steh...@gmail.com
wrote:
2013/12/31 Fabrízio de Royes Mello fabriziome...@gmail.com
On Tue, Dec 31, 2013 at 9:08 AM, Pavel Stehule
pavel.steh...@gmail.com wrote:
Hello
I am looking on this patch
ALTER TABLE foo SET (ext.somext.do_replicate=true);
Why is there fixed prefix ext ?
This feature is similar to attaching setting to function
CREATE OR REPLACE FUNCTION ... SET var = ...;
We can use someprefix.someguc without problems there.
Hi,
We use the prefix ext (aka namespace) to distinguish these options
which are related to extensions.
Have you seen the previous thread [1] ?
yes, but I don't understand why it is necessary? I use a analogy with
custom GUC - and there we don't use similar prefix. Only any prefix is
required - and it can contain a dot.
We use the namespace ext to the internal code
(src/backend/access/common/reloptions.c) skip some validations and store
the custom GUC.
Do you think we don't need to use the ext namespace?
yes - there be same mechanism as we use for GUC
If we going to that way then we can expand the use of this patch to store
custom GUCs to functions also, and we can wrote a function (like
current_setting) to get specific GUC values, like:
ALTER TABLE foo SET (myextension.option=on);
SELECT current_setting('foo'::regclass, 'myextension.option');
Comments?
--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
Timbira: http://www.timbira.com.br
Blog sobre TI: http://fabriziomello.blogspot.com
Perfil Linkedin: http://br.linkedin.com/in/fabriziomello
Twitter: http://twitter.com/fabriziomello
Re: [HACKERS] [PATCH] Store Extension Options
2013/12/31 Fabrízio de Royes Mello fabriziome...@gmail.com
On Tue, Dec 31, 2013 at 10:37 AM, Pavel Stehule pavel.steh...@gmail.com
wrote:
2013/12/31 Fabrízio de Royes Mello fabriziome...@gmail.com
On Tue, Dec 31, 2013 at 9:38 AM, Pavel Stehule pavel.steh...@gmail.com
wrote:
2013/12/31 Fabrízio de Royes Mello fabriziome...@gmail.com
On Tue, Dec 31, 2013 at 9:08 AM, Pavel Stehule
pavel.steh...@gmail.com wrote:
Hello
I am looking on this patch
ALTER TABLE foo SET (ext.somext.do_replicate=true);
Why is there fixed prefix ext ?
This feature is similar to attaching setting to function
CREATE OR REPLACE FUNCTION ... SET var = ...;
We can use someprefix.someguc without problems there.
Hi,
We use the prefix ext (aka namespace) to distinguish these options
which are related to extensions.
Have you seen the previous thread [1] ?
yes, but I don't understand why it is necessary? I use a analogy with
custom GUC - and there we don't use similar prefix. Only any prefix is
required - and it can contain a dot.
We use the namespace ext to the internal code
(src/backend/access/common/reloptions.c) skip some validations and store
the custom GUC.
Do you think we don't need to use the ext namespace?
yes - there be same mechanism as we use for GUC
If we going to that way then we can expand the use of this patch to store
custom GUCs to functions also, and we can wrote a function (like
current_setting) to get specific GUC values, like:
ALTER TABLE foo SET (myextension.option=on);
SELECT current_setting('foo'::regclass, 'myextension.option');
I like it
Pavel
Comments?
--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
Timbira: http://www.timbira.com.br
Blog sobre TI: http://fabriziomello.blogspot.com
Perfil Linkedin: http://br.linkedin.com/in/fabriziomello
Twitter: http://twitter.com/fabriziomello
Re: [HACKERS] [PATCH] Store Extension Options
On Thu, Nov 21, 2013 at 11:06 AM, Robert Haas robertmh...@gmail.com wrote: On Wed, Nov 20, 2013 at 9:35 PM, Fabrízio de Royes Mello fabriziome...@gmail.com wrote: So, with this patch we can do that: ALTER TABLE foo SET (ext.somext.do_replicate=true); When 'ext' is the fixed prefix, 'somext' is the extension name, 'do_replicate' is the extension option and 'true' is the value. This doesn't seem like a particular good choice of syntax; What's your syntax suggestion? I dunno, but I doubt that hardcoding ext as an abbreviation for extension is a good decision. I also doubt that any fixed prefix is a good decision. I use this form to simplify implementation and not change sql syntax, but we can discuss another way or syntax. and I also have my doubts about the usefulness of the feature. This feature can be used for replication solutions, but also can be used for any extension that need do some specific configuration about tables, attributes and/or indexes. So, create your own configuration table with a column of type regclass. This can be a solution, but with a config table we have some problems: a) no dependency tracking (pg_depend) b) no correct locking c) no relcache d) harder to do correctly for columns Regards, -- Fabrízio de Royes Mello Consultoria/Coaching PostgreSQL Timbira: http://www.timbira.com.br Blog sobre TI: http://fabriziomello.blogspot.com Perfil Linkedin: http://br.linkedin.com/in/fabriziomello Twitter: http://twitter.com/fabriziomello
Re: [HACKERS] [PATCH] Store Extension Options
On Wed, Nov 20, 2013 at 9:35 PM, Fabrízio de Royes Mello fabriziome...@gmail.com wrote: So, with this patch we can do that: ALTER TABLE foo SET (ext.somext.do_replicate=true); When 'ext' is the fixed prefix, 'somext' is the extension name, 'do_replicate' is the extension option and 'true' is the value. This doesn't seem like a particular good choice of syntax; What's your syntax suggestion? I dunno, but I doubt that hardcoding ext as an abbreviation for extension is a good decision. I also doubt that any fixed prefix is a good decision. and I also have my doubts about the usefulness of the feature. This feature can be used for replication solutions, but also can be used for any extension that need do some specific configuration about tables, attributes and/or indexes. So, create your own configuration table with a column of type regclass. -- 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] [PATCH] Store Extension Options
On Wed, Nov 20, 2013 at 1:52 PM, Fabrízio de Royes Mello fabriziome...@gmail.com wrote: The main goal of this patch is enable to an user the capability to store options (relations and attributes) related to extensions by using a fixed prefix called 'ext' in the defined name. It's cant be useful for replication solutions. So, with this patch we can do that: ALTER TABLE foo SET (ext.somext.do_replicate=true); When 'ext' is the fixed prefix, 'somext' is the extension name, 'do_replicate' is the extension option and 'true' is the value. This doesn't seem like a particular good choice of syntax; and I also have my doubts about the usefulness of the feature. -- 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] [PATCH] Store Extension Options
On Thu, Nov 21, 2013 at 12:05 AM, Robert Haas robertmh...@gmail.com wrote: On Wed, Nov 20, 2013 at 1:52 PM, Fabrízio de Royes Mello fabriziome...@gmail.com wrote: The main goal of this patch is enable to an user the capability to store options (relations and attributes) related to extensions by using a fixed prefix called 'ext' in the defined name. It's cant be useful for replication solutions. So, with this patch we can do that: ALTER TABLE foo SET (ext.somext.do_replicate=true); When 'ext' is the fixed prefix, 'somext' is the extension name, 'do_replicate' is the extension option and 'true' is the value. This doesn't seem like a particular good choice of syntax; What's your syntax suggestion? and I also have my doubts about the usefulness of the feature. This feature can be used for replication solutions, but also can be used for any extension that need do some specific configuration about tables, attributes and/or indexes. Regards, -- Fabrízio de Royes Mello Consultoria/Coaching PostgreSQL Timbira: http://www.timbira.com.br Blog sobre TI: http://fabriziomello.blogspot.com Perfil Linkedin: http://br.linkedin.com/in/fabriziomello Twitter: http://twitter.com/fabriziomello
