Re: row filtering for logical replication

Amit Kapila <amit.kapila16@gmail.com>

From: Amit Kapila <amit.kapila16@gmail.com>
To: Tomas Vondra <tomas.vondra@enterprisedb.com>
Cc: Ajin Cherian <itsajin@gmail.com>, Euler Taveira <euler@eulerto.com>, Peter Smith <smithpb2250@gmail.com>, Rahila Syed <rahilasyed90@gmail.com>, Peter Eisentraut <peter.eisentraut@enterprisedb.com>, Önder Kalacı <onderkalaci@gmail.com>, japin <japinli@hotmail.com>, Michael Paquier <michael@paquier.xyz>, David Steele <david@pgmasters.net>, Craig Ringer <craig@2ndquadrant.com>, Tomas Vondra <tomas.vondra@2ndquadrant.com>, Amit Langote <amitlangote09@gmail.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2021-09-27T04:02:40Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →
  1. Release cache tuple when no longer needed

  2. Add some additional tests for row filters in logical replication.

  3. Fix one of the tests introduced in commit 52e4f0cd47.

  4. Allow specifying row filters for logical replication of tables.

  5. Move scanint8() to numutils.c

  6. Replace Test::More plans with done_testing

  7. Reduce relcache access in WAL sender streaming logical changes

  8. Small cleanups related to PUBLICATION framework code

  9. Add a view to show the stats of subscription workers.

  10. Allow publishing the tables of schema.

  11. Doc: improve documentation of CREATE/ALTER SUBSCRIPTION.

  12. Add PublicationTable and PublicationRelInfo structs

  13. Remove unused argument "txn" in maybe_send_schema().

  14. Add prepare API support for streaming transactions in logical replication.

  15. Unify PostgresNode's new() and get_new_node() methods

  16. Use l*_node() family of functions where appropriate

  17. Add support for prepared transactions to built-in logical replication.

  18. Restore the portal-level snapshot after procedure COMMIT/ROLLBACK.

  19. Rename a parse node to be more general

  20. Remove unused column atttypmod from initial tablesync query

  21. SEARCH and CYCLE clauses

On Sat, Sep 25, 2021 at 3:36 PM Tomas Vondra
<tomas.vondra@enterprisedb.com> wrote:
>
> On 9/25/21 6:23 AM, Amit Kapila wrote:
> > On Sat, Sep 25, 2021 at 3:30 AM Tomas Vondra
> > <tomas.vondra@enterprisedb.com> wrote:
> >>
> >> On 9/24/21 7:20 AM, Amit Kapila wrote:
> >>>
> >>> I think the right way to support functions is by the explicit marking
> >>> of functions and in one of the emails above Jeff Davis also agreed
> >>> with the same. I think we should probably introduce a new marking for
> >>> this. I feel this is important because without this it won't be safe
> >>> to access even some of the built-in functions that can access/update
> >>> database (non-immutable functions) due to logical decoding environment
> >>> restrictions.
> >>>
> >>
> >> I agree that seems reasonable. Is there any reason why not to just use
> >> IMMUTABLE for this purpose? Seems like a good match to me.
> >>
> >
> > It will just solve one part of the puzzle (related to database access)
> > but it is better to avoid the risk of broken replication by explicit
> > marking especially for UDFs or other user-defined objects. You seem to
> > be okay documenting such risk but I am not sure we have an agreement
> > on that especially because that was one of the key points of
> > discussions in this thread and various people told that we need to do
> > something about it. I personally feel we should do something if we
> > want to allow user-defined functions or operators because as reported
> > in the thread this problem has been reported multiple times. I think
> > we can go ahead with IMMUTABLE built-ins for the first version and
> > then allow UDFs later or let's try to find a way for explicit marking.
> >
>
> Well, I know multiple people mentioned that issue. And I certainly agree
> just documenting the risk would not be an ideal solution. Requiring the
> functions to be labeled helps, but we've seen people marking volatile
> functions as immutable in order to allow indexing, so we'll have to
> document the risks anyway.
>
> All I'm saying is that allowing built-in functions/operators but not
> user-defined variants seems like an annoying break of extensibility.
> People are used that user-defined stuff can be used just like built-in
> functions and operators.
>

I agree with you that allowing UDFs in some way would be good for this
feature. I think once we get the base feature committed then we can
discuss whether and how to allow UDFs. Do we want to have an
additional label for it or can we come up with something which allows
the user to continue replication even if she has dropped the object
used in the function? It seems like we can limit the scope of base
patch functionality to allow the use of immutable built-in functions
in row filter expressions.

-- 
With Regards,
Amit Kapila.