Re: [PATCH] Store Extension Options

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.

Regards

Pavel

On Tue, Dec 31, 2013 at 9:08 AM, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
wrote:
>
> Hello
>
> I am looking on this patch
>
> ALTER TABLE foo SET (ext.somext.do_replicate=true);
>
> Why is there fixed prefix "ext" ?
>
> This feature is similar to attaching setting to function
>
> CREATE OR REPLACE FUNCTION ... SET var = ...;
>
> We can use someprefix.someguc without problems there.
>

Hi,

We use the prefix "ext" (aka namespace) to distinguish these options which
are related to "extensions".

Have you seen the previous thread [1] ?

Regards,

[1]
http://www.postgresql.org/message-id/CAFcNs+rqCq1H5eXW-cvdti6T-xo2STEkhjREx=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

2013/12/31 Fabrízio de Royes Mello <fabriziomello(at)gmail(dot)com>

>
> On Tue, Dec 31, 2013 at 9:08 AM, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
> wrote:
> >
> > Hello
> >
> > I am looking on this patch
> >
> > ALTER TABLE foo SET (ext.somext.do_replicate=true);
> >
> > Why is there fixed prefix "ext" ?
> >
> > This feature is similar to attaching setting to function
> >
> > CREATE OR REPLACE FUNCTION ... SET var = ...;
> >
> > We can use someprefix.someguc without problems there.
> >
>
> Hi,
>
> We use the prefix "ext" (aka namespace) to distinguish these options which
> are related to "extensions".
>
> Have you seen the previous thread [1] ?
>

yes, but I don't understand why it is necessary? I use a analogy with
custom GUC - and there we don't use similar prefix. Only any prefix is
required - and it can contain a dot.

Regards

Pavel

>
> Regards,
>
> [1]
> http://www.postgresql.org/message-id/CAFcNs+rqCq1H5eXW-cvdti6T-xo2STEkhjREx=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
>

On Tue, Dec 31, 2013 at 9:38 AM, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
wrote:
>
>
> 2013/12/31 Fabrízio de Royes Mello <fabriziomello(at)gmail(dot)com>
>>
>>
>> On Tue, Dec 31, 2013 at 9:08 AM, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
wrote:
>> >
>> > Hello
>> >
>> > I am looking on this patch
>> >
>> > ALTER TABLE foo SET (ext.somext.do_replicate=true);
>> >
>> > Why is there fixed prefix "ext" ?
>> >
>> > This feature is similar to attaching setting to function
>> >
>> > CREATE OR REPLACE FUNCTION ... SET var = ...;
>> >
>> > We can use someprefix.someguc without problems there.
>> >
>>
>> Hi,
>>
>> We use the prefix "ext" (aka namespace) to distinguish these options
which are related to "extensions".
>>
>> Have you seen the previous thread [1] ?
>
>
> yes, but I don't understand why it is necessary? I use a analogy with
custom GUC - and there we don't use similar prefix. Only any prefix is
required - and it can contain a dot.
>

We use the namespace "ext" to the internal code
(src/backend/access/common/reloptions.c) skip some validations and store
the custom GUC.

Do you think we don't need to use the "ext" namespace?

Regards,

--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
>> Timbira: http://www.timbira.com.br
>> Blog sobre TI: http://fabriziomello.blogspot.com
>> Perfil Linkedin: http://br.linkedin.com/in/fabriziomello
>> Twitter: http://twitter.com/fabriziomello

2013/12/31 Fabrízio de Royes Mello <fabriziomello(at)gmail(dot)com>

>
> On Tue, Dec 31, 2013 at 9:38 AM, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
> wrote:
> >
> >
> > 2013/12/31 Fabrízio de Royes Mello <fabriziomello(at)gmail(dot)com>
> >>
> >>
> >> On Tue, Dec 31, 2013 at 9:08 AM, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
> wrote:
> >> >
> >> > Hello
> >> >
> >> > I am looking on this patch
> >> >
> >> > ALTER TABLE foo SET (ext.somext.do_replicate=true);
> >> >
> >> > Why is there fixed prefix "ext" ?
> >> >
> >> > This feature is similar to attaching setting to function
> >> >
> >> > CREATE OR REPLACE FUNCTION ... SET var = ...;
> >> >
> >> > We can use someprefix.someguc without problems there.
> >> >
> >>
> >> Hi,
> >>
> >> We use the prefix "ext" (aka namespace) to distinguish these options
> which are related to "extensions".
> >>
> >> Have you seen the previous thread [1] ?
> >
> >
> > yes, but I don't understand why it is necessary? I use a analogy with
> custom GUC - and there we don't use similar prefix. Only any prefix is
> required - and it can contain a dot.
> >
>
> We use the namespace "ext" to the internal code
> (src/backend/access/common/reloptions.c) skip some validations and store
> the custom GUC.
>
> Do you think we don't need to use the "ext" namespace?
>

yes - there be same mechanism as we use for GUC

Pavel

>
>
> Regards,
>
>
> --
> Fabrízio de Royes Mello
> Consultoria/Coaching PostgreSQL
> >> Timbira: http://www.timbira.com.br
> >> Blog sobre TI: http://fabriziomello.blogspot.com
> >> Perfil Linkedin: http://br.linkedin.com/in/fabriziomello
> >> Twitter: http://twitter.com/fabriziomello
>

On Tue, Dec 31, 2013 at 10:37 AM, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
wrote:
>
> 2013/12/31 Fabrízio de Royes Mello <fabriziomello(at)gmail(dot)com>
>>
>> On Tue, Dec 31, 2013 at 9:38 AM, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
wrote:
>> >
>> > 2013/12/31 Fabrízio de Royes Mello <fabriziomello(at)gmail(dot)com>
>> >>
>> >> On Tue, Dec 31, 2013 at 9:08 AM, Pavel Stehule <
pavel(dot)stehule(at)gmail(dot)com> wrote:
>> >> >
>> >> > Hello
>> >> >
>> >> > I am looking on this patch
>> >> >
>> >> > ALTER TABLE foo SET (ext.somext.do_replicate=true);
>> >> >
>> >> > Why is there fixed prefix "ext" ?
>> >> >
>> >> > This feature is similar to attaching setting to function
>> >> >
>> >> > CREATE OR REPLACE FUNCTION ... SET var = ...;
>> >> >
>> >> > We can use someprefix.someguc without problems there.
>> >> >
>> >>
>> >> Hi,
>> >>
>> >> We use the prefix "ext" (aka namespace) to distinguish these options
which are related to "extensions".
>> >>
>> >> Have you seen the previous thread [1] ?
>> >
>> >
>> > yes, but I don't understand why it is necessary? I use a analogy with
custom GUC - and there we don't use similar prefix. Only any prefix is
required - and it can contain a dot.
>> >
>>
>> We use the namespace "ext" to the internal code
(src/backend/access/common/reloptions.c) skip some validations and store
the custom GUC.
>>
>> Do you think we don't need to use the "ext" namespace?
>
>
> yes - there be same mechanism as we use for GUC
>

If we going to that way then we can expand the use of this patch to store
custom GUCs to functions also, and we can wrote a function (like
current_setting) to get specific GUC values, like:

ALTER TABLE foo SET (myextension.option=on);

SELECT current_setting('foo'::regclass, 'myextension.option');

Comments?

--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
>> Timbira: http://www.timbira.com.br
>> Blog sobre TI: http://fabriziomello.blogspot.com
>> Perfil Linkedin: http://br.linkedin.com/in/fabriziomello
>> Twitter: http://twitter.com/fabriziomello

2013/12/31 Fabrízio de Royes Mello <fabriziomello(at)gmail(dot)com>

>
> On Tue, Dec 31, 2013 at 10:37 AM, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
> wrote:
> >
> > 2013/12/31 Fabrízio de Royes Mello <fabriziomello(at)gmail(dot)com>
> >>
> >> On Tue, Dec 31, 2013 at 9:38 AM, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
> wrote:
> >> >
> >> > 2013/12/31 Fabrízio de Royes Mello <fabriziomello(at)gmail(dot)com>
> >> >>
> >> >> On Tue, Dec 31, 2013 at 9:08 AM, Pavel Stehule <
> pavel(dot)stehule(at)gmail(dot)com> wrote:
> >> >> >
> >> >> > Hello
> >> >> >
> >> >> > I am looking on this patch
> >> >> >
> >> >> > ALTER TABLE foo SET (ext.somext.do_replicate=true);
> >> >> >
> >> >> > Why is there fixed prefix "ext" ?
> >> >> >
> >> >> > This feature is similar to attaching setting to function
> >> >> >
> >> >> > CREATE OR REPLACE FUNCTION ... SET var = ...;
> >> >> >
> >> >> > We can use someprefix.someguc without problems there.
> >> >> >
> >> >>
> >> >> Hi,
> >> >>
> >> >> We use the prefix "ext" (aka namespace) to distinguish these options
> which are related to "extensions".
> >> >>
> >> >> Have you seen the previous thread [1] ?
> >> >
> >> >
> >> > yes, but I don't understand why it is necessary? I use a analogy with
> custom GUC - and there we don't use similar prefix. Only any prefix is
> required - and it can contain a dot.
> >> >
> >>
> >> We use the namespace "ext" to the internal code
> (src/backend/access/common/reloptions.c) skip some validations and store
> the custom GUC.
> >>
> >> Do you think we don't need to use the "ext" namespace?
> >
> >
> > yes - there be same mechanism as we use for GUC
> >
>
> If we going to that way then we can expand the use of this patch to store
> custom GUCs to functions also, and we can wrote a function (like
> current_setting) to get specific GUC values, like:
>
> ALTER TABLE foo SET (myextension.option=on);
>
> SELECT current_setting('foo'::regclass, 'myextension.option');
>

I like it

Pavel

>
> Comments?
>
> --
> Fabrízio de Royes Mello
> Consultoria/Coaching PostgreSQL
> >> Timbira: http://www.timbira.com.br
> >> Blog sobre TI: http://fabriziomello.blogspot.com
> >> Perfil Linkedin: http://br.linkedin.com/in/fabriziomello
> >> Twitter: http://twitter.com/fabriziomello
>

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

On Thu, Jan 2, 2014 at 7:19 AM, Andres Freund <andres(at)2ndquadrant(dot)com>
wrote:
>
> On 2013-12-31 13:37:59 +0100, Pavel Stehule wrote:
> > > We use the namespace "ext" to the internal code
> > > (src/backend/access/common/reloptions.c) skip some validations and
store
> > > the custom GUC.
> > >
> > > Do you think we don't need to use the "ext" namespace?
> > >
> >
> > yes - there be same mechanism as we use for GUC
>
> There is no existing mechanism to handle conflicts for GUCs. The
> difference is that for GUCs nearly no "namespaced" GUCs exist (plperl,
> plpgsql have some), but postgres defines at least autovacuum. and
> toast. namespaces for relation options.
>

autovacuum. namespace ???

The HEAP_RELOPT_NAMESPACES (src/include/access/reloptions.h) constant
define only "toast" and "null" as a valid relation option namespace.

I missed something?

Regards,

--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
>> Timbira: http://www.timbira.com.br
>> Blog sobre TI: http://fabriziomello.blogspot.com
>> Perfil Linkedin: http://br.linkedin.com/in/fabriziomello
>> Twitter: http://twitter.com/fabriziomello

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(at)2ndquadrant(dot)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(at)2ndquadrant(dot)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(at)gmail(dot)com> escreveu:
>
>> On Thu, Jan 2, 2014 at 4:19 AM, Andres Freund <andres(at)2ndquadrant(dot)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(at)gmail(dot)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(at)gmail(dot)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(at)2ndquadrant(dot)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(at)2ndquadrant(dot)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(at)2ndquadrant(dot)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(at)2ndquadrant(dot)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(at)2ndquadrant(dot)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(at)sss(dot)pgh(dot)pa(dot)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(at)gmail(dot)com> writes:
> On Sat, Jan 4, 2014 at 1:00 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)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(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Robert Haas <robertmhaas(at)gmail(dot)com> writes:
>> On Sat, Jan 4, 2014 at 1:00 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)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(at)gmail(dot)com> wrote:
>
> On Sun, Jan 5, 2014 at 3:45 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> > Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> >> On Sat, Jan 4, 2014 at 1:00 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> >>> I would suggest addressing Robert's concern about lack of error
checking
> >>> by refusing to allow a custom reloption to be set unless the relevant
> >>> extension is loaded and checks it. Unlike the postgresql.conf
problem,
> >>> I don't see any very good use-case for allowing an unchecked ALTER
TABLE
> >>> to occur.
> >
> >> How do you plan to resolve the associated dump/restore hazard?
> >
> > pg_dump creates extensions before tables, no? So what dump/restore
> > hazard?
>
> Creating the extension doesn't guarantee that the shared library will
> always be loaded. If nothing else, think about partial restores.
>

You are correct. pg_dump export reloptions using "WITH" clause of CREATE
TABLE statement. I.e.:

CREATE TABLE foo (
)
WITH (autovacuum_enabled=false, bdr.do_replicate=false);

So if this statement checks for 'bdr' extension is loaded then in partial
restore it can be fail. At this point we have two choices:

1) do not check if extension already is loaded

2) hack the pg_dump to produce an "ALTER TABLE ... SET (...)" instead of
"CREATE TABLE ... WITH (...)" to set reloptions

Comments?

--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
>> Timbira: http://www.timbira.com.br
>> Blog sobre TI: http://fabriziomello.blogspot.com
>> Perfil Linkedin: http://br.linkedin.com/in/fabriziomello
>> Twitter: http://twitter.com/fabriziomello

Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> On Sun, Jan 5, 2014 at 3:45 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)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

=?ISO-8859-1?Q?Fabr=EDzio_de_Royes_Mello?= <fabriziomello(at)gmail(dot)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(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> =?ISO-8859-1?Q?Fabr=EDzio_de_Royes_Mello?= <fabriziomello(at)gmail(dot)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(at)gmail(dot)com> writes:
> On Sun, Jan 5, 2014 at 10:50 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)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(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Robert Haas <robertmhaas(at)gmail(dot)com> writes:
>> On Sun, Jan 5, 2014 at 10:50 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)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 another problem: what if a table gets renamed?

In order to solve that problem, we created a tables table:

CREATE TABLE tools.tables( id SERIAL PRIMARY KEY, table_schema text, table_name text );

That way if we need to rename a table we update one record in one place instead of a bunch of places (we have other code that makes use of tools.tables). (And no, we can't use OIDs because they're not immutable between dumps).

This is obviously ugly and fragile. It'd be much better if we could just define a custom setting on the table itself that says "hey, dump the data from here!". (FWIW, same applies to sequnces).

Actually, I just checked and our seed object stuff doesn't use tools.tables, but our inheritance framework and a change notification system we wrote does. Those are other examples of where we need to store additional information about a table and had to create a system of our own to handle it.
--
Jim C. Nasby, Data Architect jim(at)nasby(dot)net
512.569.9461 (cell) http://jim.nasby.net

On Mon, Jan 6, 2014 at 1:50 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>
> =?ISO-8859-1?Q?Fabr=EDzio_de_Royes_Mello?= <fabriziomello(at)gmail(dot)com>
writes:
> > You are correct. pg_dump export reloptions using "WITH" clause of CREATE
> > TABLE statement. I.e.:
>
> > CREATE TABLE foo (
> > )
> > WITH (autovacuum_enabled=false, bdr.do_replicate=false);
>
> > So if this statement checks for 'bdr' extension is loaded then in
partial
> > restore it can be fail.
>
> I see absolutely *nothing* wrong with failing that command if bdr is not
> installed. For an analogy, if this table includes a column of type bar
> defined by some extension baz, we are certainly going to fail the
> CREATE TABLE if baz isn't installed.
>

Ok.

> Now, if bdr is installed but the validation doesn't happen unless bdr
> is "loaded" in some sense, then that is an implementation deficiency
> that I think we can insist be rectified before this feature is accepted.
>

Check if extension is already installed is not enough for the first version
of this feature?

Regards,

--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
>> Timbira: http://www.timbira.com.br
>> Blog sobre TI: http://fabriziomello.blogspot.com
>> Perfil Linkedin: http://br.linkedin.com/in/fabriziomello
>> Twitter: http://twitter.com/fabriziomello

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(at)gmx(dot)net> wrote:
>
> On Sat, 2014-01-11 at 00:48 -0200, Fabrízio de Royes Mello wrote:
> > > Now, if bdr is installed but the validation doesn't happen unless
> > bdr
> > > is "loaded" in some sense, then that is an implementation deficiency
> > > that I think we can insist be rectified before this feature is
> > accepted.
> > >
>
> > Check if extension is already installed is not enough for the first
> > version of this feature?
>
> Elsewhere it was argued that tying this to extensions is not
> appropriate. I agree.
>
> It depends on how this feature is supposed to be used exactly. A
> replication plugin might very well be loaded via
> session_preload_libraries and not appear in SQL at all. In that case
> you need some C-level hook. In another case, an extension might want to
> inspect relation options from user-space triggers. So you'd need to
> register some SQL-level function for option validation.
>
> This could end up being two separate but overlapping features.
>

Hi all,

I taken this weekend to work on this patch and on monday or tuesday I'll
send it.

But I have some doubts:

1) I'm not convinced to tying this to extensions. I think this feature must
enable us to just store a custom GUC. We can set custom GUCs in a backend
session using "SET class.variable = value", and this feature could just
enable us to store it for relations/attributes. Without the complexity and
overhead to register a function to validate them. That way we can use this
feature to extensions and other needs too.

2) If we're implement the Robert's idea to have a function to validate the
extension options then we must think about how a extension developer will
register this function. Beacuse when we install a extension must have one
way to get de pg_proc OID and store it in the pg_extension (or a different
catalog). Or we'll implement some way to register this function at the SQL
level, like "ALTER EXTENSION bdr SET VALIDATE FUNCTION
bdr_options_validate();" or another sintax of course.

I don't know if you guys understood my concerns!! :-)

Comments?

Regards,

--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
>> Timbira: http://www.timbira.com.br
>> Blog sobre TI: http://fabriziomello.blogspot.com
>> Perfil Linkedin: http://br.linkedin.com/in/fabriziomello
>> Twitter: http://twitter.com/fabriziomello

On Sun, Feb 9, 2014 at 2:22 PM, Fabrízio de Royes Mello <
fabriziomello(at)gmail(dot)com> wrote:
>
>
> On Sat, Jan 11, 2014 at 2:47 AM, Peter Eisentraut <peter_e(at)gmx(dot)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(at)gmail(dot)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(at)2ndquadrant(dot)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(at)gmail(dot)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!

Fixed.

> > @@ -202,6 +210,17 @@ ALTER INDEX distributors SET (fillfactor = 75);
> > REINDEX INDEX distributors;
> > </programlisting></para>
> >
> > + <para>
> > + To set a custom storage parameter:
> > +<programlisting>
> > +ALTER INDEX distributors
> > + SET (bdr.do_replicate=true);
> > +</programlisting>
> > + (bdr=custom name, do_replicate=option name and
> > + true=option value)
> > +</para>
> > +
> > +
> > </refsect1>
>
> It might be best to avoid using bdr.do_replicate in the example, since
> it's still a moving target. It might be better to use a generic example
> like somenamespace.optionname=true, in which case the explanation isn't
> needed either.
>

Fixed.

> The patch applies and builds fine, the tests pass, and the code looks
> OK to me. I don't have a strong opinion on validating custom reloption
> values through hooks as discussed earlier in the thread, but the simple
> version (i.e. your latest patch) seems at least a useful starting point.
>

Thanks for your review.

Regards,

