Subject: Re: row filtering for logical replication
To: Stephen Frost <sfrost@snowman.net>
Cc: Euler Taveira <euler@timbira.com.br>, Craig Ringer
 <craig@2ndquadrant.com>, pgsql-hackers <pgsql-hackers@postgresql.org>
References: <20181214153836.GU3415@tamriel.snowman.net>
 <74e4d796-20a9-21fd-0aba-6c0be6d10226@2ndquadrant.com>
 <20181227190511.GI2528@tamriel.snowman.net>
From: Petr Jelinek <petr.jelinek@2ndquadrant.com>
Openpgp: preference=signencrypt
Autocrypt: addr=petr.jelinek@2ndquadrant.com; prefer-encrypt=mutual; keydata=
 mQINBFawwSABEAC45Vua8rSggpM2kkMMmoOHz51a+fi3OXFfnrStVh80pM2/J9nSzjrIKQKf
 H5PtRnoYzSKnoUDoLOkLwDYitK3HzUUWXma+AWAqef2Y0GSZ7A3jJkD+JHOfkpS1CIpkEB/e
 WjA8Wqegwzc/OEE5111/l0Q1e6jxgZI86h8MMnm35Of1Y6Lz4kqWCyxHhaTJKTCkq66ICQ2C
 VJkbvuUoEE+Hr/I38dPHTAqIlCnycVnJPLHBQ/UYGHR/USF8qI/qJJx6tJcNAEqz69QZ/VlY
 31UyJHdYe5NtBVz+rYZCOVWfpe3crbHuq9FjAPT+xcZefdxridmMKuW8eBmmXI9iZ3EG+X8J
 IsdLWL8QoqW0lnO1otYz1QDjdOvbEpq+u5/TAU8pgtaYH2J+6/GaQ6hMLyLBkaIAtnqlLCzq
 pxXcxMsFT14P5/rZA7EbBI8RS1ZIWJBlS2oO+dtVuENLJUC0SpptaRyn/uyOIopt7hSmBekL
 SzFnet7P9qofFJY+fQEz84v0G1gvwonHGHiCGgLpbMdPtnfPzda1aiTURvvOD7w/RmX1/ogk
 TqSGO4ejVNO/28QYouxmxzy8jbbIEiLCbHpLR/M/TvYHHPURxcx+MxVl3G+d/4Ihy4abxOdW
 pe9zLIcVMvqiUFQO28/79Gfhb7G4K+L21ORB82QYkzm3M7ebXQARAQABtCNQZXRyIEplbMOt
 bmVrIDxwam1vZG9zQHBqbW9kb3MubmV0PokCQAQTAQoAKgIbAwUJBaOagAULCQgHAwUVCgkI
 CwUWAgMBAAIeAQIXgAUCVrDBqwIZAQAKCRBci2l0giH4y/QJEACfiLHyvxlwMvo3Y88Qa9QI
 i7Hu3ZW+xxojojdRd7do0nBnf2a4YstD9u6ZOLJx4f5Ijb/8LxY10NgLugmfQ7sw2W8ui6wT
 NiAeNHpr4wpT3IV+V53psOMRYPdq2zNTLO4iNxqPFPgNufR/m4FqcGe0Wo7U0wd1p0MErXTg
 EXJDVmjPDkYnzE9ejSCOxF57+o3xs6+eW9Al2m3nqP8JCzpauCr0gqpQzCaw0VZ05aLL0t9E
 B0fdm90+ydVe10Wg+I348005smuSSckBCHZqUfjjWvjwE6v/6dZ3/50ycAbi6cTe7Fo4kOSO
 sXBFXkzYGWYrEFPkzK0zDH65D98gL1+l9uokCR2t/OYVwudbUCV3cS3xRd0eFgHbXuzV+aZc
 jRjqkPfpCA2loKtrX2U739mpVSHnpIfSBf48fLg7O5dVn5ZCrydGkEzgvyynPo7IikQrIA02
 91IoSp4RisKsJpaXHwHu6DH2rDCbYFumAMgXk2osLFX6VMPoSWKVRkr87A0VLafCtwaqr6Hy
 sFE6Lmu7jP4/BDr+9A2MXK+v0yYrXqqsZN6V9856ywlZrWK3MB6jc2bKpKS2YshneKxpitNV
 0ITs2Li0OlAXOEHE0eXwI93POgLoyH+8JiNbem3orw3Mnc+7XpIY86AejwUiYl8T7EO/2tIx
 HDaXLK3BPNOdH7kCDQRWsMEgARAA6Z3NrDLdcFF/P3TD/F7WQtMALEEKn5TXSF/K2VHt21GD
 8tGY1DZhbxjLuQJeheY/2L4F4Y5WCkAoUdBXQs7nIauN+63EL88/qPdH6f3+/HZ+cmPG1E5o
 dBABCkC42eGcI5O8/9TAaFQcSzg86pdAE8v927tnc6aH4W4/Mj7RJNlQiZaz2ngCC9mr5uu+
 CysOTfKCS0llNUkExZOJzZsQU3zDkcg3+xmgtHkCjglJ006KuytHj55HR6TzaKaAW8coHGoS
 OI/BQ9KAzvlj+32/zS8FSISZSTY3spMpraADsYklejCeJOmqQ3nTrCtwlX3hTSggfr4eelKA
 hdZKs1rIimNBtX90jJc0u2A/GmC959aTSrCUcfaPRweKUuexg7jxuhXyg9br2GscD2G6W7EV
 c+3s4dOHgoJFVMBBIqTtZviaNJ63IsZIFTYJzGKKMHOioZ3naoBVav0w63waDzj82nPYMIPj
 TxnClKiecuBQQPzMaG0jTmK0lis6rO0nIXrCwca8e/3Xzvi7sykOdcz9t4ef8HrDCCkG0yDO
 o8qHy1e7EtmJD7kWffIXZMt4TZVxpiOqgWtv+heJAfmqzr8pkNLUubmgvNdnakXJOJT41Lui
 50C5OhpU9mfFCIDOeZXTQLsHA4KYaL8W/qcjv/YDmvpJtnK24y6tByWuTu7r/SMAEQEAAYkC
 JQQYAQoADwUCVrDBIAIbDAUJBaOagAAKCRBci2l0giH4yyWpD/91jd4BMN2CDYqLtU7kaoAa
 aXnRVpf0z+R8rJ4LZJX1UNbHjsdClMQLfdBuZeDsdYar4iMfwScz6fiCiRwaRpr3XT6znaM0
 wba6fjzEqVSHr3WB1jrb/TMiNaNgQJaRPhI5C7VsLuPrv5/L8gFmyjl712q7F3busGFBKrjK
 qTESMu+sHNvVeC3QyBFCe2fudbAJdaaRsUoTbAZYiowVoQHOow8eBoWY+RzQoYGuw1fls5Ck
 85N73pYHXzjfk1s49sDKTYI263h9wbGGA234biHRI3NQN6sSYWRr/u5GOwTm/tzyadRj0pnZ
 S6KyLSEL4qx8jiyniD/XLoftlvR4YWQ0BXveEElx2n3TThLL1cVtOI4qE9BZKm32euBqwLta
 k9Jc659hmZB9E9aWLo8KcLjngwqSZupoHRPZ3yAlgZkpDoTIZpMCGxGEuuxgnosRbNpOJ1fE
 nNXenYYfi87tHRUCRKeHhoKkfGZEgC9zv9Y97j75npqV2tQyh4fNQT1tGdBigtvzI4nHM1/1
 PLJtJBdRHucP76vUH9RlP9EE/Sg5DFQqsfSYPqxLW9r+ZfSEeT0APdz7TQC9MzNzUhyXvA5a
 RQQCRMXKbGaYhKkYEYiYOGqtTxN+PmyvNUBmFEcagBX2TXqEkgnI6zOnN13mB4SihJ2lne2r
 9RNiPYs/hmRxYg==
Message-ID: <70ccb3a2-48d2-c923-71c3-7bd5346a64aa@2ndquadrant.com>
Date: Fri, 28 Dec 2018 00:15:10 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101
 Thunderbird/60.2.1
MIME-Version: 1.0
In-Reply-To: <20181227190511.GI2528@tamriel.snowman.net>
Content-Type: text/plain; charset=windows-1252
Content-Language: en-US
Content-Transfer-Encoding: 8bit
Precedence: bulk

Hi,

On 27/12/2018 20:05, Stephen Frost wrote:
> Greetings,
> 
> * Petr Jelinek (petr.jelinek@2ndquadrant.com) wrote:
>> On 14/12/2018 16:38, Stephen Frost wrote:
>>> * Petr Jelinek (petr.jelinek@2ndquadrant.com) wrote:
>>>> I do see the appeal here, if you consider logical replication to be a
>>>> streaming select it probably applies well.
>>>>
>>>> But given that this is happening inside output plugin which does not
>>>> have full executor setup and has catalog-only snapshot I am not sure how
>>>> feasible it is to try to merge these two things. As per my previous
>>>> email it's possible that we'll have to be stricter about what we allow
>>>> in expressions here.
>>>
>>> I can certainly understand the concern about trying to combine the
>>> implementation of this with that of RLS; perhaps that isn't a good fit
>>> due to the additional constraints put on logical decoding.
>>>
>>> That said, I still think it might make sense to consider these filters
>>> for logical decoding to be policies and, ideally, to allow users to use
>>> the same policy for both.
>>
>> I am not against that as long as it's possible to have policy for
>> logical replication without having it for RLS and vice versa.
> 
> RLS already is able to be enabled/disabled on a per-table basis.  I
> could see how we might want to extend the existing policy system to have
> a way to enable/disable individual policies for RLS but that should be
> reasonably straight-forward to do, I would think.

