Re: ERROR: "ft1" is of the wrong type.

At Fri, 9 Jul 2021 11:03:56 +0900, Michael Paquier <michael(at)paquier(dot)xyz> wrote in
> On Fri, Jul 09, 2021 at 10:44:13AM +0900, Kyotaro Horiguchi wrote:
> > Sounds reasonable. So the attached are that for PG11-PG14. 11 and 12
> > shares the same patch.
>
> How much do the regression tests published upthread in
> https://postgr.es/m/20210219.173039.609314751334535042.horikyota.ntt@gmail.com
> apply here? Shouldn't we also have some regression tests for the new
> error cases you are adding? I agree that we'd better avoid removing

Mmm. Ok, I distributed the mother regression test into each version.

PG11, 12:

- ATT_TABLE | ATT_MATVIEW | ATT_INDEX | ATT_PARTITIONED_INDEX

Added.

- ATT_TABLE | ATT_PARTITIONED_INDEX

This test doesn't detect the "is of the wrong type" issue.

The item is practically a dead one since the combination is caught
by transformPartitionCmd before visiting ATPrepCmd, which emits a
bit different error message for the test.

"\"%s\" is not a partitioned table or index"

ATPrepCmd emits an error that:

"\"%s\" is not a table or partitioned index"

Hmm.. somewhat funny. Actually ATT_TABLE is a bit off here but
there's no symbol ATT_PARTITIONED_TABLE. Theoretically the symbol
is needed but practically not. I don't think we need to do more
than that at least for these versions. (Or we don't even need to
add this item.)

PG13:

- ATT_TABLE | ATT_MATVIEW | ATT_INDEX | ATT_PARTITIONED_INDEX

Same to PG12.

- ATT_TABLE | ATT_PARTITIONED_INDEX:

This version raches this item in ATPrepCmd because the commit
1281a5c907 moved the parse-transform phase to the ATExec stage,
which is visited after ATPrepCmd.

On the other hand, when the target relation is a regular table, the
error is missed by ATPrepCmd then the control reaches to the
Exec-stage. The error is finally aught by transformPartitionCmd.

Of course this works fine but doesn't seem clean, but it is
apparently a matter of the master branch.

- ATT_TABLE | ATT_MATVIEW | ATT_INDEX | ATT_PARTITIONED_INDEX | ATT_FOREIGN_TABLE
Added and works as expected.

PG14:

- ATT_INDEX

I noticed that this combination has been reverted by the commit
ec48314708.

- ATT_TABLE | ATT_MATVIEW | ATT_INDEX | ATT_PARTITIONED_INDEX
- ATT_TABLE | ATT_PARTITIONED_INDEX:
- ATT_TABLE | ATT_MATVIEW | ATT_INDEX | ATT_PARTITIONED_INDEX | ATT_FOREIGN_TABLE

Same as PG13.

So, PG14 and 13 share the same fix and test.

> error cases you are adding? I agree that we'd better avoid removing
> those entries, one argument in favor of not removing any entries being
> that this could have an impact on forks.

Ok. The attached are the two patchsets for PG14-13 and PG12-11
containing the fix and the regression test.

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center