--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
>> Timbira: http://www.timbira.com.br
>> Blog sobre TI: http://fabriziomello.blogspot.com
>> Perfil Linkedin: http://br.linkedin.com/in/fabriziomello
>> Twitter: http://twitter.com/fabriziomello

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(dot)1390023795(at)sss(dot)pgh(dot)pa(dot)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(at)2ndquadrant(dot)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(dot)1390023795(at)sss(dot)pgh(dot)pa(dot)us
>
>
This link is broken...

--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
>> Timbira: http://www.timbira.com.br
>> Blog sobre TI: http://fabriziomello.blogspot.com
>> Perfil Linkedin: http://br.linkedin.com/in/fabriziomello
>> Twitter: http://twitter.com/fabriziomello

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(at)2ndquadrant(dot)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(dot)1390023795(at)sss(dot)pgh(dot)pa(dot)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(at)2ndquadrant(dot)com>wrote:

> 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(at)2ndquadrant(dot)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(dot)1390023795(at)sss(dot)pgh(dot)pa(dot)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
>
>
Sorry... my fault!! Thanks!

--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
>> Timbira: http://www.timbira.com.br
>> Blog sobre TI: http://fabriziomello.blogspot.com
>> Perfil Linkedin: http://br.linkedin.com/in/fabriziomello
>> Twitter: http://twitter.com/fabriziomello

On Fri, Mar 7, 2014 at 5:56 PM, Alvaro Herrera <alvherre(at)2ndquadrant(dot)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(dot)1390023795(at)sss(dot)pgh(dot)pa(dot)us
>
>
Alvaro,

Do you need some help?

Regards,

--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
>> Timbira: http://www.timbira.com.br
>> Blog sobre TI: http://fabriziomello.blogspot.com
>> Perfil Linkedin: http://br.linkedin.com/in/fabriziomello
>> Twitter: http://twitter.com/fabriziomello

Fabrízio de Royes Mello escribió:
> On Fri, Mar 7, 2014 at 5:56 PM, Alvaro Herrera <alvherre(at)2ndquadrant(dot)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(dot)1390023795(at)sss(dot)pgh(dot)pa(dot)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ó:
> =?ISO-8859-1?Q?Fabr=EDzio_de_Royes_Mello?= <fabriziomello(at)gmail(dot)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(at)2ndquadrant(dot)com> wrote:
> Tom Lane escribió:
>> =?ISO-8859-1?Q?Fabr=EDzio_de_Royes_Mello?= <fabriziomello(at)gmail(dot)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

On Tue, Mar 11, 2014 at 2:43 PM, Simon Riggs <simon(at)2ndquadrant(dot)com> wrote:
>
> On 11 March 2014 17:26, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com> wrote:
> > Tom Lane escribió:
> >> =?ISO-8859-1?Q?Fabr=EDzio_de_Royes_Mello?= <fabriziomello(at)gmail(dot)com>
writes:
> >> > You are correct. pg_dump export reloptions using "WITH" clause of
CREATE
> >> > TABLE statement. I.e.:
> >>
> >> > CREATE TABLE foo (
> >> > )
> >> > WITH (autovacuum_enabled=false, bdr.do_replicate=false);
> >>
> >> > So if this statement checks for 'bdr' extension is loaded then in
partial
> >> > restore it can be fail.
> >>
> >> I see absolutely *nothing* wrong with failing that command if bdr is
not
> >> installed. For an analogy, if this table includes a column of type bar
> >> defined by some extension baz, we are certainly going to fail the
> >> CREATE TABLE if baz isn't installed.
> >>
> >> Now, if bdr is installed but the validation doesn't happen unless bdr
> >> is "loaded" in some sense, then that is an implementation deficiency
> >> that I think we can insist be rectified before this feature is
accepted.
> >
> > So, I spent some time on this patch the last couple of days to add some
> > validation. But after submitting it, it seems to me that there wasn't
> > as much consensus on how to handle validation than at first I thought.
> >
> > So, the first and simplest way to go about this, of course, is just not
> > validate anything. This is what Fabrízio's patch does. So the table
> > owner can execute
> > ALTER TABLE mary SET
(supercalifragilisticexpialidocious.umbrella_flight = 'hell yeah')
> > and that would be it. Whether a module makes use of this later or not,
> > is not that guy's problem. This is mostly what we do for GUCs, note, so
> > it's not exactly groundbreaking.
>
> If a module fails to use a parameter that may be a problem. But
> forcing us to validate this using user written code may not improve
> the situation.
>
> What happens if I have two extensions that both use the namespace foo?
> That means we would run two validation routines on it, and if they
> disagree on the set of options and values then we are hosed.
>
> -1 to *requiring* validation for table-level options for exactly the
> same reasons we no longer validate custom GUCs.
>