Sure, I was mostly referring to having ability of enable/disable this
independently of enabling/disabling RLS which you are okay with based on
bellow so no issue there from my side.

> 
>> I also wonder if policies are flexible enough to allow for specifying
>> OLD and NEW - the replication filtering deals with DML, not with what's
>> visible, it might very well depend on differences between these (that's
>> something the current patch is missing as well BTW).
> 
> The policy system already has the notion of a 'visible' check and a
> 'does the new row match this' check (USING vs. WITH CHECK policies).
> Perhaps if you could outline the specific use-cases that you're thinking
> about, we could discuss them and make sure that they fit within those
> mechanisms- or, if not, discuss if such a use-case would make sense for
> RLS as well and, if so, figure out a way to support that for both.

So we'd use USING for old row images (UPDATE/DELETE) and WITH CHECK for
new ones (UPDATE/INSERT)? I think OLD/NEW is somewhat more natural
naming of this as there is no "SELECT" part of operation here, but as
long as the functionality is there I don't mind syntax that much.

> 
>>> In the end, the idea of having to build a single large and complex
>>> 'create publication' command which has a bunch of tables, each with
>>> their own filter clauses, just strikes me as pretty painful.
>>>
>>>> The other issue with merging this is that the use-case for filtering out
>>>> the data in logical replication is not necessarily about security, but
>>>> often about sending only relevant data. So it makes sense to have filter
>>>> on publication without RLS enabled on table and if we'd force that, we'd
>>>> limit usefulness of this feature.
>>>
>>> I definitely have a serious problem if we are going to say that you
>>> can't use this filtering for security-sensitive cases.
>>
>> I am saying it should not be tied to only security sensitive cases,
>> because it has use cases that have nothing to do with security (ie, I
>> don't want this to depend on RLS being enabled for a table).
> 
> I'm fine with this being able to be independently enabled/disabled,
> apart from RLS.
> 

Cool.

>>>> We definitely want to eventually create subscriptions as non-superuser
>>>> but that has zero effect on this as everything here is happening on
>>>> different server than where subscription lives (we already allow
>>>> creation of publications with just CREATE privilege on database and
>>>> ownership of the table).
>>>
>>> What I wasn't clear about above was the idea that we might allow a user
>>> other than the table owner to publish a given table, but that such a
>>> publication should certanily only be allowed to include the rows which
>>> that user has access to- as regulated by RLS.  If the RLS policy is too
>>> complex to allow that then I would think we'd simply throw an error at
>>> the create publication time and the would-be publisher would need to
>>> figure that out with the table owner.
>>
>> My opinion is that this is useful, but not necessarily something v1
>> patch needs to solve. Having too many publications and subscriptions to
>> various places is not currently practical anyway due to decoding
>> duplicating all the work for every connection.
> 
> I agree that supporting this could be done in a later patch, however, I
> do feel that when we go to add support for non-owners to create
> publications then RLS needs to be supported at that point (and by more
> than just 'throw an error').  I can agree with incremental improvements
> but I don't want to get to a point where we've got a bunch of
> independent things only half of which work with other parts of the
> system.

Yes, using RLS infrastructure now will make it easier to add support for
publishing without being owner at some later point, just let's please
not make publishing without being owner part of requirements for this.


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