From: | Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> |
---|---|
To: | Önder Kalacı <onderkalaci(at)gmail(dot)com> |
Cc: | vignesh C <vignesh21(at)gmail(dot)com>, "shiy(dot)fnst(at)fujitsu(dot)com" <shiy(dot)fnst(at)fujitsu(dot)com>, "wangw(dot)fnst(at)fujitsu(dot)com" <wangw(dot)fnst(at)fujitsu(dot)com>, Marco Slot <marco(dot)slot(at)gmail(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Andres Freund <andres(at)anarazel(dot)de>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: [PATCH] Use indexes on the subscriber when REPLICA IDENTITY is full on the publisher |
Date: | 2023-03-06 09:40:24 |
Message-ID: | CAA4eK1+Q7+n6DmU0zYP+Mabyr+eOFocCTS8UeYmfs7hBc-5tUw@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Fri, Mar 3, 2023 at 6:40 PM Önder Kalacı <onderkalaci(at)gmail(dot)com> wrote:
>
> Hi Vignesh,
>
> Thanks for the review
>
>>
>> 1) We are currently calling RelationGetIndexList twice, once in
>> FindUsableIndexForReplicaIdentityFull function and in the caller too,
>> we could avoid one of the calls by passing the indexlist to the
>> function or removing the check here, index list check can be handled
>> in FindUsableIndexForReplicaIdentityFull.
>> + if (remoterel->replident == REPLICA_IDENTITY_FULL &&
>> + RelationGetIndexList(localrel) != NIL)
>> + {
>> + /*
>> + * If we had a primary key or relation identity with a
>> unique index,
>> + * we would have already found and returned that oid.
>> At this point,
>> + * the remote relation has replica identity full and
>> we have at least
>> + * one local index defined.
>> + *
>> + * We are looking for one more opportunity for using
>> an index. If
>> + * there are any indexes defined on the local
>> relation, try to pick
>> + * a suitable index.
>> + *
>> + * The index selection safely assumes that all the
>> columns are going
>> + * to be available for the index scan given that
>> remote relation has
>> + * replica identity full.
>> + */
>> + return FindUsableIndexForReplicaIdentityFull(localrel);
>> + }
>> +
>
> makes sense, done
>
Today, I was looking at this comment and the fix for it. It seems to
me that it would be better to not add the check (indexlist != NIL)
here and rather get the indexlist in
FindUsableIndexForReplicaIdentityFull(). It will anyway return
InvalidOid, if there is no index and that way code will look a bit
cleaner.
--
With Regards,
Amit Kapila.
From | Date | Subject | |
---|---|---|---|
Next Message | Katsuragi Yuta | 2023-03-06 10:13:58 | Re: [Proposal] Add foreign-server health checks infrastructure |
Previous Message | Kartyshov Ivan | 2023-03-06 09:40:16 | Re: [HACKERS] make async slave to wait for lsn to be replayed |