On Thu, Nov 25, 2021 at 12:03 AM Amit Kapila <amit.kapil...@gmail.com> wrote: > > On Tue, Nov 23, 2021 at 4:58 PM Ajin Cherian <itsa...@gmail.com> wrote: > > > > Attaching a new patchset v41 which includes changes by both Peter and > > myself. > > > > Patches v40-0005 and v40-0006 have been merged to create patch > > v41-0005 which reduces the patches to 6 again. > > This patch-set contains changes addressing the following review comments: > > > > On Mon, Nov 15, 2021 at 5:48 PM Amit Kapila <amit.kapil...@gmail.com> wrote: > > > > > > What I meant was that with this new code we have regressed the old > > > behavior. Basically, imagine a case where no filter was given for any > > > of the tables. Then after the patch, we will remove all the old tables > > > whereas before the patch it will remove the oldrels only when they are > > > not specified as part of new rels. If you agree with this, then we can > > > retain the old behavior and for the new tables, we can always override > > > the where clause for a SET variant of command. > > > > Fixed and modified the behaviour to match with what the schema patch > > implemented. > > > > + > + /* > + * If the new relation or the old relation has a where clause, > + * we need to remove it so that it can be added afresh later. > + */ > + if (RelationGetRelid(newpubrel->relation) == oldrelid && > + newpubrel->whereClause == NULL && rfisnull) > > Can't we use _equalPublicationTable() here? It compares the whereClause as > well. >
Fixed in v44 [1] > Few more comments: > ================= > 0001 ... . > 3. In the function header comment of rowfilter_walker, you mentioned > the simple expressions allowed but we should write why we are doing > so. It has been discussed in detail in various emails in this thread. > AFAIR, below are the reasons: > A. We don't want to allow user-defined functions or operators because > (a) if the user drops such a function/operator or if there is any > other error via that function, the walsender won't be able to recover > from such an error even if we fix the function's problem because it > uses a historic snapshot to access row-filter; (b) any other table > could be accessed via a function which won't work because of historic > snapshots in logical decoding environment. > > B. We don't allow anything other immutable built-in functions as those > can access database and would lead to the problem (b) mentioned in the > previous paragraph. > Updated comment in v44 [1] > Don't we need to check for user-defined types similar to user-defined > functions and operators? If not why? Fixed in v44 [1] ------ [1] https://www.postgresql.org/message-id/CAHut%2BPtjxzedJPbSZyb9pd72%2BUrGEj6HagQQbCdO0YJvr7OyJg%40mail.gmail.com Kind Regards, Peter Smith. Fujitsu Australia
