pg.ddx.io  pgsql-bugs@postgresql.org mailing list archive  

help / color / mirror / Atom feed
From: Michael Paquier <michael@paquier.xyz>
To: Japin Li <japinli@hotmail.com>
Cc: holly.roberts@starlingbank.com
Cc: peter@eisentraut.org
Cc: pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #17409: Unable to alter data type of clustered column which is referenced by foreign key
Date: Fri, 18 Feb 2022 13:27:06 +0900
Message-ID: <Yg8gGrJebITuPKu8@paquier.xyz> (raw)
In-Reply-To: <ME3P282MB1667F5A392BB4F26E6B0D82AB6369@ME3P282MB1667.AUSP282.PROD.OUTLOOK.COM>
References: <17409-52871dda8b5741cb@postgresql.org>
	<MEYP282MB16691B29F4FEA0CC45CD5FFAB6369@MEYP282MB1669.AUSP282.PROD.OUTLOOK.COM>
	<ME3P282MB1667E2B286DA22A16FD8A8FFB6369@ME3P282MB1667.AUSP282.PROD.OUTLOOK.COM>
	<ME3P282MB1667F5A392BB4F26E6B0D82AB6369@ME3P282MB1667.AUSP282.PROD.OUTLOOK.COM>

On Fri, Feb 18, 2022 at 01:30:46AM +0800, Japin Li wrote:
> On Fri, 18 Feb 2022 at 01:28, Japin Li <japinli@hotmail.com> wrote:
>> The RememberClusterOnForRebuilding() use the tab->clusterOnIndex to check
>> the cluster index exist or not, however, the cluster index can occur more
>> than once, so I think we should check the clustered index by index name.
>> Here is a patch to fix it.  Any suggestions?
> 
> Sorry for forgetting attach the patch.

Reusing the test case at the top of the thread, this can also be
triggered for replica identities, as of the code just at the top of
what you have patched.  See for example:
CREATE TABLE parent (
  parent_field INTEGER CONSTRAINT pk_parent PRIMARY KEY);
ALTER TABLE parent REPLICA IDENTITY USING INDEX pk_parent;
CREATE TABLE child (
  child_field INTEGER,
  CONSTRAINT fk_child FOREIGN KEY (child_field) REFERENCES parent (parent_field));
-- error here
ALTER TABLE parent ALTER COLUMN parent_field SET DATA TYPE BIGINT;

We surely should have test cases for all that.

Anyway, I'd need to think more about your suggestion, but is that
actually the best thing we can do?  In this case, we'd attempt to
register twice an index to rebuild within
RememberIndexForRebuilding(), for the same relation, but there is no
need to do so.  Shouldn't we try instead to do a better job at
scanning the pg_depend entries in ATExecAlterColumnType() and avoid
the risk to do the same job twice?  That would take care of the
replica identity case that I have just mentioned, and of the clustered
index case reported upthread.
--
Michael

Attachments:

  signature.asc (application/pgp-signature, 832B) download

view thread (16+ messages)  latest in thread

Message-ID: <Yg8gGrJebITuPKu8@paquier.xyz>
Permalink:  ../Yg8gGrJebITuPKu8@paquier.xyz/
Also on:    postgresql.org/message-id/Yg8gGrJebITuPKu8@paquier.xyz

 · 

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: pgsql-bugs@postgresql.org
  Cc: michael@paquier.xyz, japinli@hotmail.com, holly.roberts@starlingbank.com, peter@eisentraut.org, pgsql-bugs@lists.postgresql.org
  Subject: Re: BUG #17409: Unable to alter data type of clustered column which is referenced by foreign key
  In-Reply-To: <Yg8gGrJebITuPKu8@paquier.xyz>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox