Обсуждение: [PATCH] Store Extension Options
Hello
I am looking on this patchALTER TABLE foo SET (ext.somext.do_replicate=true);Why is there fixed prefix "ext" ?
CREATE OR REPLACE FUNCTION ... SET var = ...;We can use someprefix.someguc without problems there.
Regards
Pavel
Pavel
<div dir="ltr"><br />On Tue, Dec 31, 2013 at 9:08 AM, Pavel Stehule <<a href="mailto:pavel.stehule@gmail.com">pavel.stehule@gmail.com</a>>wrote:<br />><br />> Hello<br />><br />>I am looking on this patch<br />><br /> > ALTER TABLE foo SET (ext.somext.do_replicate=true);<br />><br />>Why is there fixed prefix "ext" ?<br />><br />> This feature is similar to attaching setting to function<br />><br/>> CREATE OR REPLACE FUNCTION ... SET var = ...;<br /> ><br />> We can use someprefix.someguc withoutproblems there.<br />><br /><br />Hi,<br /><br />We use the prefix "ext" (aka namespace) to distinguish these optionswhich are related to "extensions".<br /><br /> Have you seen the previous thread [1] ?<br /><br />Regards,<br /><br/>[1] <a href="http://www.postgresql.org/message-id/CAFcNs+rqCq1H5eXW-cvdti6T-xo2STEkhjREx=OdmAkK5tiOOw@mail.gmail.com">http://www.postgresql.org/message-id/CAFcNs+rqCq1H5eXW-cvdti6T-xo2STEkhjREx=OdmAkK5tiOOw@mail.gmail.com</a><br /><br/>--<br />Fabrízio de Royes Mello<br />Consultoria/Coaching PostgreSQL<br />>> Timbira: <a href="http://www.timbira.com.br">http://www.timbira.com.br</a><br/>>> Blog sobre TI: <a href="http://fabriziomello.blogspot.com">http://fabriziomello.blogspot.com</a><br/> >> Perfil Linkedin: <a href="http://br.linkedin.com/in/fabriziomello">http://br.linkedin.com/in/fabriziomello</a><br/>>> Twitter: <a href="http://twitter.com/fabriziomello">http://twitter.com/fabriziomello</a></div>
2013/12/31 Fabrízio de Royes Mello <fabriziomello@gmail.com>
Hi,
On Tue, Dec 31, 2013 at 9:08 AM, Pavel Stehule <pavel.stehule@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.
>
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
Pavel
Regards,
[1] http://www.postgresql.org/message-id/CAFcNs+rqCq1H5eXW-cvdti6T-xo2STEkhjREx=OdmAkK5tiOOw@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
<div dir="ltr"><div class="gmail_extra"><br />On Tue, Dec 31, 2013 at 9:38 AM, Pavel Stehule <<a href="mailto:pavel.stehule@gmail.com">pavel.stehule@gmail.com</a>>wrote:<br />><br />><br />> 2013/12/31 Fabríziode Royes Mello <<a href="mailto:fabriziomello@gmail.com">fabriziomello@gmail.com</a>><br /> >><br />>><br/>>> On Tue, Dec 31, 2013 at 9:08 AM, Pavel Stehule <<a href="mailto:pavel.stehule@gmail.com">pavel.stehule@gmail.com</a>>wrote:<br />>> ><br />>> > Hello<br/>>> ><br /> >> > I am looking on this patch<br />>> ><br />>> > ALTER TABLEfoo SET (ext.somext.do_replicate=true);<br />>> ><br />>> > Why is there fixed prefix "ext" ?<br />>>><br /> >> > This feature is similar to attaching setting to function<br />>> ><br />>>> CREATE OR REPLACE FUNCTION ... SET var = ...;<br />>> ><br />>> > We can use someprefix.somegucwithout problems there.<br /> >> ><br />>><br />>> Hi,<br />>><br />>>We use the prefix "ext" (aka namespace) to distinguish these options which are related to "extensions".<br />>><br/>>> Have you seen the previous thread [1] ?<br /> ><br />><br />> yes, but I don't understandwhy it is necessary? I use a analogy with custom GUC - and there we don't use similar prefix. Only any prefix isrequired - and it can contain a dot.<br />><br /><br /> We use the namespace "ext" to the internal code (src/backend/access/common/reloptions.c)skip some validations and store the custom GUC.<br /><br />Do you think we don'tneed to use the "ext" namespace?<br /><br /><br />Regards,<br /><br />--<br />Fabrízio de Royes Mello<br />Consultoria/CoachingPostgreSQL<br />>> Timbira: <a href="http://www.timbira.com.br">http://www.timbira.com.br</a><br/>>> Blog sobre TI: <a href="http://fabriziomello.blogspot.com">http://fabriziomello.blogspot.com</a><br/> >> Perfil Linkedin: <a href="http://br.linkedin.com/in/fabriziomello">http://br.linkedin.com/in/fabriziomello</a><br/>>> Twitter: <a href="http://twitter.com/fabriziomello">http://twitter.com/fabriziomello</a></div></div>
2013/12/31 Fabrízio de Royes Mello <fabriziomello@gmail.com>
We use the namespace "ext" to the internal code (src/backend/access/common/reloptions.c) skip some validations and store the custom GUC.
On Tue, Dec 31, 2013 at 9:38 AM, Pavel Stehule <pavel.stehule@gmail.com> wrote:
>
>
> 2013/12/31 Fabrízio de Royes Mello <fabriziomello@gmail.com>
>>
>>
>> On Tue, Dec 31, 2013 at 9:08 AM, Pavel Stehule <pavel.stehule@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.
>
Do you think we don't need to use the "ext" namespace?
yes - there be same mechanism as we use for GUC
Pavel
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
<div dir="ltr"><div class="gmail_extra"><br />On Tue, Dec 31, 2013 at 10:37 AM, Pavel Stehule <<a href="mailto:pavel.stehule@gmail.com">pavel.stehule@gmail.com</a>>wrote:<br />><br />> 2013/12/31 Fabrízio de RoyesMello <<a href="mailto:fabriziomello@gmail.com">fabriziomello@gmail.com</a>><br /> >><br />>> On Tue,Dec 31, 2013 at 9:38 AM, Pavel Stehule <<a href="mailto:pavel.stehule@gmail.com">pavel.stehule@gmail.com</a>> wrote:<br/>>> ><br />>> > 2013/12/31 Fabrízio de Royes Mello <<a href="mailto:fabriziomello@gmail.com">fabriziomello@gmail.com</a>><br/> >> >><br />>> >> On Tue,Dec 31, 2013 at 9:08 AM, Pavel Stehule <<a href="mailto:pavel.stehule@gmail.com">pavel.stehule@gmail.com</a>> wrote:<br/>>> >> ><br />>> >> > Hello<br /> >> >> ><br />>> >> >I am looking on this patch<br />>> >> ><br />>> >> > ALTER TABLE foo SET (ext.somext.do_replicate=true);<br/>>> >> ><br />>> >> > Why is there fixed prefix "ext" ?<br/> >> >> ><br />>> >> > This feature is similar to attaching setting to function<br />>>>> ><br />>> >> > CREATE OR REPLACE FUNCTION ... SET var = ...;<br />>> >>><br /> >> >> > We can use someprefix.someguc without problems there.<br />>> >> ><br/>>> >><br />>> >> Hi,<br />>> >><br />>> >> We use the prefix "ext"(aka namespace) to distinguish these options which are related to "extensions".<br /> >> >><br />>>>> Have you seen the previous thread [1] ?<br />>> ><br />>> ><br />>> > 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.<br /> >> ><br />>><br />>> We use thenamespace "ext" to the internal code (src/backend/access/common/reloptions.c) skip some validations and store the customGUC.<br />>><br />>> Do you think we don't need to use the "ext" namespace?<br /> ><br />><br />>yes - there be same mechanism as we use for GUC<br />><br /><br />If we going to that way then we can expand theuse of this patch to store custom GUCs to functions also, and we can wrote a function (like current_setting) to get specificGUC values, like:<br /><br /></div><div class="gmail_extra">ALTER TABLE foo SET (myextension.option=on);<br /></div><divclass="gmail_extra"><br />SELECT current_setting('foo'::regclass, 'myextension.option');<br /></div><div class="gmail_extra"><br/></div><div class="gmail_extra">Comments?<br /></div><div class="gmail_extra"><br />--<br />Fabríziode Royes Mello<br />Consultoria/Coaching PostgreSQL<br />>> Timbira: <a href="http://www.timbira.com.br">http://www.timbira.com.br</a><br/> >> Blog sobre TI: <a href="http://fabriziomello.blogspot.com">http://fabriziomello.blogspot.com</a><br/>>> Perfil Linkedin: <a href="http://br.linkedin.com/in/fabriziomello">http://br.linkedin.com/in/fabriziomello</a><br/> >> Twitter: <a href="http://twitter.com/fabriziomello">http://twitter.com/fabriziomello</a></div></div>
2013/12/31 Fabrízio de Royes Mello <fabriziomello@gmail.com>
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:
On Tue, Dec 31, 2013 at 10:37 AM, Pavel Stehule <pavel.stehule@gmail.com> wrote:
>
> 2013/12/31 Fabrízio de Royes Mello <fabriziomello@gmail.com>
>>
>> On Tue, Dec 31, 2013 at 9:38 AM, Pavel Stehule <pavel.stehule@gmail.com> wrote:
>> >
>> > 2013/12/31 Fabrízio de Royes Mello <fabriziomello@gmail.com>
>> >>
>> >> On Tue, Dec 31, 2013 at 9:08 AM, Pavel Stehule <pavel.stehule@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
>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
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
<div dir="ltr"><div class="gmail_extra"><br />On Thu, Jan 2, 2014 at 7:19 AM, Andres Freund <<a href="mailto:andres@2ndquadrant.com">andres@2ndquadrant.com</a>>wrote:<br />><br />> On 2013-12-31 13:37:59 +0100,Pavel Stehule wrote:<br /> > > > We use the namespace "ext" to the internal code<br />> > > (src/backend/access/common/reloptions.c)skip some validations and store<br />> > > the custom GUC.<br />> >><br />> > > Do you think we don't need to use the "ext" namespace?<br /> > > ><br />> ><br/>> > yes - there be same mechanism as we use for GUC<br />><br />> There is no existing mechanism tohandle conflicts for GUCs. The<br />> difference is that for GUCs nearly no "namespaced" GUCs exist (plperl,<br /> >plpgsql have some), but postgres defines at least autovacuum. and<br />> toast. namespaces for relation options.<br/>><br /><br /></div><div class="gmail_extra">autovacuum. namespace ???<br /></div><div class="gmail_extra"><br/></div><div class="gmail_extra">The HEAP_RELOPT_NAMESPACES (src/include/access/reloptions.h) constantdefine only "toast" and "null" as a valid relation option namespace. <br /><br />I missed something?<br /></div><divclass="gmail_extra"><br /></div><div class="gmail_extra">Regards,<br /><br /></div><div class="gmail_extra">--<br/>Fabrízio de Royes Mello<br />Consultoria/Coaching PostgreSQL<br />>> Timbira: <a href="http://www.timbira.com.br">http://www.timbira.com.br</a><br/> >> Blog sobre TI: <a href="http://fabriziomello.blogspot.com">http://fabriziomello.blogspot.com</a><br/>>> Perfil Linkedin: <a href="http://br.linkedin.com/in/fabriziomello">http://br.linkedin.com/in/fabriziomello</a><br/> >> Twitter: <a href="http://twitter.com/fabriziomello">http://twitter.com/fabriziomello</a></div></div>
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 <andres@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
On Thu, Jan 2, 2014 at 4:19 AM, Andres Freund <andres@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
Enviado via iPhone > Em 02/01/2014, às 22:16, Robert Haas <robertmhaas@gmail.com> escreveu: > >> On Thu, Jan 2, 2014 at 4:19 AM, Andres Freund <andres@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
On Sat, Jan 4, 2014 at 11:07 AM, Fabrizio Mello <fabriziomello@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
On 2014-01-04 11:54:46 -0500, Robert Haas wrote: > On Sat, Jan 4, 2014 at 11:07 AM, Fabrizio Mello <fabriziomello@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
Andres Freund <andres@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
On 2014-01-04 13:00:03 -0500, Tom Lane wrote: > Andres Freund <andres@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
Andres Freund <andres@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
On 2014-01-04 14:06:19 -0500, Tom Lane wrote: > Andres Freund <andres@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
Andres Freund <andres@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
On Sat, Jan 4, 2014 at 1:00 PM, Tom Lane <tgl@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
Robert Haas <robertmhaas@gmail.com> writes: > On Sat, Jan 4, 2014 at 1:00 PM, Tom Lane <tgl@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
On Sun, Jan 5, 2014 at 3:45 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Robert Haas <robertmhaas@gmail.com> writes: >> On Sat, Jan 4, 2014 at 1:00 PM, Tom Lane <tgl@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
On Mon, Jan 6, 2014 at 1:08 AM, Robert Haas <robertmhaas@gmail.com> wrote:
>
> On Sun, Jan 5, 2014 at 3:45 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> > Robert Haas <robertmhaas@gmail.com> writes:
> >> On Sat, Jan 4, 2014 at 1:00 PM, Tom Lane <tgl@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.
>
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
Robert Haas <robertmhaas@gmail.com> writes: > On Sun, Jan 5, 2014 at 3:45 PM, Tom Lane <tgl@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
Fabrízio de Royes Mello <fabriziomello@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
On Sun, Jan 5, 2014 at 10:50 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Fabrízio de Royes Mello <fabriziomello@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
Robert Haas <robertmhaas@gmail.com> writes: > On Sun, Jan 5, 2014 at 10:50 PM, Tom Lane <tgl@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
On Mon, Jan 6, 2014 at 2:58 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Robert Haas <robertmhaas@gmail.com> writes: >> On Sun, Jan 5, 2014 at 10:50 PM, Tom Lane <tgl@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
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 anotherproblem: 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 codethat 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 thatsays "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 notificationsystem we wrote does. Those are other examples of where we need to store additional information about a tableand had to create a system of our own to handle it. -- Jim C. Nasby, Data Architect jim@nasby.net 512.569.9461 (cell) http://jim.nasby.net
On Mon, Jan 6, 2014 at 1:50 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> Fabrízio de Royes Mello <fabriziomello@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.
>
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
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
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.
On Sat, Jan 11, 2014 at 2:47 AM, Peter Eisentraut <peter_e@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
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
>> Blog sobre TI: http://fabriziomello.blogspot.com
>> Perfil Linkedin: http://br.linkedin.com/in/fabriziomello
>> Twitter: http://twitter.com/fabriziomello
On Sun, Feb 9, 2014 at 2:22 PM, Fabrízio de Royes Mello <fabriziomello@gmail.com> wrote:
>
>
> On Sat, Jan 11, 2014 at 2:47 AM, Peter Eisentraut <peter_e@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
Вложения
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, fabriziomello@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="PARAMETER">name</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
On Fri, Feb 28, 2014 at 5:08 AM, Abhijit Menon-Sen <ams@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, fabriziomello@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="PARAMETER">name</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!>
> 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, fabriziomello@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="PARAMETER">name</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.
>
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
Вложения
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.1390023795@sss.pgh.pa.us -- Álvaro Herrera http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
On Fri, Mar 7, 2014 at 5:56 PM, Alvaro Herrera <alvherre@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.1390023795@sss.pgh.pa.us
This link is broken...
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
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
>> Blog sobre TI: http://fabriziomello.blogspot.com
>> Perfil Linkedin: http://br.linkedin.com/in/fabriziomello
>> Twitter: http://twitter.com/fabriziomello
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 <alvherre@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.1390023795@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.1390023795@sss.pgh.pa.us Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
On Fri, Mar 7, 2014 at 7:21 PM, Andres Freund <andres@2ndquadrant.com> wrote:
On 2014-03-07 19:14:48 -0300, Fabrízio de Royes Mello wrote:It is a message id, and it seemt o point to an appropriate thread? You
> On Fri, Mar 7, 2014 at 5:56 PM, Alvaro Herrera <alvherre@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.1390023795@sss.pgh.pa.us
> >
> >
> This link is broken...
can relatively easily lookup message ids using urls like
http://www.postgresql.org/message-id/4530.1390023795@sss.pgh.pa.us
Sorry... my fault!! Thanks!
--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
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
>> Blog sobre TI: http://fabriziomello.blogspot.com
>> Perfil Linkedin: http://br.linkedin.com/in/fabriziomello
>> Twitter: http://twitter.com/fabriziomello
On Fri, Mar 7, 2014 at 5:56 PM, Alvaro Herrera <alvherre@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.1390023795@sss.pgh.pa.us
Alvaro,
Do you need some help?
Regards,
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
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
>> Blog sobre TI: http://fabriziomello.blogspot.com
>> Perfil Linkedin: http://br.linkedin.com/in/fabriziomello
>> Twitter: http://twitter.com/fabriziomello
Fabrízio de Royes Mello escribió: > On Fri, Mar 7, 2014 at 5:56 PM, Alvaro Herrera <alvherre@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.1390023795@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 Herrera http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
Вложения
Tom Lane escribió: > Fabrízio de Royes Mello <fabriziomello@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 Herrera http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
On 11 March 2014 17:26, Alvaro Herrera <alvherre@2ndquadrant.com> wrote: > Tom Lane escribió: >> Fabrízio de Royes Mello <fabriziomello@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
<div dir="ltr"><div class="gmail_extra"><br />On Tue, Mar 11, 2014 at 2:43 PM, Simon Riggs <<a href="mailto:simon@2ndquadrant.com">simon@2ndquadrant.com</a>>wrote:<br />><br />> On 11 March 2014 17:26, AlvaroHerrera <<a href="mailto:alvherre@2ndquadrant.com">alvherre@2ndquadrant.com</a>> wrote:<br /> > > Tom Laneescribió:<br />> >> Fabrízio de Royes Mello <<a href="mailto:fabriziomello@gmail.com">fabriziomello@gmail.com</a>>writes:<br />> >> > You are correct. pg_dumpexport reloptions using "WITH" clause of CREATE<br /> > >> > TABLE statement. I.e.:<br />> >><br/>> >> > CREATE TABLE foo (<br />> >> > )<br />> >> > WITH (autovacuum_enabled=false,bdr.do_replicate=false);<br />> >><br /> > >> > So if this statement checksfor 'bdr' extension is loaded then in partial<br />> >> > restore it can be fail.<br />> >><br/>> >> I see absolutely *nothing* wrong with failing that command if bdr is not<br /> > >> installed. For an analogy, if this table includes a column of type bar<br />> >> defined by some extension baz,we are certainly going to fail the<br />> >> CREATE TABLE if baz isn't installed.<br /> > >><br />>>> Now, if bdr is installed but the validation doesn't happen unless bdr<br />> >> is "loaded" in somesense, then that is an implementation deficiency<br />> >> that I think we can insist be rectified before thisfeature is accepted.<br /> > ><br />> > So, I spent some time on this patch the last couple of days to addsome<br />> > validation. But after submitting it, it seems to me that there wasn't<br />> > as much consensuson how to handle validation than at first I thought.<br /> > ><br />> > So, the first and simplest wayto go about this, of course, is just not<br />> > validate anything. This is what Fabrízio's patch does. So thetable<br />> > owner can execute<br />> > ALTER TABLE mary SET (supercalifragilisticexpialidocious.umbrella_flight= 'hell yeah')<br /> > > and that would be it. Whether a modulemakes use of this later or not,<br />> > is not that guy's problem. This is mostly what we do for GUCs, note,so<br />> > it's not exactly groundbreaking.<br /> ><br />> If a module fails to use a parameter that maybe a problem. But<br />> forcing us to validate this using user written code may not improve<br />> the situation.<br/>><br />> What happens if I have two extensions that both use the namespace foo?<br /> > That meanswe would run two validation routines on it, and if they<br />> disagree on the set of options and values then weare hosed.<br />><br />> -1 to *requiring* validation for table-level options for exactly the<br /> > same reasonswe no longer validate custom GUCs.<br />><br /><br />In a previous email [1] I asked about alternatives to drivethe work but unfortunately no one replied. So because we already do that to custom GUCs, and is the simpler way to implementthis feature then I did that.<br /><br />Regards,<br /><br />[1] <a href="http://www.postgresql.org/message-id/CAFcNs+r1ZXtRUZLEcEuJ1sF9Qr6Ciks7he-EsMkZOZnh4nxAUA@mail.gmail.com">http://www.postgresql.org/message-id/CAFcNs+r1ZXtRUZLEcEuJ1sF9Qr6Ciks7he-EsMkZOZnh4nxAUA@mail.gmail.com</a><br /><br/>--<br />Fabrízio de Royes Mello<br />Consultoria/Coaching PostgreSQL<br />>> Timbira: <a href="http://www.timbira.com.br">http://www.timbira.com.br</a><br/>>> Blog sobre TI: <a href="http://fabriziomello.blogspot.com">http://fabriziomello.blogspot.com</a><br/> >> Perfil Linkedin: <a href="http://br.linkedin.com/in/fabriziomello">http://br.linkedin.com/in/fabriziomello</a><br/>>> Twitter: <a href="http://twitter.com/fabriziomello">http://twitter.com/fabriziomello</a></div></div>
Simon Riggs <simon@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
On 11 March 2014 18:33, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Simon Riggs <simon@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
<div dir="ltr"><div class="gmail_extra">On Tue, Mar 11, 2014 at 8:42 PM, Simon Riggs <<a href="mailto:simon@2ndquadrant.com">simon@2ndquadrant.com</a>>wrote:<br />><br />> On 11 March 2014 18:33, Tom Lane<<a href="mailto:tgl@sss.pgh.pa.us">tgl@sss.pgh.pa.us</a>> wrote:<br /> > > Simon Riggs <simon@2ndQuadrant.com>writes:<br />> >> -1 to *requiring* validation for table-level options for exactlythe<br />> >> same reasons we no longer validate custom GUCs.<br />> ><br /> > > Well, that isan interesting analogy, but I'm not sure how much it applies<br />> > here. In the case of a GUC, you can fairlyeasily validate it once the<br />> > module does get loaded (and before the module actually tries to do<br />> > anything with it). I don't see how that's going to work for table<br />> > options. I trust nobody isseriously proposing that on module load, we're<br />> > going to scan the whole of pg_class looking to see if thereare incorrect<br /> > > settings. (Even if we did, what would we do about it? Not try to force a<br />> >pg_class update, for sure. And what if the module is loading into the<br />> > postmaster thanks to a preloadspec?)<br /> ><br />> Thank goodness for that. Strict validation does seem scary.<br />><br />> > Idon't really think partial validation makes sense. We could just remove<br />> > the whole topic, and tell extensionauthors that it's up to them to defend<br /> > > themselves against bizarre values stored for their tableoptions. But I'm<br />> > wondering if there's really so much use-case for a feature like that.<br />><br/>> DBAs are fairly used to the idea that if you put crap data in the<br /> > database then bad things happen.We provide the table, they provide<br />> the data. Validation is possible, but not enforced as essential.<br />>(Except in terms of the datatype - but then we are also validating<br /> > data to specific types here).<br />><br/>> So I think that DBAs will also cope rather well with table-level<br />> options without us nannying them.<br/>><br />> There is nothing more annoying that needing to run scripts in a<br /> > specific sequence tomake them work, or dumps that fail because<br />> certain modules aren't loaded yet (or cannot ever be so). And maybe<br/>> the DBA wants to annotate tables based on a design and then later move<br /> > to implement modules totake advantage of the annotation.<br />><br />> Having an option be set and yet be unvalidated and/or unused is no<br/>> more annoying than having a column in a table that is known incorrect<br /> > and/or not accessed. Searchingfor badly set options needs to be<br />> possible, even easy, but hard validation can cause problems. And ifwe<br />> try and force it, whats to stop people from using a dummy validator<br /> > just to circumvent the strictness?<br/>><br /><br /></div><div class="gmail_extra">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].<br /><br />Am I correct?<br/><br /><br />[1] <a href="http://www.postgresql.org/message-id/20140307205649.GF4759@eldon.alvh.no-ip.org">http://www.postgresql.org/message-id/20140307205649.GF4759@eldon.alvh.no-ip.org</a><br /></div><divclass="gmail_extra"><br />--<br />Fabrízio de Royes Mello<br />Consultoria/Coaching PostgreSQL<br />>>Timbira: <a href="http://www.timbira.com.br">http://www.timbira.com.br</a><br />>> Blog sobre TI: <a href="http://fabriziomello.blogspot.com">http://fabriziomello.blogspot.com</a><br/> >> Perfil Linkedin: <a href="http://br.linkedin.com/in/fabriziomello">http://br.linkedin.com/in/fabriziomello</a><br/>>> Twitter: <a href="http://twitter.com/fabriziomello">http://twitter.com/fabriziomello</a></div></div>
On Mon, Mar 10, 2014 at 9:33 PM, Alvaro Herrera <alvherre@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
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
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 Herrera http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
On 12 March 2014 22:58, Robert Haas <robertmhaas@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
On Wed, Mar 12, 2014 at 9:38 PM, Simon Riggs <simon@2ndquadrant.com> wrote: > On 12 March 2014 22:58, Robert Haas <robertmhaas@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
On 13 March 2014 02:14, Robert Haas <robertmhaas@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
* Tom Lane (tgl@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
On Thu, Mar 13, 2014 at 12:47 AM, Simon Riggs <simon@2ndquadrant.com> wrote: > On 13 March 2014 02:14, Robert Haas <robertmhaas@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
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
On Thu, Mar 13, 2014 at 9:26 AM, Andres Freund <andres@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
On 2014-03-13 10:03:03 -0400, Robert Haas wrote: > On Thu, Mar 13, 2014 at 9:26 AM, Andres Freund <andres@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
On 13 March 2014 13:17, Robert Haas <robertmhaas@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
Andres Freund <andres@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
[ forgot to respond to this part ] Andres Freund <andres@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
On 2014-03-13 10:24:09 -0400, Tom Lane wrote: > Andres Freund <andres@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
On 13 March 2014 13:17, Stephen Frost <sfrost@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
On Thu, Mar 13, 2014 at 10:20 AM, Andres Freund <andres@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
On 13 March 2014 14:03, Robert Haas <robertmhaas@gmail.com> wrote: > On Thu, Mar 13, 2014 at 9:26 AM, Andres Freund <andres@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
On Thu, Mar 13, 2014 at 10:22 AM, Simon Riggs <simon@2ndquadrant.com> wrote: > On 13 March 2014 13:17, Robert Haas <robertmhaas@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
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 Herrera http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
On 2014-03-13 10:26:11 -0400, Tom Lane wrote: > [ forgot to respond to this part ] > > Andres Freund <andres@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
On 13 March 2014 14:36, Simon Riggs <simon@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
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
Andres Freund <andres@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
On Thu, Mar 13, 2014 at 10:27 AM, Andres Freund <andres@2ndquadrant.com> wrote: > On 2014-03-13 10:24:09 -0400, Tom Lane wrote: >> Andres Freund <andres@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
On Thu, Mar 13, 2014 at 10:45 AM, Andres Freund <andres@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
On 2014-03-13 11:11:51 -0400, Tom Lane wrote: > Andres Freund <andres@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.GG8268@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
On Thu, Mar 13, 2014 at 11:11 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Andres Freund <andres@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
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 Herrera http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
On 2014-03-13 11:15:56 -0400, Robert Haas wrote: > On Thu, Mar 13, 2014 at 10:27 AM, Andres Freund <andres@2ndquadrant.com> wrote: > > On 2014-03-13 10:24:09 -0400, Tom Lane wrote: > >> Andres Freund <andres@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
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
On 2014-03-13 11:26:10 -0400, Robert Haas wrote: > On Thu, Mar 13, 2014 at 11:11 AM, Tom Lane <tgl@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
Andres Freund <andres@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
On Thu, Mar 13, 2014 at 12:11 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Andres Freund <andres@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
On Thu, Mar 13, 2014 at 11:42 AM, Andres Freund <andres@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 <tgl@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
On Thu, Mar 13, 2014 at 11:37 AM, Andres Freund <andres@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
On Thu, Mar 13, 2014 at 11:30 AM, Alvaro Herrera <alvherre@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
<div dir="ltr"><div class="gmail_extra"><br />On Thu, Mar 13, 2014 at 5:35 PM, Robert Haas <<a href="mailto:robertmhaas@gmail.com">robertmhaas@gmail.com</a>>wrote:<br />><br />> Well, I'm not sure that's reallyany big deal, but I'm not wedded to<br /> > the label idea. My principal concern is: I'm opposed to allowing<br/>> unvalidated options into the database. I think it should be a<br />> requirement that if the validatorcan't be found and called, then the<br /> > reloption is no good and you just reject it. So, if we go with the<br/>> reloptions route, I'd want to see pg_register_option_namespace go away<br />> in favor of some solution thatpreserves that property. One thing I<br /> > kind of like about the LABEL approach is that it applies to virtually<br/>> every object type, meaning that we might not have to repeat this<br />> discussion when (as seems inevitable)people want to be able to do<br /> > things to schemas or tablespaces or roles. But I don't hold that<br />>position so strongly as to be unwilling to entertain any other<br />> options.<br />><br /><br />During the lastdays 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. PersonallyI don't like that.<br /><br />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 aboutobjects.<br /><br />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 kindcallled 'CUSTOM'.<br /><br />I thought about this because representation (syntax) of SECLABELs and COMMENTs are similar.They describe something about objects, they have local and shared catalog.<br /><br />This is just something thatoccurred to me. Maybe I'm completely wrong. Or not!<br /><br /></div><div class="gmail_extra">Comments?<br /><br /></div><divclass="gmail_extra">--<br />Fabrízio de Royes Mello<br />Consultoria/Coaching PostgreSQL<br />>> Timbira:<a href="http://www.timbira.com.br">http://www.timbira.com.br</a><br /> >> Blog sobre TI: <a href="http://fabriziomello.blogspot.com">http://fabriziomello.blogspot.com</a><br/>>> Perfil Linkedin: <a href="http://br.linkedin.com/in/fabriziomello">http://br.linkedin.com/in/fabriziomello</a><br/> >> Twitter: <a href="http://twitter.com/fabriziomello">http://twitter.com/fabriziomello</a></div></div>