From: | Önder Kalacı <onderkalaci(at)gmail(dot)com> |
---|---|
To: | Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> |
Cc: | Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, Peter Smith <smithpb2250(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: doc: improve the restriction description of using indexes on REPLICA IDENTITY FULL table. |
Date: | 2023-07-19 08:09:41 |
Message-ID: | CACawEhU=qKjLyk688t6UhxTQUN-S-iOJKiOXV0wdco5dUgT5tQ@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Hi Masahiko, Amit, all
I've updated the patch.
>
I think the flow is much nicer now compared to the HEAD. I really don't
have any
comments regarding the accuracy of the code changes, all looks good to me.
Overall, I cannot see any behavioral changes as you already alluded to.
Maybe few minor notes regarding the comments:
/*
> + * And must reference the remote relation column. This is because if it
> + * doesn't, the sequential scan is favorable over index scan in most
> + * cases..
> + */
I think the reader might have lost the context (or say in the future due to
another refactor etc). So maybe start with:
/* And the leftmost index field must refer to the ...
Also, now in IsIndexUsableForReplicaIdentityFull() some of the conditions
have comments
some don't. Should we comment on the missing ones as well, maybe such as:
/* partial indexes are not support *
> if (indexInfo->ii_Predicate != NIL)
>
and,
> /* all indexes must have at least one attribute */
> Assert(indexInfo->ii_NumIndexAttrs >= 1);
>
>>
>> BTW, IsIndexOnlyExpression() is not necessary but the current code
>> still works fine. So do we need to backpatch it to PG16? I'm thinking
>> we can apply it to only HEAD.
>
> Either way is fine but I think if we backpatch it then the code
> remains consistent and the backpatching would be easier.
>
Yeah, I also have a slight preference for backporting. It could make it
easier to maintain the code
in the future in case of another backport(s). With the cost of making it
slightly harder for you now :)
Thanks,
Onder
From | Date | Subject | |
---|---|---|---|
Next Message | Michael Paquier | 2023-07-19 08:11:27 | Re: pg_recvlogical prints bogus error when interrupted |
Previous Message | Bharath Rupireddy | 2023-07-19 08:03:15 | Re: pg_recvlogical prints bogus error when interrupted |