Re: ERROR: "ft1" is of the wrong type. - Mailing list pgsql-hackers
| From | Kyotaro Horiguchi |
|---|---|
| Subject | Re: ERROR: "ft1" is of the wrong type. |
| Date | |
| Msg-id | 20210709.210031.862146579075829807.horikyota.ntt@gmail.com Whole thread Raw |
| List | pgsql-hackers |
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 From 80fa3ae43f5697a562b2ba460a50cdedf8d53e55 Mon Sep 17 00:00:00 2001 From: Kyotaro Horiguchi <horikyota.ntt@gmail.com> Date: Thu, 8 Jul 2021 17:55:25 +0900 Subject: [PATCH v3 1/2] Add missing targets in ATWrongRelkindError ATWrongRelkindError yields an ambiguous message ""tbl" is of the wrong type" due the lack of some combinations of allowed_targets. Add the missing items. We don't remove apparently unused entries in case someone is using them. --- src/backend/commands/tablecmds.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 97a9725df7..d02e01cb20 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -6029,6 +6029,12 @@ ATWrongRelkindError(Relation rel, int allowed_targets) case ATT_TABLE | ATT_MATVIEW | ATT_INDEX: msg = _("\"%s\" is not a table, materialized view, or index"); break; + case ATT_TABLE | ATT_MATVIEW | ATT_INDEX | ATT_PARTITIONED_INDEX: + msg = _("\"%s\" is not a table, materialized view, index, or partitioned index"); + break; + case ATT_TABLE | ATT_MATVIEW | ATT_INDEX | ATT_PARTITIONED_INDEX | ATT_FOREIGN_TABLE: + msg = _("\"%s\" is not a table, materialized view, index, partitioned index, or foreign table"); + break; case ATT_TABLE | ATT_MATVIEW | ATT_FOREIGN_TABLE: msg = _("\"%s\" is not a table, materialized view, or foreign table"); break; @@ -6041,6 +6047,9 @@ ATWrongRelkindError(Relation rel, int allowed_targets) case ATT_TABLE | ATT_MATVIEW | ATT_INDEX | ATT_FOREIGN_TABLE: msg = _("\"%s\" is not a table, materialized view, index, or foreign table"); break; + case ATT_TABLE | ATT_PARTITIONED_INDEX: + msg = _("\"%s\" is not a table or partitioned index"); + break; case ATT_VIEW: msg = _("\"%s\" is not a view"); break; -- 2.27.0 From 3fc4f789006cb6f6239dda7e6f7c817f9da41812 Mon Sep 17 00:00:00 2001 From: Kyotaro Horiguchi <horikyota.ntt@gmail.com> Date: Fri, 9 Jul 2021 20:19:23 +0900 Subject: [PATCH v3 2/2] regress - Add missing targets in ATWrongRelkindError --- src/test/regress/expected/alter_table.out | 7 +++++++ src/test/regress/expected/foreign_data.out | 4 +++- src/test/regress/sql/alter_table.sql | 6 ++++++ src/test/regress/sql/foreign_data.sql | 3 ++- 4 files changed, 18 insertions(+), 2 deletions(-) diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out index f81bdf513b..b63b4d34b0 100644 --- a/src/test/regress/expected/alter_table.out +++ b/src/test/regress/expected/alter_table.out @@ -6,6 +6,7 @@ SET client_min_messages TO 'warning'; DROP ROLE IF EXISTS regress_alter_table_user1; RESET client_min_messages; CREATE USER regress_alter_table_user1; +CREATE VIEW at_v1 AS SELECT 1 as a; -- -- add attribute -- @@ -120,6 +121,9 @@ ALTER INDEX attmp_idx ALTER COLUMN 4 SET STATISTICS 1000; ERROR: column number 4 of relation "attmp_idx" does not exist ALTER INDEX attmp_idx ALTER COLUMN 2 SET STATISTICS -1; DROP TABLE attmp; +-- test that the command correctly complains for the object of a wrong type +ALTER TABLE at_v1 ALTER COLUMN a SET STATISTICS 0; -- ERROR +ERROR: "at_v1" is not a table, materialized view, index, partitioned index, or foreign table -- -- rename - check on both non-temp and temp tables -- @@ -4111,6 +4115,9 @@ ALTER TABLE hash_parted ATTACH PARTITION fail_part FOR VALUES WITH (MODULUS 3, R ERROR: every hash partition modulus must be a factor of the next larger modulus DETAIL: The new modulus 3 is not a factor of 4, the modulus of existing partition "hpart_1". DROP TABLE fail_part; +-- check that attach partition correctly complains for the object of a wrong type +ALTER TABLE at_v1 ATTACH PARTITION dummy default; -- ERROR +ERROR: "at_v1" is not a table or partitioned index -- -- DETACH PARTITION -- diff --git a/src/test/regress/expected/foreign_data.out b/src/test/regress/expected/foreign_data.out index 5385f98a0f..0f56cae353 100644 --- a/src/test/regress/expected/foreign_data.out +++ b/src/test/regress/expected/foreign_data.out @@ -877,8 +877,10 @@ ERROR: column "no_column" of relation "ft1" does not exist ALTER FOREIGN TABLE ft1 DROP COLUMN IF EXISTS no_column; NOTICE: column "no_column" of relation "ft1" does not exist, skipping ALTER FOREIGN TABLE ft1 DROP COLUMN c9; +ALTER FOREIGN TABLE ft1 SET TABLESPACE ts; -- ERROR (wrong object type) +ERROR: "ft1" is not a table, materialized view, index, or partitioned index ALTER FOREIGN TABLE ft1 SET SCHEMA foreign_schema; -ALTER FOREIGN TABLE ft1 SET TABLESPACE ts; -- ERROR +ALTER FOREIGN TABLE ft1 SET TABLESPACE ts; -- ERROR (not found) ERROR: relation "ft1" does not exist ALTER FOREIGN TABLE foreign_schema.ft1 RENAME c1 TO foreign_column_1; ALTER FOREIGN TABLE foreign_schema.ft1 RENAME TO foreign_table_1; diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql index dc0200adcb..0c13478e3e 100644 --- a/src/test/regress/sql/alter_table.sql +++ b/src/test/regress/sql/alter_table.sql @@ -8,6 +8,7 @@ DROP ROLE IF EXISTS regress_alter_table_user1; RESET client_min_messages; CREATE USER regress_alter_table_user1; +CREATE VIEW at_v1 AS SELECT 1 as a; -- -- add attribute @@ -158,6 +159,8 @@ ALTER INDEX attmp_idx ALTER COLUMN 2 SET STATISTICS -1; DROP TABLE attmp; +-- test that the command correctly complains for the object of a wrong type +ALTER TABLE at_v1 ALTER COLUMN a SET STATISTICS 0; -- ERROR -- -- rename - check on both non-temp and temp tables @@ -2640,6 +2643,9 @@ ALTER TABLE hash_parted ATTACH PARTITION fail_part FOR VALUES WITH (MODULUS 8, R ALTER TABLE hash_parted ATTACH PARTITION fail_part FOR VALUES WITH (MODULUS 3, REMAINDER 2); DROP TABLE fail_part; +-- check that attach partition correctly complains for the object of a wrong type +ALTER TABLE at_v1 ATTACH PARTITION dummy default; -- ERROR + -- -- DETACH PARTITION -- diff --git a/src/test/regress/sql/foreign_data.sql b/src/test/regress/sql/foreign_data.sql index 73f9f621d8..e96aef5396 100644 --- a/src/test/regress/sql/foreign_data.sql +++ b/src/test/regress/sql/foreign_data.sql @@ -406,8 +406,9 @@ ALTER FOREIGN TABLE ft1 OPTIONS (DROP delimiter, SET quote '~', ADD escape '@'); ALTER FOREIGN TABLE ft1 DROP COLUMN no_column; -- ERROR ALTER FOREIGN TABLE ft1 DROP COLUMN IF EXISTS no_column; ALTER FOREIGN TABLE ft1 DROP COLUMN c9; +ALTER FOREIGN TABLE ft1 SET TABLESPACE ts; -- ERROR (wrong object type) ALTER FOREIGN TABLE ft1 SET SCHEMA foreign_schema; -ALTER FOREIGN TABLE ft1 SET TABLESPACE ts; -- ERROR +ALTER FOREIGN TABLE ft1 SET TABLESPACE ts; -- ERROR (not found) ALTER FOREIGN TABLE foreign_schema.ft1 RENAME c1 TO foreign_column_1; ALTER FOREIGN TABLE foreign_schema.ft1 RENAME TO foreign_table_1; \d foreign_schema.foreign_table_1 -- 2.27.0 From 71f6da9183e6d41d2f787f6a591772956a2b53df Mon Sep 17 00:00:00 2001 From: Kyotaro Horiguchi <horikyota.ntt@gmail.com> Date: Thu, 8 Jul 2021 18:03:17 +0900 Subject: [PATCH v3 1/2] Add missing targets in ATWrongRelkindError ATWrongRelkindError yields an ambiguous message ""tbl" is of the wrong type" due the lack of some combinations of allowed_targets. Add the missing items. We don't remove apparently unused entries in case someone is using them. --- src/backend/commands/tablecmds.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 816ce8521f..8dc43195a5 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -5372,6 +5372,9 @@ ATWrongRelkindError(Relation rel, int allowed_targets) case ATT_TABLE | ATT_MATVIEW | ATT_INDEX: msg = _("\"%s\" is not a table, materialized view, or index"); break; + case ATT_TABLE | ATT_MATVIEW | ATT_INDEX | ATT_PARTITIONED_INDEX: + msg = _("\"%s\" is not a table, materialized view, index, or partitioned index"); + break; case ATT_TABLE | ATT_MATVIEW | ATT_FOREIGN_TABLE: msg = _("\"%s\" is not a table, materialized view, or foreign table"); break; @@ -5384,6 +5387,9 @@ ATWrongRelkindError(Relation rel, int allowed_targets) case ATT_TABLE | ATT_MATVIEW | ATT_INDEX | ATT_FOREIGN_TABLE: msg = _("\"%s\" is not a table, materialized view, index, or foreign table"); break; + case ATT_TABLE | ATT_PARTITIONED_INDEX: + msg = _("\"%s\" is not a table or partitioned index"); + break; case ATT_VIEW: msg = _("\"%s\" is not a view"); break; -- 2.27.0 From 18c3101d4f2dc95c917a19c82757bd6f4b81fa2a Mon Sep 17 00:00:00 2001 From: Kyotaro Horiguchi <horikyota.ntt@gmail.com> Date: Fri, 9 Jul 2021 20:40:52 +0900 Subject: [PATCH v3 2/2] regress - Add missing targets in ATWrongRelkindError --- src/test/regress/expected/alter_table.out | 4 ++++ src/test/regress/expected/foreign_data.out | 4 +++- src/test/regress/sql/alter_table.sql | 4 ++++ src/test/regress/sql/foreign_data.sql | 3 ++- 4 files changed, 13 insertions(+), 2 deletions(-) diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out index 8d91e2d6c8..5f6d4ed7c3 100644 --- a/src/test/regress/expected/alter_table.out +++ b/src/test/regress/expected/alter_table.out @@ -6,6 +6,7 @@ SET client_min_messages TO 'warning'; DROP ROLE IF EXISTS regress_alter_table_user1; RESET client_min_messages; CREATE USER regress_alter_table_user1; +CREATE VIEW at_v1 AS SELECT 1 as a; -- -- add attribute -- @@ -3948,6 +3949,9 @@ ERROR: remainder for hash partition must be less than modulus ALTER TABLE hash_parted ATTACH PARTITION fail_part FOR VALUES WITH (MODULUS 3, REMAINDER 2); ERROR: every hash partition modulus must be a factor of the next larger modulus DROP TABLE fail_part; +-- check that attach partition correctly complains for the object of a wrong type +ALTER TABLE at_v1 ATTACH PARTITION dummy default; -- ERROR +ERROR: "at_v1" is not a partitioned table or index -- -- DETACH PARTITION -- diff --git a/src/test/regress/expected/foreign_data.out b/src/test/regress/expected/foreign_data.out index b9e25820bc..ffa9287967 100644 --- a/src/test/regress/expected/foreign_data.out +++ b/src/test/regress/expected/foreign_data.out @@ -877,8 +877,10 @@ ERROR: column "no_column" of relation "ft1" does not exist ALTER FOREIGN TABLE ft1 DROP COLUMN IF EXISTS no_column; NOTICE: column "no_column" of relation "ft1" does not exist, skipping ALTER FOREIGN TABLE ft1 DROP COLUMN c9; +ALTER FOREIGN TABLE ft1 SET TABLESPACE ts; -- ERROR (wrong object type) +ERROR: "ft1" is not a table, materialized view, index, or partitioned index ALTER FOREIGN TABLE ft1 SET SCHEMA foreign_schema; -ALTER FOREIGN TABLE ft1 SET TABLESPACE ts; -- ERROR +ALTER FOREIGN TABLE ft1 SET TABLESPACE ts; -- ERROR (not found) ERROR: relation "ft1" does not exist ALTER FOREIGN TABLE foreign_schema.ft1 RENAME c1 TO foreign_column_1; ALTER FOREIGN TABLE foreign_schema.ft1 RENAME TO foreign_table_1; diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql index cd925ea8ce..a92cea7f87 100644 --- a/src/test/regress/sql/alter_table.sql +++ b/src/test/regress/sql/alter_table.sql @@ -8,6 +8,7 @@ DROP ROLE IF EXISTS regress_alter_table_user1; RESET client_min_messages; CREATE USER regress_alter_table_user1; +CREATE VIEW at_v1 AS SELECT 1 as a; -- -- add attribute @@ -2593,6 +2594,9 @@ ALTER TABLE hash_parted ATTACH PARTITION fail_part FOR VALUES WITH (MODULUS 8, R ALTER TABLE hash_parted ATTACH PARTITION fail_part FOR VALUES WITH (MODULUS 3, REMAINDER 2); DROP TABLE fail_part; +-- check that attach partition correctly complains for the object of a wrong type +ALTER TABLE at_v1 ATTACH PARTITION dummy default; -- ERROR + -- -- DETACH PARTITION -- diff --git a/src/test/regress/sql/foreign_data.sql b/src/test/regress/sql/foreign_data.sql index 73f9f621d8..e96aef5396 100644 --- a/src/test/regress/sql/foreign_data.sql +++ b/src/test/regress/sql/foreign_data.sql @@ -406,8 +406,9 @@ ALTER FOREIGN TABLE ft1 OPTIONS (DROP delimiter, SET quote '~', ADD escape '@'); ALTER FOREIGN TABLE ft1 DROP COLUMN no_column; -- ERROR ALTER FOREIGN TABLE ft1 DROP COLUMN IF EXISTS no_column; ALTER FOREIGN TABLE ft1 DROP COLUMN c9; +ALTER FOREIGN TABLE ft1 SET TABLESPACE ts; -- ERROR (wrong object type) ALTER FOREIGN TABLE ft1 SET SCHEMA foreign_schema; -ALTER FOREIGN TABLE ft1 SET TABLESPACE ts; -- ERROR +ALTER FOREIGN TABLE ft1 SET TABLESPACE ts; -- ERROR (not found) ALTER FOREIGN TABLE foreign_schema.ft1 RENAME c1 TO foreign_column_1; ALTER FOREIGN TABLE foreign_schema.ft1 RENAME TO foreign_table_1; \d foreign_schema.foreign_table_1 -- 2.27.0
pgsql-hackers by date: