Re: No toast table for pg_shseclabel but for pg_seclabel

Hi,

postgres=# SELECT oid::regclass, reltoastrelid FROM pg_class WHERE relname IN ('pg_seclabel', 'pg_shseclabel');
oid | reltoastrelid
---------------+---------------
pg_seclabel | 3598
pg_shseclabel | 0
(2 rows)

Isn't that a somewhat odd choice? Why do we assume that there cannot be
lengthy seclabels on shared objects? Granted, most shared objects aren't
candidates for large amounts of data, but both users and databases don't
seem to fall into that category.

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

On 2014-07-04 11:50:17 +0200, Andres Freund wrote:
> Hi,
>
> postgres=# SELECT oid::regclass, reltoastrelid FROM pg_class WHERE relname IN ('pg_seclabel', 'pg_shseclabel');
> oid | reltoastrelid
> ---------------+---------------
> pg_seclabel | 3598
> pg_shseclabel | 0
> (2 rows)
>
> Isn't that a somewhat odd choice? Why do we assume that there cannot be
> lengthy seclabels on shared objects? Granted, most shared objects aren't
> candidates for large amounts of data, but both users and databases don't
> seem to fall into that category.

Hm. It seems they were explicitly removed around
http://archives.postgresql.org/message-id/1309888389-sup-3853%40alvh.no-ip.org

I don't understand the reasoning there. There's a toast table for
non-shared objects. Why would we expect less data for the shared ones,
even though they're pretty basic objects and more likely to be used to
store policies and such?

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

Here is no other reason than what Alvaro mentioned in the upthread.
We intended to store security label of SELinux (less than 100bytes at most),
so I didn't think it leads any problem actually.

On the other hands, pg_seclabel was merged in another development cycle.
We didn't have deep discussion about necessity of toast table of pg_seclabel.
I added its toast table mechanically.

It never means we need toast table for local object but don't need it for shared
database object.

Thanks,

2014-07-04 19:11 GMT+09:00 Andres Freund <andres(at)2ndquadrant(dot)com>:
> On 2014-07-04 11:50:17 +0200, Andres Freund wrote:
>> Hi,
>>
>> postgres=# SELECT oid::regclass, reltoastrelid FROM pg_class WHERE relname IN ('pg_seclabel', 'pg_shseclabel');
>> oid | reltoastrelid
>> ---------------+---------------
>> pg_seclabel | 3598
>> pg_shseclabel | 0
>> (2 rows)
>>
>> Isn't that a somewhat odd choice? Why do we assume that there cannot be
>> lengthy seclabels on shared objects? Granted, most shared objects aren't
>> candidates for large amounts of data, but both users and databases don't
>> seem to fall into that category.
>
> Hm. It seems they were explicitly removed around
> http://archives.postgresql.org/message-id/1309888389-sup-3853%40alvh.no-ip.org
>
> I don't understand the reasoning there. There's a toast table for
> non-shared objects. Why would we expect less data for the shared ones,
> even though they're pretty basic objects and more likely to be used to
> store policies and such?
>
> Greetings,
>
> Andres Freund
>
> --
> Andres Freund http://www.2ndQuadrant.com/
> PostgreSQL Development, 24x7 Support, Training & Services

--
KaiGai Kohei <kaigai(at)kaigai(dot)gr(dot)jp>

Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp> writes:
> Here is no other reason than what Alvaro mentioned in the upthread.
> We intended to store security label of SELinux (less than 100bytes at most),
> so I didn't think it leads any problem actually.

> On the other hands, pg_seclabel was merged in another development cycle.
> We didn't have deep discussion about necessity of toast table of pg_seclabel.
> I added its toast table mechanically.

So maybe we should get rid of the toast table for pg_seclabel. One less
catalog table for a feature that hardly anyone is using seems like a fine
idea to me ...

regards, tom lane

