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
-
Fix unexpected error messages for various flavors of ALTER TABLE
- 85a8c3a4b60f 11.13 landed
- b242e1d239df 12.8 landed
- 5226243459f1 13.4 landed
- 0c83eb2e0edb 14.0 landed
-
Revert per-index collation version tracking feature.
- ec4831470826 14.0 cited
-
Restructure ALTER TABLE execution to fix assorted bugs.
- 1281a5c907b4 13.0 cited