Re: partitioned tables and contrib/sepgsql

On Mon, Mar 27, 2017 at 11:22 AM, Mike Palmiotto
<mike(dot)palmiotto(at)crunchydata(dot)com> wrote:
> On Mon, Mar 27, 2017 at 10:47 AM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>> On Thu, Mar 9, 2017 at 9:47 AM, Stephen Frost <sfrost(at)snowman(dot)net> wrote:
>>> While going over the contrib modules, I noticed that sepgsql was not
>>> updated for partitioned tables. What that appears to mean is that it's
>>> not possible to define labels on partitioned tables.
>>
>> It works for me:
>>
>> rhaas=# load 'dummy_seclabel';
>> LOAD
>> rhaas=# create table foo (a int, b text) partition by range (a);
>> CREATE TABLE
>> rhaas=# security label on table foo is 'classified';
>> SECURITY LABEL
>>
>> What exactly is the problem you're seeing?
>
> IIRC the initial concern was that contrib/sepgsql was not updated for
> RELKIND_PARTITIONED_TABLE, so the post-create hook would not work
> properly for partitioned tables.
>
> I've looked into it a bit and saw a post-alter hook in
> StoreCatalogInheritance1. It seems like it may just be an issue of
> adding the RELKIND_PARTITIONED_TABLE to sepgsql_relation_post_create.

I thought that kind of thing might be the issue, but it didn't seem to
match Stephen's description. Adding RELKIND_PARTITIONED_TABLE to that
function seems like it would probably be sufficient to make that hook
work, although that would need to be tested, but there are numerous
other references to RELKIND_RELATION in contrib/sepgsql, some of which
probably also need to be updated to consider
RELKIND_PARTITIONED_TABLE.

In my view, this is really an enhancement to sepgsql to handle a new
PostgreSQL feature rather than a defect in the partitioning commit, so
I don't think it falls on Amit Langote (as the author) or me (as the
committer) to fix. There might similarly be updates to sepgsql to do
something with permissions on logical replication's new publication
and subscription objects, but we should similarly regard those as
possible new features, not things that Petr or Peter need to work on.
Note that sepgsql hasn't been updated to work with RLS yet, either,
but we didn't regard that as an open item for RLS, or if we did the
resolution was just to document it. I am not opposed to giving a
little more time to get this straightened out, but if a patch doesn't
show up fairly soon then I think we should just document that sepgsql
doesn't support partitioned tables in v10. sepgsql has a fairly
lengthy list of implementation restrictions already, so one more is
not going to kill anybody -- or if it will then that person should
produce a patch soon.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company