On Fri, Jul 4, 2014 at 10:53:15AM -0400, Tom Lane wrote:
> Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp> writes:
> > Here is no other reason than what Alvaro mentioned in the upthread.
> > We intended to store security label of SELinux (less than 100bytes at most),
> > so I didn't think it leads any problem actually.
>
> > On the other hands, pg_seclabel was merged in another development cycle.
> > We didn't have deep discussion about necessity of toast table of pg_seclabel.
> > I added its toast table mechanically.
>
> So maybe we should get rid of the toast table for pg_seclabel. One less
> catalog table for a feature that hardly anyone is using seems like a fine
> idea to me ...

Is this still an open item?

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ Everyone has their own god. +

Bruce Momjian <bruce(at)momjian(dot)us> writes:
> On Fri, Jul 4, 2014 at 10:53:15AM -0400, Tom Lane wrote:
>> So maybe we should get rid of the toast table for pg_seclabel. One less
>> catalog table for a feature that hardly anyone is using seems like a fine
>> idea to me ...

> Is this still an open item?

I haven't done anything about it ...

regards, tom lane

On Sat, Oct 11, 2014 at 5:40 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>
> Bruce Momjian <bruce(at)momjian(dot)us> writes:
> > On Fri, Jul 4, 2014 at 10:53:15AM -0400, Tom Lane wrote:
> >> So maybe we should get rid of the toast table for pg_seclabel. One
less
> >> catalog table for a feature that hardly anyone is using seems like a
fine
> >> idea to me ...
>
> > Is this still an open item?
>
> I haven't done anything about it ...
>

If the final decision is get rid the toast table for pg_seclabel and as
I've time then I did it.

Patch attached.

Regards,

--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
>> Timbira: http://www.timbira.com.br
>> Blog: http://fabriziomello.github.io
>> Linkedin: http://br.linkedin.com/in/fabriziomello
>> Twitter: http://twitter.com/fabriziomello
>> Github: http://github.com/fabriziomello

On 2014-10-11 18:19:05 -0300, Fabrízio de Royes Mello wrote:
> On Sat, Oct 11, 2014 at 5:40 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> >
> > Bruce Momjian <bruce(at)momjian(dot)us> writes:
> > > On Fri, Jul 4, 2014 at 10:53:15AM -0400, Tom Lane wrote:
> > >> So maybe we should get rid of the toast table for pg_seclabel. One
> less
> > >> catalog table for a feature that hardly anyone is using seems like a
> fine
> > >> idea to me ...
> >
> > > Is this still an open item?
> >
> > I haven't done anything about it ...
> >
>
> If the final decision is get rid the toast table for pg_seclabel and as
> I've time then I did it.

I still think this the wrong direction. I really fail to see why we want
to restrict security policies to some rather small size.

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

* Andres Freund (andres(at)2ndquadrant(dot)com) wrote:
> On 2014-10-11 18:19:05 -0300, Fabrízio de Royes Mello wrote:
> > On Sat, Oct 11, 2014 at 5:40 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> > >
> > > Bruce Momjian <bruce(at)momjian(dot)us> writes:
> > > > On Fri, Jul 4, 2014 at 10:53:15AM -0400, Tom Lane wrote:
> > > >> So maybe we should get rid of the toast table for pg_seclabel. One
> > less
> > > >> catalog table for a feature that hardly anyone is using seems like a
> > fine
> > > >> idea to me ...
> > >
> > > > Is this still an open item?
> > >
> > > I haven't done anything about it ...
> > >
> >
> > If the final decision is get rid the toast table for pg_seclabel and as
> > I've time then I did it.
>
> I still think this the wrong direction. I really fail to see why we want
> to restrict security policies to some rather small size.

I agree with this.

There's no ability to store multiple labels for the same object and
provider with multiple rows (which is fine by itself), and so that means
security providers with multiple overlapping labels for the same object
need to combine them together and store them together. While I agree
that individual labels don't tend to get very long, when you combine
overlapping ones, they could get long enough to need toasting.

