Re: Confused comment about drop replica identity index

2022-01-02 Thread Michael Paquier
On Thu, Dec 30, 2021 at 06:45:30AM +, houzj.f...@fujitsu.com wrote: > I think forbids DROP INDEX might not completely solve this problem. Because > user could still use other command to delete the index, for example: ALTER > TABLE DROP COLUMN. After dropping the column, the index on it will

RE: Confused comment about drop replica identity index

2021-12-29 Thread houzj.f...@fujitsu.com
On Tues, Dec 21, 2021 8:47 AM Michael Paquier wrote: > On Mon, Dec 20, 2021 at 11:57:32AM -0300, Euler Taveira wrote: > > What do you think about the attached patch? It forbids the DROP INDEX. > > We might add a detail message but I didn't in this patch. > > Yeah. I'd agree about doing

Re: Confused comment about drop replica identity index

2021-12-21 Thread Michael Paquier
On Mon, Dec 20, 2021 at 11:57:32AM -0300, Euler Taveira wrote: > On Mon, Dec 20, 2021, at 8:11 AM, Michael Paquier wrote: >> That's mostly fine. I have made some adjustments as per the >> attached. > > Your patch looks good to me. Thanks. I have done this part for now. -- Michael

RE: Confused comment about drop replica identity index

2021-12-20 Thread wangw.f...@fujitsu.com
On Tue, Dec 20, 2021 at 19:11PM, Michael Paquier wrote: > That's mostly fine. I have made some adjustments as per the attached. Thanks for reviewing. > + The default for non-system tables. Records the old values of the > columns > + of the primary key, if any. The default for

Re: Confused comment about drop replica identity index

2021-12-20 Thread Michael Paquier
On Mon, Dec 20, 2021 at 11:57:32AM -0300, Euler Taveira wrote: > What do you think about the attached patch? It forbids the DROP INDEX. We > might > add a detail message but I didn't in this patch. Yeah. I'd agree about doing something like that on HEAD, and that would help with some of the

Re: Confused comment about drop replica identity index

2021-12-20 Thread Euler Taveira
On Mon, Dec 20, 2021, at 8:11 AM, Michael Paquier wrote: > On Mon, Dec 20, 2021 at 03:46:13AM +, wangw.f...@fujitsu.com wrote: > > Here is a patch to correct wrong comment about > > REPLICA_IDENTITY_INDEX, And improve the pg-doc. > > That's mostly fine. I have made some adjustments as per

Re: Confused comment about drop replica identity index

2021-12-20 Thread Michael Paquier
On Mon, Dec 20, 2021 at 03:46:13AM +, wangw.f...@fujitsu.com wrote: > Here is a patch to correct wrong comment about > REPLICA_IDENTITY_INDEX, And improve the pg-doc. That's mostly fine. I have made some adjustments as per the attached. + The default for non-system tables. Records

RE: Confused comment about drop replica identity index

2021-12-19 Thread wangw.f...@fujitsu.com
On Tue, Dec 16, 2021 at 10:27AM, Michael Paquier wrote: > On Tue, Dec 16, 2021 at 06:40AM, Michael Paquier wrote: > > Would you like to write a patch to address all that? > > OK, I will push it soon. Here is a patch to correct wrong comment about REPLICA_IDENTITY_INDEX, And improve the pg-doc.

Re: Confused comment about drop replica identity index

2021-12-16 Thread Euler Taveira
On Thu, Dec 16, 2021, at 8:55 PM, Michael Paquier wrote: > On Thu, Dec 16, 2021 at 03:08:46PM -0300, Alvaro Herrera wrote: > > Hmm, so if a table has REPLICA IDENTITY INDEX and there is a publication > > with an explicit column list, then we need to forbid the DROP INDEX for > > that index. > >

Re: Confused comment about drop replica identity index

2021-12-16 Thread Michael Paquier
On Thu, Dec 16, 2021 at 03:08:46PM -0300, Alvaro Herrera wrote: > Hmm, so if a table has REPLICA IDENTITY INDEX and there is a publication > with an explicit column list, then we need to forbid the DROP INDEX for > that index. Hmm. I have not followed this thread very closely. > I wonder why

Re: Confused comment about drop replica identity index

2021-12-16 Thread Alvaro Herrera
On 2021-Dec-15, Michael Paquier wrote: > On Tue, Dec 14, 2021 at 07:10:49PM +0530, Ashutosh Bapat wrote: > > This code in RelationGetIndexList() is not according to that comment. > > > >if (replident == REPLICA_IDENTITY_DEFAULT && OidIsValid(pkeyIndex)) > > relation->rd_replidindex =

RE: Confused comment about drop replica identity index

2021-12-15 Thread wangw.f...@fujitsu.com
On Tue, Dec 16, 2021 at 06:40AM, Michael Paquier wrote: > Would you like to write a patch to address all that? OK, I will push it soon. Regards, Wang wei

Re: Confused comment about drop replica identity index

2021-12-15 Thread Michael Paquier
On Wed, Dec 15, 2021 at 09:18:26AM +, wangw.f...@fujitsu.com wrote: > Yeah, if we can add some details to pg-doc and code comments, I think it will > be more friendly to PG users and developers. Would you like to write a patch to address all that? Thanks, -- Michael signature.asc

RE: Confused comment about drop replica identity index

2021-12-15 Thread wangw.f...@fujitsu.com
On Tue, Dec 15, 2021 at 11:25AM, Michael Paquier wrote: > Yeah, the comment is wrong. If the index of a REPLICA_IDENTITY_INDEX is > dropped, I recall that the behavior is the same as REPLICA_IDENTITY_NOTHING. Thank you for your response. I agreed that the comment is wrong. > Not sure about the

Re: Confused comment about drop replica identity index

2021-12-14 Thread Michael Paquier
On Tue, Dec 14, 2021 at 07:10:49PM +0530, Ashutosh Bapat wrote: > This code in RelationGetIndexList() is not according to that comment. > >if (replident == REPLICA_IDENTITY_DEFAULT && OidIsValid(pkeyIndex)) > relation->rd_replidindex = pkeyIndex; > else if (replident ==

Re: Confused comment about drop replica identity index

2021-12-14 Thread Ashutosh Bapat
On Tue, Dec 14, 2021 at 6:08 PM wangw.f...@fujitsu.com wrote: > > Hi hackers, > > When I doing development based by PG, I found the following comment have a > little problem in file src/include/catalog/pg_class.h. > > /* > * an explicitly chosen candidate key's columns are used as replica