In a previous email [1] I asked about alternatives to drive the work but
unfortunately no one replied. So because we already do that to custom GUCs,
and is the simpler way to implement this feature then I did that.

Regards,

[1]
http://www.postgresql.org/message-id/CAFcNs+r1ZXtRUZLEcEuJ1sF9Qr6Ciks7he-EsMkZOZnh4nxAUA@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

Simon Riggs <simon(at)2ndQuadrant(dot)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(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Simon Riggs <simon(at)2ndQuadrant(dot)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

On Tue, Mar 11, 2014 at 8:42 PM, Simon Riggs <simon(at)2ndquadrant(dot)com> wrote:
>
> On 11 March 2014 18:33, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> > Simon Riggs <simon(at)2ndQuadrant(dot)com> writes:
> >> -1 to *requiring* validation for table-level options for exactly the
> >> same reasons we no longer validate custom GUCs.
> >
> > Well, that is an interesting analogy, but I'm not sure how much it
applies
> > here. In the case of a GUC, you can fairly easily validate it once the
> > module does get loaded (and before the module actually tries to do
> > anything with it). I don't see how that's going to work for table
> > options. I trust nobody is seriously proposing that on module load,
we're
> > going to scan the whole of pg_class looking to see if there are
incorrect
> > settings. (Even if we did, what would we do about it? Not try to
force a
> > pg_class update, for sure. And what if the module is loading into the
> > postmaster thanks to a preload spec?)
>
> Thank goodness for that. Strict validation does seem scary.
>
> > I don't really think partial validation makes sense. We could just
remove
> > the whole topic, and tell extension authors that it's up to them to
defend
> > themselves against bizarre values stored for their table options. But
I'm
> > wondering if there's really so much use-case for a feature like that.
>
> DBAs are fairly used to the idea that if you put crap data in the
> database then bad things happen. We provide the table, they provide
> the data. Validation is possible, but not enforced as essential.
> (Except in terms of the datatype - but then we are also validating
> data to specific types here).
>
> So I think that DBAs will also cope rather well with table-level
> options without us nannying them.
>
> There is nothing more annoying that needing to run scripts in a
> specific sequence to make them work, or dumps that fail because
> certain modules aren't loaded yet (or cannot ever be so). And maybe
> the DBA wants to annotate tables based on a design and then later move
> to implement modules to take advantage of the annotation.
>
> Having an option be set and yet be unvalidated and/or unused is no
> more annoying than having a column in a table that is known incorrect
> and/or not accessed. Searching for badly set options needs to be
> possible, even easy, but hard validation can cause problems. And if we
> try and force it, whats to stop people from using a dummy validator
> just to circumvent the strictness?
>

Then I think my patch is more adherent given these conclusions, except by
the some adjustments suggested by Tom Lane and mentioned by Alvaro Herrera
[1].

Am I correct?

[1]
http://www.postgresql.org/message-id/20140307205649.GF4759@eldon.alvh.no-ip.org

--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
>> Timbira: http://www.timbira.com.br
>> Blog sobre TI: http://fabriziomello.blogspot.com
>> Perfil Linkedin: http://br.linkedin.com/in/fabriziomello
>> Twitter: http://twitter.com/fabriziomello

On Mon, Mar 10, 2014 at 9:33 PM, Alvaro Herrera
<alvherre(at)2ndquadrant(dot)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(at)gmail(dot)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(at)2ndquadrant(dot)com> wrote:
> On 12 March 2014 22:58, Robert Haas <robertmhaas(at)gmail(dot)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(at)gmail(dot)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

On Thu, Mar 13, 2014 at 12:47 AM, Simon Riggs <simon(at)2ndquadrant(dot)com> wrote:
> On 13 March 2014 02:14, Robert Haas <robertmhaas(at)gmail(dot)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

* Tom Lane (tgl(at)sss(dot)pgh(dot)pa(dot)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 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(at)2ndquadrant(dot)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(at)2ndquadrant(dot)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(at)gmail(dot)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(at)2ndquadrant(dot)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(at)2ndquadrant(dot)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(at)2ndquadrant(dot)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(at)snowman(dot)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(at)2ndquadrant(dot)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(at)gmail(dot)com> wrote:
> On Thu, Mar 13, 2014 at 9:26 AM, Andres Freund <andres(at)2ndquadrant(dot)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

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 Thu, Mar 13, 2014 at 10:22 AM, Simon Riggs <simon(at)2ndquadrant(dot)com> wrote:
> On 13 March 2014 13:17, Robert Haas <robertmhaas(at)gmail(dot)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

On 2014-03-13 10:26:11 -0400, Tom Lane wrote:
> [ forgot to respond to this part ]
>
> Andres Freund <andres(at)2ndquadrant(dot)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(at)2ndquadrant(dot)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(at)2ndquadrant(dot)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(at)2ndquadrant(dot)com> wrote:
> On 2014-03-13 10:24:09 -0400, Tom Lane wrote:
>> Andres Freund <andres(at)2ndquadrant(dot)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(at)2ndquadrant(dot)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(at)2ndquadrant(dot)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(dot)GG8268(at)awork2(dot)anarazel(dot)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(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Andres Freund <andres(at)2ndquadrant(dot)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(at)2ndquadrant(dot)com> wrote:
> > On 2014-03-13 10:24:09 -0400, Tom Lane wrote:
> >> Andres Freund <andres(at)2ndquadrant(dot)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(at)sss(dot)pgh(dot)pa(dot)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(at)2ndquadrant(dot)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(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Andres Freund <andres(at)2ndquadrant(dot)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(at)2ndquadrant(dot)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(at)sss(dot)pgh(dot)pa(dot)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(at)2ndquadrant(dot)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(at)2ndquadrant(dot)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

On Thu, Mar 13, 2014 at 5:35 PM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>
> Well, I'm not sure that's really any big deal, but I'm not wedded to
> the label idea. My principal concern is: I'm opposed to allowing
> unvalidated options into the database. I think it should be a
> requirement that if the validator can't be found and called, then the
> reloption is no good and you just reject it. So, if we go with the
> reloptions route, I'd want to see pg_register_option_namespace go away
> in favor of some solution that preserves that property. One thing I
> kind of like about the LABEL approach is that it applies to virtually
> every object type, meaning that we might not have to repeat this
> discussion when (as seems inevitable) people want to be able to do
> things to schemas or tablespaces or roles. But I don't hold that
> position so strongly as to be unwilling to entertain any other
> options.
>

During the last days I thought about this discussion and to use SECLABELs
sounds weird to me. Here in Brazil we call this kind of thing 'gambiarra'.
Because we'll try to use something that born with a very well defined
purpose to another purpose. Personally I don't like that.

If we think more about SECLABELs, in a more abstract way, it is just a
'property' about database objects. And the same is COMMENTs. Both SECLABEL
and COMMENT provide a way to store something about objects.

Maybe we can think about a new object on top of COMMENT and SECLABELs. An
object called 'PROPERTY' or 'OPTION'. And COMMENTs and SECLABELs can be
just a kind of this new object, and we must introduce a new kind callled
'CUSTOM'.

I thought about this because representation (syntax) of SECLABELs and
COMMENTs are similar. They describe something about objects, they have
local and shared catalog.

This is just something that occurred to me. Maybe I'm completely wrong. Or
not!

Comments?

--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
>> Timbira: http://www.timbira.com.br
>> Blog sobre TI: http://fabriziomello.blogspot.com
>> Perfil Linkedin: http://br.linkedin.com/in/fabriziomello
>> Twitter: http://twitter.com/fabriziomello