Admittedly, you could complicate the system by defining those labels as
new labels, but we are likely working with an external authorization
system and it's a lot less trouble to attach multiple labels to the
given object than to ask everyone else to change because PG ran out of
room in the text column because it can't TOAST it..

Then there's the other discussion about using the security labels
structure for more than just security labels, which could end up with a
lot of other use-cases where the "label" is even larger.

Thanks,

Stephen

On Sat, Oct 11, 2014 at 06:01:58PM -0400, Stephen Frost wrote:
> > I still think this the wrong direction. I really fail to see why we want
> > to restrict security policies to some rather small size.
>
> I agree with this.
>
> There's no ability to store multiple labels for the same object and
> provider with multiple rows (which is fine by itself), and so that means
> security providers with multiple overlapping labels for the same object
> need to combine them together and store them together. While I agree
> that individual labels don't tend to get very long, when you combine
> overlapping ones, they could get long enough to need toasting.
>
> Admittedly, you could complicate the system by defining those labels as
> new labels, but we are likely working with an external authorization
> system and it's a lot less trouble to attach multiple labels to the
> given object than to ask everyone else to change because PG ran out of
> room in the text column because it can't TOAST it..
>
> Then there's the other discussion about using the security labels
> structure for more than just security labels, which could end up with a
> lot of other use-cases where the "label" is even larger.

OK, the attached patch adds a TOAST table to the shared table
pg_shseclabel for use with long labels. The new query output shows the
shared and non-shared seclabel tables now both have TOAST tables:

test=> SELECT oid::regclass, reltoastrelid FROM pg_class WHERE relname IN ('pg_seclabel', 'pg_shseclabel');
oid | reltoastrelid
---------------+---------------
pg_seclabel | 3598
pg_shseclabel | 4060
(2 rows)

Previously pg_shseclabel was zero.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ Everyone has their own god. +

On Thu, Mar 19, 2015 at 11:50:36AM -0400, Bruce Momjian wrote:
> > Then there's the other discussion about using the security labels
> > structure for more than just security labels, which could end up with a
> > lot of other use-cases where the "label" is even larger.
>
> OK, the attached patch adds a TOAST table to the shared table
> pg_shseclabel for use with long labels. The new query output shows the
> shared and non-shared seclabel tables now both have TOAST tables:
>
> test=> SELECT oid::regclass, reltoastrelid FROM pg_class WHERE relname IN ('pg_seclabel', 'pg_shseclabel');
> oid | reltoastrelid
> ---------------+---------------
> pg_seclabel | 3598
> pg_shseclabel | 4060
> (2 rows)
>
> Previously pg_shseclabel was zero.

Patch applied.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ Everyone has their own god. +

On March 22, 2015 3:15:07 AM GMT+01:00, Bruce Momjian <bruce(at)momjian(dot)us> wrote:
>On Thu, Mar 19, 2015 at 11:50:36AM -0400, Bruce Momjian wrote:
>> > Then there's the other discussion about using the security labels
>> > structure for more than just security labels, which could end up
>with a
>> > lot of other use-cases where the "label" is even larger.
>>
>> OK, the attached patch adds a TOAST table to the shared table
>> pg_shseclabel for use with long labels. The new query output shows
>the
>> shared and non-shared seclabel tables now both have TOAST tables:
>>
>> test=> SELECT oid::regclass, reltoastrelid FROM pg_class WHERE
>relname IN ('pg_seclabel', 'pg_shseclabel');
>> oid | reltoastrelid
>> ---------------+---------------
>> pg_seclabel | 3598
>> pg_shseclabel | 4060
>> (2 rows)
>>
>> Previously pg_shseclabel was zero.
>
>Patch applied.

Thanks.

--
Please excuse brevity and formatting - I am writing this on my mobile phone.

Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services