public inbox for [email protected]
help / color / mirror / Atom feedFrom: Tomas Vondra <[email protected]>
To: Amit Kapila <[email protected]>
Cc: [email protected] <[email protected]>
Cc: Peter Eisentraut <[email protected]>
Cc: Alvaro Herrera <[email protected]>
Cc: Justin Pryzby <[email protected]>
Cc: Rahila Syed <[email protected]>
Cc: Peter Smith <[email protected]>
Cc: pgsql-hackers <[email protected]>
Cc: [email protected] <[email protected]>
Subject: Re: Column Filtering in Logical Replication
Date: Fri, 11 Mar 2022 13:50:29 +0100
Message-ID: <[email protected]> (raw)
In-Reply-To: <CAA4eK1J9UDnM-xN7viu4eO_DDL_PqMG9F6ZsskqEU7snujXi+A@mail.gmail.com>
References: <[email protected]>
<[email protected]>
<[email protected]>
<CAA4eK1KONbrck=hQTFSFgwio5rqu-82KZba2HLE_AcVa_RWYcQ@mail.gmail.com>
<[email protected]>
<CAA4eK1JyoVty_BZOMJdx7VMkBTadvv8oOYrh9xTQ5BY+k-3+OA@mail.gmail.com>
<[email protected]>
<CAA4eK1JJ5qgqbjfYQU9F4R1+OABxec-DQechQXus0fanDjaAVg@mail.gmail.com>
<OS0PR01MB571614E4A39DD33669D5AFD6940A9@OS0PR01MB5716.jpnprd01.prod.outlook.com>
<[email protected]>
<[email protected]>
<[email protected]>
<CAA4eK1J9UDnM-xN7viu4eO_DDL_PqMG9F6ZsskqEU7snujXi+A@mail.gmail.com>
On 3/11/22 10:52, Amit Kapila wrote:
> On Fri, Mar 11, 2022 at 7:26 AM Tomas Vondra
> <[email protected]> wrote:
>>
>> On 3/10/22 20:10, Tomas Vondra wrote:
>>>
>>>
>>> FWIW I think the reason is pretty simple - pgoutput_row_filter_init is
>>> broken. It assumes you can just do this
>>>
>>> rftuple = SearchSysCache2(PUBLICATIONRELMAP,
>>> ObjectIdGetDatum(entry->publish_as_relid),
>>> ObjectIdGetDatum(pub->oid));
>>>
>>> if (HeapTupleIsValid(rftuple))
>>> {
>>> /* Null indicates no filter. */
>>> rfdatum = SysCacheGetAttr(PUBLICATIONRELMAP, rftuple,
>>> Anum_pg_publication_rel_prqual,
>>> &pub_no_filter);
>>> }
>>> else
>>> {
>>> pub_no_filter = true;
>>> }
>>>
>>>
>>> and pub_no_filter=true means there's no filter at all. Which is
>>> nonsense, because we're using publish_as_relid here - the publication
>>> may not include this particular ancestor, in which case we need to just
>>> ignore this publication.
>>>
>>> So yeah, this needs to be reworked.
>>>
>>
>> I spent a bit of time looking at this, and I think a minor change in
>> get_rel_sync_entry() fixes this - it's enough to ensure rel_publications
>> only includes publications that actually include publish_as_relid.
>>
>
> Thanks for looking into this. I think in the first patch before
> calling get_partition_ancestors() we need to ensure it is a partition
> (the call expects that) and pubviaroot is true.
Does the call really require that? Also, I'm not sure why we'd need to
look at pubviaroot - that's already considered earlier when calculating
publish_as_relid, here we just need to know the relationship of the two
OIDs (if one is ancestor/child of the other).
> I think it would be
> good if we can avoid an additional call to get_partition_ancestors()
> as it could be costly.
Maybe. OTOH we only should do this only very rarely anyway.
> I wonder why it is not sufficient to ensure
> that publish_as_relid exists after ancestor in ancestors list before
> assigning the ancestor to publish_as_relid? This only needs to be done
> in case of (if (!publish)). I have not tried this so I could be wrong.
>
I'm not sure what exactly are you proposing. Maybe try coding it? That's
probably faster than trying to describe what the code might do ...
regards
--
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
view thread (20+ messages)
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: Column Filtering in Logical Replication
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox