Re: BUG #17409: Unable to alter data type of clustered column which is referenced by foreign key

On Fri, 18 Feb 2022 at 12:27, Michael Paquier <michael(at)paquier(dot)xyz> wrote:
> On Fri, Feb 18, 2022 at 01:30:46AM +0800, Japin Li wrote:
>> On Fri, 18 Feb 2022 at 01:28, Japin Li <japinli(at)hotmail(dot)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;
>

Thanks for pointing out this.

> We surely should have test cases for all that.
>

Agreed.

> Anyway, I'd need to think more about your suggestion, but is that
> actually the best thing we can do?

Thanks for your review, I'm also not sure this solution is best or not.
It is just one solution for this problem.

> 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.

Sorry, I don't get your mind. In both cases, the RememberIndexForRebuilding()
didn't called, why you say "register twice an index to rebuild within
RememberIndexForRebuilding()".

> 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.

Yeah, I forget the replica identity case.

--
Regrads,
Japin Li.
ChengDu WenWu Information Technology Co.,Ltd.