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

Kyotaro Horiguchi <horikyota.ntt@gmail.com>

From: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
To: michael@paquier.xyz
Cc: peter.eisentraut@enterprisedb.com, ahsan.hadi@gmail.com, pgsql-hackers@lists.postgresql.org
Date: 2021-07-09T12:00:31Z
Lists: pgsql-hackers

Attachments

At Fri, 9 Jul 2021 11:03:56 +0900, Michael Paquier <michael@paquier.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

Commits

  1. Fix unexpected error messages for various flavors of ALTER TABLE

  2. Revert per-index collation version tracking feature.

  3. Restructure ALTER TABLE execution to fix assorted bugs.