Thread: pgsql: Change pg_seclabel.provider and pg_shseclabel.provider to type "
Change pg_seclabel.provider and pg_shseclabel.provider to type "name". These were "text", but that's a bad idea because it has collation-dependent ordering. No index in template0 should have collation-dependent ordering, especially not indexes on shared catalogs. There was general agreement that provider names don't need to be longer than other identifiers, so we can fix this at a small waste of table space by changing from text to name. There's no way to fix the problem in the back branches, but we can hope that security labels don't yet have widespread-enough usage to make it urgent to fix. There needs to be a regression sanity test to prevent us from making this same mistake again; but before putting that in, we'll need to get rid of similar brain fade in the recently-added pg_replication_origin catalog. Note: for lack of a suitable testing environment, I've not really exercised this change. I trust the buildfarm will show up any mistakes. Branch ------ master Details ------- http://git.postgresql.org/pg/commitdiff/b82a7be603f1811a0a707b53c62de6d5d9431740 Modified Files -------------- doc/src/sgml/catalogs.sgml | 6 +++--- src/backend/commands/seclabel.c | 24 ++++++++++++++---------- src/include/catalog/catversion.h | 2 +- src/include/catalog/indexing.h | 4 ++-- src/include/catalog/pg_seclabel.h | 2 +- src/include/catalog/pg_shseclabel.h | 2 +- 6 files changed, 22 insertions(+), 18 deletions(-)
Re: pgsql: Change pg_seclabel.provider and pg_shseclabel.provider to type "
From
andres@anarazel.de (Andres Freund)
Date:
On 2015-05-19 00:08:05 +0000, Tom Lane wrote: > Note: for lack of a suitable testing environment, I've not really exercised > this change. I trust the buildfarm will show up any mistakes. Shouldn't src/test/modules/dummy_seclabel/ exercise this without further dependencies? Greetings, Andres Freund
andres@anarazel.de (Andres Freund) writes:
> On 2015-05-19 00:08:05 +0000, Tom Lane wrote:
>> Note: for lack of a suitable testing environment, I've not really exercised
>> this change. I trust the buildfarm will show up any mistakes.
> Shouldn't src/test/modules/dummy_seclabel/ exercise this without further
> dependencies?
I have no idea how thoroughly that tests it --- I was assuming we'd need
to see a run from contrib/sepgsql to be sure it's OK.
regards, tom lane
Re: pgsql: Change pg_seclabel.provider and pg_shseclabel.provider to type "
From
Andrew Dunstan
Date:
On 05/18/2015 08:20 PM, Tom Lane wrote: > andres@anarazel.de (Andres Freund) writes: >> On 2015-05-19 00:08:05 +0000, Tom Lane wrote: >>> Note: for lack of a suitable testing environment, I've not really exercised >>> this change. I trust the buildfarm will show up any mistakes. >> Shouldn't src/test/modules/dummy_seclabel/ exercise this without further >> dependencies? > I have no idea how thoroughly that tests it --- I was assuming we'd need > to see a run from contrib/sepgsql to be sure it's OK. > > The buildfarm has no support for contrib/sepgsql cheers andrew
Re: pgsql: Change pg_seclabel.provider and pg_shseclabel.provider to type "
From
Stephen Frost
Date:
* Andrew Dunstan (andrew@dunslane.net) wrote:
> On 05/18/2015 08:20 PM, Tom Lane wrote:
> >andres@anarazel.de (Andres Freund) writes:
> >>On 2015-05-19 00:08:05 +0000, Tom Lane wrote:
> >>>Note: for lack of a suitable testing environment, I've not really exercised
> >>>this change. I trust the buildfarm will show up any mistakes.
> >>Shouldn't src/test/modules/dummy_seclabel/ exercise this without further
> >>dependencies?
> >I have no idea how thoroughly that tests it --- I was assuming we'd need
> >to see a run from contrib/sepgsql to be sure it's OK.
>
> The buildfarm has no support for contrib/sepgsql
Right, that's on my list of things to address, though I'm not
particularly worried about the changes being made.
I expect we'll have some additional changes to sepgsql to discuss for
9.6, once we're ready to discuss such.
Thanks!
Stephen
Attachment
Stephen Frost <sfrost@snowman.net> writes:
> * Andrew Dunstan (andrew@dunslane.net) wrote:
>> The buildfarm has no support for contrib/sepgsql
> Right, that's on my list of things to address, though I'm not
> particularly worried about the changes being made.
The replacement patch seems even less likely to have any issues, anyway.
regards, tom lane