Re: partitioned tables and contrib/sepgsql - Mailing list pgsql-hackers

On Mon, Mar 27, 2017 at 11:46 AM, Robert Haas <robertmhaas@gmail.com> wrote:
> On Mon, Mar 27, 2017 at 11:22 AM, Mike Palmiotto
> <mike.palmiotto@crunchydata.com> wrote:
>> On Mon, Mar 27, 2017 at 10:47 AM, Robert Haas <robertmhaas@gmail.com> wrote:
>>> On Thu, Mar 9, 2017 at 9:47 AM, Stephen Frost <sfrost@snowman.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.

Agreed.

>
> 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.

I'll keep these items in mind for future reference. Thanks.

>  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.

Okay, I'll make sure I get something fleshed out today or tomorrow.

-- 
Mike Palmiotto
Software Engineer
Crunchy Data Solutions
https://crunchydata.com