pg.ddx.io pgsql-bugs@postgresql.org mailing list archive
help / color / mirror / Atom feedBUG #18297: Error when adding a column to a parent table with complex inheritance
12+ messages / 5 participants
[nested] [flat]
Commits from this thread:
2024-01-24 bb812ab091 51193e7a7d ad6fbbeeb0 5a7833f496 bfec14d06d 2f72428371 Fix ALTER TABLE .. ADD COLUMN with complex inheritance trees
* BUG #18297: Error when adding a column to a parent table with complex inheritance
@ 2024-01-16 04:00 PG Bug reporting form <noreply@postgresql.org>
0 siblings, 1 reply; 12+ messages in thread
From: PG Bug reporting form @ 2024-01-16 04:00 UTC (permalink / raw)
To: pgsql-bugs@lists.postgresql.org; +Cc: exclusion@gmail.com
The following bug has been logged on the website:
Bug reference: 18297
Logged by: Alexander Lakhin
Email address: exclusion@gmail.com
PostgreSQL version: 16.1
Operating system: Ubuntu 22.04
Description:
The following query:
CREATE TABLE a ();
CREATE TABLE b () INHERITS (a);
CREATE TABLE c () INHERITS (b);
CREATE TABLE d () INHERITS (a,b,c);
ALTER TABLE a ADD COLUMN i int;
fails with:
NOTICE: merging definition of column "i" for child "d"
ERROR: tuple already updated by self
While with a simpler hierarchy:
CREATE TABLE a ();
CREATE TABLE b () INHERITS (a);
CREATE TABLE c () INHERITS (a,b);
the column added successfully:
ALTER TABLE a ADD COLUMN i int;
NOTICE: merging definition of column "i" for child "c"
ALTER TABLE
In the failed case the error occurred when table d was processed the third
time. First (following chain a -> b -> c-> d) the table got the column i
added, second (a -> b -> d) it got merged column definition, third
(a -> d) an attempt to merge the column definition once more failed.
This error can be seen at least on REL_12_STABLE .. master.
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: BUG #18297: Error when adding a column to a parent table with complex inheritance
@ 2024-01-17 07:48 Tender Wang <tndrwang@gmail.com>
parent: PG Bug reporting form <noreply@postgresql.org>
0 siblings, 1 reply; 12+ messages in thread
From: Tender Wang @ 2024-01-17 07:48 UTC (permalink / raw)
To: exclusion@gmail.com; pgsql-bugs@lists.postgresql.org
Hmm, thanks for the report.
I can repeat the aboved issue on master, even on pg10 and pg 11.
I analyzed this issue, and I found that ATExecAddColumn(), we forgot to
call CommandCounterIncrement() in if (colDef->inhcount > 0) {...} branch.
So the third(a->d) updates the first(a->b->c->d) tuple.
Attached patch is my quickly fixed solution.
--
Tender Wang
OpenPie: https://en.openpie.com/
PG Bug reporting form <noreply@postgresql.org> 于2024年1月16日周二 17:32写道：
> The following bug has been logged on the website:
>
> Bug reference: 18297
> Logged by: Alexander Lakhin
> Email address: exclusion@gmail.com
> PostgreSQL version: 16.1
> Operating system: Ubuntu 22.04
> Description:
>
> The following query:
> CREATE TABLE a ();
> CREATE TABLE b () INHERITS (a);
> CREATE TABLE c () INHERITS (b);
> CREATE TABLE d () INHERITS (a,b,c);
>
> ALTER TABLE a ADD COLUMN i int;
> fails with:
> NOTICE: merging definition of column "i" for child "d"
> ERROR: tuple already updated by self
>
> While with a simpler hierarchy:
> CREATE TABLE a ();
> CREATE TABLE b () INHERITS (a);
> CREATE TABLE c () INHERITS (a,b);
>
> the column added successfully:
> ALTER TABLE a ADD COLUMN i int;
> NOTICE: merging definition of column "i" for child "c"
> ALTER TABLE
>
> In the failed case the error occurred when table d was processed the third
> time. First (following chain a -> b -> c-> d) the table got the column i
> added, second (a -> b -> d) it got merged column definition, third
> (a -> d) an attempt to merge the column definition once more failed.
>
> This error can be seen at least on REL_12_STABLE .. master.
>
>
Attachments:
0001-Fix-self-updated-tuple-invisible-for-catalog-tuple.patch (application/octet-stream, 3.0K) download
From 3209272bff1025f547893f2cc857e8eedaf3f13d Mon Sep 17 00:00:00 2001
From: "tender.wang" <tender.wang@openpie.com>
Date: Wed, 17 Jan 2024 15:36:38 +0800
Subject: [PATCH] Fix self updated tuple invisible for catalog tuple.
We may update tuple more times in complex inheritance case.
Previously, we forgot to update the Command Counter, so we couldn't
update the tuple which we updated it before.
---
src/backend/commands/tablecmds.c | 2 ++
src/test/regress/expected/inherit.out | 16 ++++++++++++++++
src/test/regress/sql/inherit.sql | 8 ++++++++
3 files changed, 26 insertions(+)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 2a56a4357c..198b763c2c 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -7077,6 +7077,8 @@ ATExecAddColumn(List **wqueue, AlteredTableInfo *tab, Relation rel,
colDef->colname, RelationGetRelationName(rel))));
table_close(attrdesc, RowExclusiveLock);
+ /* Make the attribute's catalog entry visible */
+ CommandCounterIncrement();
return InvalidObjectAddress;
}
}
diff --git a/src/test/regress/expected/inherit.out b/src/test/regress/expected/inherit.out
index 0f1aa831f6..c748e08b60 100644
--- a/src/test/regress/expected/inherit.out
+++ b/src/test/regress/expected/inherit.out
@@ -1088,6 +1088,22 @@ Inherits: inht1,
inhs1
DROP TABLE inhts;
+-- Test for adding a column to a parent table with complex inheritance
+CREATE TABLE inhta ();
+CREATE TABLE inhtb () INHERITS (inhta);
+CREATE TABLE inhtc () INHERITS (inhtb);
+CREATE TABLE inhtd () INHERITS (inhta,inhtb,inhtc);
+ALTER TABLE inhta ADD COLUMN i int;
+NOTICE: merging definition of column "i" for child "inhtd"
+NOTICE: merging definition of column "i" for child "inhtd"
+\d+ inhta
+ Table "public.inhta"
+ Column | Type | Collation | Nullable | Default | Storage | Stats target | Description
+--------+---------+-----------+----------+---------+---------+--------------+-------------
+ i | integer | | | | plain | |
+Child tables: inhtb,
+ inhtd
+
-- Test for renaming in diamond inheritance
CREATE TABLE inht2 (x int) INHERITS (inht1);
CREATE TABLE inht3 (y int) INHERITS (inht1);
diff --git a/src/test/regress/sql/inherit.sql b/src/test/regress/sql/inherit.sql
index f7b4062ce5..2c4d8ed7e4 100644
--- a/src/test/regress/sql/inherit.sql
+++ b/src/test/regress/sql/inherit.sql
@@ -372,6 +372,14 @@ ALTER TABLE inhts RENAME d TO dd;
DROP TABLE inhts;
+-- Test for adding a column to a parent table with complex inheritance
+CREATE TABLE inhta ();
+CREATE TABLE inhtb () INHERITS (inhta);
+CREATE TABLE inhtc () INHERITS (inhtb);
+CREATE TABLE inhtd () INHERITS (inhta,inhtb,inhtc);
+ALTER TABLE inhta ADD COLUMN i int;
+\d+ inhta
+
-- Test for renaming in diamond inheritance
CREATE TABLE inht2 (x int) INHERITS (inht1);
CREATE TABLE inht3 (y int) INHERITS (inht1);
--
2.25.1
* Re: BUG #18297: Error when adding a column to a parent table with complex inheritance
@ 2024-01-17 11:54 Richard Guo <guofenglinux@gmail.com>
parent: Tender Wang <tndrwang@gmail.com>
0 siblings, 1 reply; 12+ messages in thread
From: Richard Guo @ 2024-01-17 11:54 UTC (permalink / raw)
To: Tender Wang <tndrwang@gmail.com>; +Cc: exclusion@gmail.com; pgsql-bugs@lists.postgresql.org
On Wed, Jan 17, 2024 at 3:48 PM Tender Wang <tndrwang@gmail.com> wrote:
> Hmm, thanks for the report.
> I can repeat the aboved issue on master, even on pg10 and pg 11.
> I analyzed this issue, and I found that ATExecAddColumn(), we forgot to
> call CommandCounterIncrement() in if (colDef->inhcount > 0) {...} branch.
> So the third(a->d) updates the first(a->b->c->d) tuple.
> Attached patch is my quickly fixed solution.
>
Indeed. We may update the same child column multiple times, but there
is no CommandCounterIncrement between. Nice catch and +1 to the fix.
To nitpick, how about go with the comment as
/* Make sure the child column change is visible */
which seems clearer.
Also I think it'd be better to include a blank line before and after the
new CommandCounterIncrement statement.
Also I suggest to drop the new added tables after we've run ALTER TABLE
in the test case.
Thanks
Richard
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: BUG #18297: Error when adding a column to a parent table with complex inheritance
@ 2024-01-17 15:03 Tender Wang <tndrwang@gmail.com>
parent: Richard Guo <guofenglinux@gmail.com>
0 siblings, 1 reply; 12+ messages in thread
From: Tender Wang @ 2024-01-17 15:03 UTC (permalink / raw)
To: Richard Guo <guofenglinux@gmail.com>; +Cc: exclusion@gmail.com; pgsql-bugs@lists.postgresql.org
Thanks for reviewing the patch.
The attached v2 patch includes all review advices.
--
Tender Wang
OpenPie: https://en.openpie.com/
Richard Guo <guofenglinux@gmail.com> 于2024年1月17日周三 19:54写道：
>
> On Wed, Jan 17, 2024 at 3:48 PM Tender Wang <tndrwang@gmail.com> wrote:
>
>> Hmm, thanks for the report.
>> I can repeat the aboved issue on master, even on pg10 and pg 11.
>> I analyzed this issue, and I found that ATExecAddColumn(), we forgot to
>> call CommandCounterIncrement() in if (colDef->inhcount > 0) {...} branch.
>> So the third(a->d) updates the first(a->b->c->d) tuple.
>> Attached patch is my quickly fixed solution.
>>
>
> Indeed. We may update the same child column multiple times, but there
> is no CommandCounterIncrement between. Nice catch and +1 to the fix.
>
> To nitpick, how about go with the comment as
>
> /* Make sure the child column change is visible */
>
> which seems clearer.
>
> Also I think it'd be better to include a blank line before and after the
> new CommandCounterIncrement statement.
>
> Also I suggest to drop the new added tables after we've run ALTER TABLE
> in the test case.
>
> Thanks
> Richard
>
Attachments:
v2-0001-Fix-self-updated-tuple-invisible-for-catalog-tupl.patch (application/octet-stream, 3.1K) download
From 1dd7a8f3da3cf018a692ca1b9e770012ee3a8967 Mon Sep 17 00:00:00 2001
From: "tender.wang" <tender.wang@openpie.com>
Date: Wed, 17 Jan 2024 23:00:11 +0800
Subject: [PATCH v2] Fix self updated tuple invisible for catalog tuple.
We may update tuple more times in complex inheritance case.
Previously, we forgot to update the Command Counter, so we couldn't
update the tuple which we updated it before.
---
src/backend/commands/tablecmds.c | 6 +++++-
src/test/regress/expected/inherit.out | 17 +++++++++++++++++
src/test/regress/sql/inherit.sql | 11 +++++++++++
3 files changed, 33 insertions(+), 1 deletion(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 2a56a4357c..1c0f4bfc78 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -7077,7 +7077,11 @@ ATExecAddColumn(List **wqueue, AlteredTableInfo *tab, Relation rel,
colDef->colname, RelationGetRelationName(rel))));
table_close(attrdesc, RowExclusiveLock);
- return InvalidObjectAddress;
+
+ /* Make sure the child column change is visible */
+ CommandCounterIncrement();
+
+ return InvalidObjectAddress;
}
}
diff --git a/src/test/regress/expected/inherit.out b/src/test/regress/expected/inherit.out
index 0f1aa831f6..8626ce4a05 100644
--- a/src/test/regress/expected/inherit.out
+++ b/src/test/regress/expected/inherit.out
@@ -1088,6 +1088,23 @@ Inherits: inht1,
inhs1
DROP TABLE inhts;
+-- Test for adding a column to a parent table with complex inheritance
+CREATE TABLE inhta ();
+CREATE TABLE inhtb () INHERITS (inhta);
+CREATE TABLE inhtc () INHERITS (inhtb);
+CREATE TABLE inhtd () INHERITS (inhta,inhtb,inhtc);
+ALTER TABLE inhta ADD COLUMN i int;
+NOTICE: merging definition of column "i" for child "inhtd"
+NOTICE: merging definition of column "i" for child "inhtd"
+\d+ inhta
+ Table "public.inhta"
+ Column | Type | Collation | Nullable | Default | Storage | Stats target | Description
+--------+---------+-----------+----------+---------+---------+--------------+-------------
+ i | integer | | | | plain | |
+Child tables: inhtb,
+ inhtd
+
+DROP TABLE inhta, inhtb, inhtc, inhtd;
-- Test for renaming in diamond inheritance
CREATE TABLE inht2 (x int) INHERITS (inht1);
CREATE TABLE inht3 (y int) INHERITS (inht1);
diff --git a/src/test/regress/sql/inherit.sql b/src/test/regress/sql/inherit.sql
index f7b4062ce5..8e537f7da3 100644
--- a/src/test/regress/sql/inherit.sql
+++ b/src/test/regress/sql/inherit.sql
@@ -372,6 +372,17 @@ ALTER TABLE inhts RENAME d TO dd;
DROP TABLE inhts;
+-- Test for adding a column to a parent table with complex inheritance
+CREATE TABLE inhta ();
+CREATE TABLE inhtb () INHERITS (inhta);
+CREATE TABLE inhtc () INHERITS (inhtb);
+CREATE TABLE inhtd () INHERITS (inhta,inhtb,inhtc);
+
+ALTER TABLE inhta ADD COLUMN i int;
+\d+ inhta
+
+DROP TABLE inhta, inhtb, inhtc, inhtd;
+
-- Test for renaming in diamond inheritance
CREATE TABLE inht2 (x int) INHERITS (inht1);
CREATE TABLE inht3 (y int) INHERITS (inht1);
--
2.25.1
* Re: BUG #18297: Error when adding a column to a parent table with complex inheritance
@ 2024-01-18 09:00 Alexander Lakhin <exclusion@gmail.com>
parent: Tender Wang <tndrwang@gmail.com>
0 siblings, 2 replies; 12+ messages in thread
From: Alexander Lakhin @ 2024-01-18 09:00 UTC (permalink / raw)
To: Tender Wang <tndrwang@gmail.com>; Richard Guo <guofenglinux@gmail.com>; +Cc: pgsql-bugs@lists.postgresql.org
Hello Tender Wang and Richard,
17.01.2024 18:03, Tender Wang wrote:
> Thanks for reviewing the patch.
> The attached v2 patch includes all review advices.
>
> --
> Tender Wang
> OpenPie: https://en.openpie.com/
>
> Richard Guo <guofenglinux@gmail.com> 于2024年1月17日周三 19:54写道：
>
>
> Indeed. We may update the same child column multiple times, but there
> is no CommandCounterIncrement between. Nice catch and +1 to the fix.
>
> To nitpick, how about go with the comment as
>
> /* Make sure the child column change is visible */
>
> which seems clearer.
>
> Also I think it'd be better to include a blank line before and after the
> new CommandCounterIncrement statement.
>
> Also I suggest to drop the new added tables after we've run ALTER TABLE
> in the test case.
>
> Thanks
> Richard
>
Thank you for working on this!
(Maybe it's possible to slightly modify an existing hierarchy with tables
a, b, c in inherit.sql for the purpose of the check added.)
I've found another situation where the same error is produced:
CREATE ROLE u;
DROP ROLE u, u;
ERROR: tuple already updated by self
Perhaps, you would like to fix it in passing too. I've rechecked all the
other object types, that can be DROPped with a list (namely, AGGREGATE,
DOMAIN, EXTENSION, FOREIGN DATA WRAPPER, FOREIGN TABLE, FUNCTION, INDEX,
MATERIALIZED VIEW, OPERATOR, PROCEDURE, ROUTINE, SEQUENCE, SERVER,
STATISTICS, TABLE, TYPE, VIEW), and found that all of these handle
such duplicate entries with no error.
Best regards,
Alexander
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: BUG #18297: Error when adding a column to a parent table with complex inheritance
@ 2024-01-18 15:42 Tender Wang <tndrwang@gmail.com>
parent: Alexander Lakhin <exclusion@gmail.com>
1 sibling, 1 reply; 12+ messages in thread
From: Tender Wang @ 2024-01-18 15:42 UTC (permalink / raw)
To: Alexander Lakhin <exclusion@gmail.com>; +Cc: Richard Guo <guofenglinux@gmail.com>; pgsql-bugs@lists.postgresql.org
Alexander Lakhin <exclusion@gmail.com> 于2024年1月18日周四 17:00写道：
> Hello Tender Wang and Richard,
>
> 17.01.2024 18:03, Tender Wang wrote:
>
> Thanks for reviewing the patch.
> The attached v2 patch includes all review advices.
>
> --
> Tender Wang
> OpenPie: https://en.openpie.com/
>
> Richard Guo <guofenglinux@gmail.com> 于2024年1月17日周三 19:54写道：
>
>>
>> Indeed. We may update the same child column multiple times, but there
>> is no CommandCounterIncrement between. Nice catch and +1 to the fix.
>>
>> To nitpick, how about go with the comment as
>>
>> /* Make sure the child column change is visible */
>>
>> which seems clearer.
>>
>> Also I think it'd be better to include a blank line before and after the
>> new CommandCounterIncrement statement.
>>
>> Also I suggest to drop the new added tables after we've run ALTER TABLE
>> in the test case.
>>
>> Thanks
>> Richard
>>
>
> Thank you for working on this!
>
> (Maybe it's possible to slightly modify an existing hierarchy with tables
> a, b, c in inherit.sql for the purpose of the check added.)
>
> I've found another situation where the same error is produced:
> CREATE ROLE u;
> DROP ROLE u, u;
> ERROR: tuple already updated by self
>
Indeed, but this issue cann't simply calling CommandCounterIncrement() to
fix.
It will report "could not find tuple for role" if we simply calling
CommandCounterIncrement() when drop seond 'u'.
I think we can sort the role_addresses list and skip the same role id. I
don't intend to fix above two issues in one patch.
So I add a new 0001 attached patch.
Perhaps, you would like to fix it in passing too. I've rechecked all the
> other object types, that can be DROPped with a list (namely, AGGREGATE,
> DOMAIN, EXTENSION, FOREIGN DATA WRAPPER, FOREIGN TABLE, FUNCTION, INDEX,
> MATERIALIZED VIEW, OPERATOR, PROCEDURE, ROUTINE, SEQUENCE, SERVER,
> STATISTICS, TABLE, TYPE, VIEW), and found that all of these handle
> such duplicate entries with no error.
>
I'm not sure if there are other problems like this.
> Best regards,
> Alexander
>
Attachments:
0001-Fix-dropped-same-role-id-twice-error.patch (application/octet-stream, 3.3K) download
From 344dc1fbfe572ef0f4b93c00ce811aa524e30ec4 Mon Sep 17 00:00:00 2001
From: "tender.wang" <tender.wang@openpie.com>
Date: Thu, 18 Jan 2024 23:26:16 +0800
Subject: [PATCH] Fix dropped same role id twice error.
---
src/backend/commands/user.c | 23 +++++++++++++++++++++++
src/test/regress/expected/create_role.out | 3 +++
src/test/regress/sql/create_role.sql | 5 +++++
3 files changed, 31 insertions(+)
diff --git a/src/backend/commands/user.c b/src/backend/commands/user.c
index 7e81589711..1fc6b9e201 100644
--- a/src/backend/commands/user.c
+++ b/src/backend/commands/user.c
@@ -1083,6 +1083,19 @@ AlterRoleSet(AlterRoleSetStmt *stmt)
return roleid;
}
+/* list_sort comparator to sort ObjectAddress by objectId */
+static int
+objectId_comparator(const ListCell *p1, const ListCell *p2)
+{
+ Oid o1 = ((ObjectAddress *) lfirst(p1))->objectId;
+ Oid o2 = ((ObjectAddress *) lfirst(p2))->objectId;
+
+ if (o1 > o2)
+ return -1;
+ if (o1 < o2)
+ return 1;
+ return 0;
+}
/*
* DROP ROLE
@@ -1093,6 +1106,7 @@ DropRole(DropRoleStmt *stmt)
Relation pg_authid_rel,
pg_auth_members_rel;
ListCell *item;
+ ObjectAddress *pre_object = NULL;
List *role_addresses = NIL;
if (!have_createrole_privilege())
@@ -1268,6 +1282,9 @@ DropRole(DropRoleStmt *stmt)
role_addresses = lappend(role_addresses, role_address);
}
+ /* Sort the role_addresses to avoid to remove same roleid repeatedly */
+ list_sort(role_addresses, objectId_comparator);
+
/*
* Second pass over the roles to be removed.
*/
@@ -1280,6 +1297,12 @@ DropRole(DropRoleStmt *stmt)
char *detail;
char *detail_log;
+ /* Skip same roleid */
+ if (pre_object &&
+ pre_object->objectId == role_address->objectId)
+ continue;
+ else
+ pre_object = role_address;
/*
* Re-find the pg_authid tuple.
*
diff --git a/src/test/regress/expected/create_role.out b/src/test/regress/expected/create_role.out
index 7117e943c2..cbfd1c6b56 100644
--- a/src/test/regress/expected/create_role.out
+++ b/src/test/regress/expected/create_role.out
@@ -4,6 +4,9 @@ CREATE ROLE regress_role_admin CREATEDB CREATEROLE REPLICATION BYPASSRLS;
GRANT CREATE ON DATABASE regression TO regress_role_admin WITH GRANT OPTION;
CREATE ROLE regress_role_limited_admin CREATEROLE;
CREATE ROLE regress_role_normal;
+--ok, can drop same role repeatedly
+CREATE ROLE regress_role_dropped;
+DROP ROLE regress_role_dropped, regress_role_dropped;
-- fail, CREATEROLE user can't give away role attributes without having them
SET SESSION AUTHORIZATION regress_role_limited_admin;
CREATE ROLE regress_nosuch_superuser SUPERUSER;
diff --git a/src/test/regress/sql/create_role.sql b/src/test/regress/sql/create_role.sql
index 12582a3cc2..96a503bf2d 100644
--- a/src/test/regress/sql/create_role.sql
+++ b/src/test/regress/sql/create_role.sql
@@ -5,6 +5,11 @@ GRANT CREATE ON DATABASE regression TO regress_role_admin WITH GRANT OPTION;
CREATE ROLE regress_role_limited_admin CREATEROLE;
CREATE ROLE regress_role_normal;
+--ok, can drop same role repeatedly
+CREATE ROLE regress_role_dropped;
+
+DROP ROLE regress_role_dropped, regress_role_dropped;
+
-- fail, CREATEROLE user can't give away role attributes without having them
SET SESSION AUTHORIZATION regress_role_limited_admin;
CREATE ROLE regress_nosuch_superuser SUPERUSER;
--
2.25.1
* Re: BUG #18297: Error when adding a column to a parent table with complex inheritance
@ 2024-01-19 09:00 Alexander Lakhin <exclusion@gmail.com>
parent: Tender Wang <tndrwang@gmail.com>
0 siblings, 1 reply; 12+ messages in thread
From: Alexander Lakhin @ 2024-01-19 09:00 UTC (permalink / raw)
To: Tender Wang <tndrwang@gmail.com>; +Cc: Richard Guo <guofenglinux@gmail.com>; pgsql-bugs@lists.postgresql.org
18.01.2024 18:42, Tender Wang wrote:
>
> I think we can sort the role_addresses list and skip the same role id. I don't intend to fix above two issues in one
> patch.
> So I add a new 0001 attached patch.
>
>
> Perhaps, you would like to fix it in passing too. I've rechecked all the
> other object types, that can be DROPped with a list (namely, AGGREGATE,
> DOMAIN, EXTENSION, FOREIGN DATA WRAPPER, FOREIGN TABLE, FUNCTION, INDEX,
> MATERIALIZED VIEW, OPERATOR, PROCEDURE, ROUTINE, SEQUENCE, SERVER,
> STATISTICS, TABLE, TYPE, VIEW), and found that all of these handle
> such duplicate entries with no error.
>
>
> I'm not sure if there are other problems like this.
>
Yes, I've encountered a similar issue, this time with ALTER (TEXT SEARCH
CONFIGURATION):
CREATE TEXT SEARCH CONFIGURATION ispell_tst (COPY=english);
CREATE TEXT SEARCH DICTIONARY ispell (Template=ispell, DictFile=ispell_sample, AffFile=ispell_sample);
ALTER TEXT SEARCH CONFIGURATION ispell_tst ALTER MAPPING FOR word, word WITH ispell;
ERROR: tuple already updated by self
And again, I could not get similar errors with all the other ALTER commands,
that accept lists.
Sorry for mixing two different problems together, I should have reported it
separately.
Best regards,
Alexander
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: BUG #18297: Error when adding a column to a parent table with complex inheritance
@ 2024-01-22 10:36 Tender Wang <tndrwang@gmail.com>
parent: Alexander Lakhin <exclusion@gmail.com>
0 siblings, 1 reply; 12+ messages in thread
From: Tender Wang @ 2024-01-22 10:36 UTC (permalink / raw)
To: Alexander Lakhin <exclusion@gmail.com>; +Cc: Richard Guo <guofenglinux@gmail.com>; pgsql-bugs@lists.postgresql.org
Alexander Lakhin <exclusion@gmail.com> 于2024年1月19日周五 17:00写道：
> 18.01.2024 18:42, Tender Wang wrote:
>
>
> I think we can sort the role_addresses list and skip the same role id. I
> don't intend to fix above two issues in one patch.
> So I add a new 0001 attached patch.
>
>
> Perhaps, you would like to fix it in passing too. I've rechecked all the
>> other object types, that can be DROPped with a list (namely, AGGREGATE,
>> DOMAIN, EXTENSION, FOREIGN DATA WRAPPER, FOREIGN TABLE, FUNCTION, INDEX,
>> MATERIALIZED VIEW, OPERATOR, PROCEDURE, ROUTINE, SEQUENCE, SERVER,
>> STATISTICS, TABLE, TYPE, VIEW), and found that all of these handle
>> such duplicate entries with no error.
>>
>
> I'm not sure if there are other problems like this.
>
>
> Yes, I've encountered a similar issue, this time with ALTER (TEXT SEARCH
> CONFIGURATION):
> CREATE TEXT SEARCH CONFIGURATION ispell_tst (COPY=english);
> CREATE TEXT SEARCH DICTIONARY ispell (Template=ispell,
> DictFile=ispell_sample, AffFile=ispell_sample);
> ALTER TEXT SEARCH CONFIGURATION ispell_tst ALTER MAPPING FOR word, word
> WITH ispell;
> ERROR: tuple already updated by self
>
Yes, this is another issue that DDL operation on an same object twice.
Maybe we can report error on Parse phase.
But as you say, all the other DDL commands run OK, so maybe it's better to
process this case in execution phase.
I will send a patch later.
And I'm just curious that how do you find these issues? Use some tools?
>
> And again, I could not get similar errors with all the other ALTER
> commands,
> that accept lists.
>
> Sorry for mixing two different problems together, I should have reported it
> separately.
>
> Best regards,
> Alexander
>
--
--
Tender Wang
OpenPie: https://en.openpie.com/
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: BUG #18297: Error when adding a column to a parent table with complex inheritance
@ 2024-01-22 17:00 Alexander Lakhin <exclusion@gmail.com>
parent: Tender Wang <tndrwang@gmail.com>
0 siblings, 1 reply; 12+ messages in thread
From: Alexander Lakhin @ 2024-01-22 17:00 UTC (permalink / raw)
To: Tender Wang <tndrwang@gmail.com>; +Cc: Richard Guo <guofenglinux@gmail.com>; pgsql-bugs@lists.postgresql.org
Hi,
22.01.2024 13:36, Tender Wang wrote:
>
> Yes, I've encountered a similar issue, this time with ALTER (TEXT SEARCH
> CONFIGURATION):
> CREATE TEXT SEARCH CONFIGURATION ispell_tst (COPY=english);
> CREATE TEXT SEARCH DICTIONARY ispell (Template=ispell, DictFile=ispell_sample, AffFile=ispell_sample);
> ALTER TEXT SEARCH CONFIGURATION ispell_tst ALTER MAPPING FOR word, word WITH ispell;
> ERROR: tuple already updated by self
>
>
> Yes, this is another issue that DDL operation on an same object twice. Maybe we can report error on Parse phase.
> But as you say, all the other DDL commands run OK, so maybe it's better to process this case in execution phase.
> I will send a patch later.
Thank you for working on this!
As these two cases look like exceptions to the common behavior, I wonder
whether we need some extra functions to deal with duplicates.
(I haven't look yet how such duplicates processed for other object types.)
>
> And I'm just curious that how do you find these issues? Use some tools?
I discovered these two issues with my kind of fuzzer, just watching out
for the interesting errors.
Best regards,
Alexander
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: BUG #18297: Error when adding a column to a parent table with complex inheritance
@ 2024-01-23 03:19 Tender Wang <tndrwang@gmail.com>
parent: Alexander Lakhin <exclusion@gmail.com>
0 siblings, 1 reply; 12+ messages in thread
From: Tender Wang @ 2024-01-23 03:19 UTC (permalink / raw)
To: Alexander Lakhin <exclusion@gmail.com>; +Cc: Richard Guo <guofenglinux@gmail.com>; pgsql-bugs@lists.postgresql.org
Alexander Lakhin <exclusion@gmail.com> 于2024年1月23日周二 01:00写道：
> Hi,
>
> 22.01.2024 13:36, Tender Wang wrote:
>
>
> Yes, I've encountered a similar issue, this time with ALTER (TEXT SEARCH
>> CONFIGURATION):
>> CREATE TEXT SEARCH CONFIGURATION ispell_tst (COPY=english);
>> CREATE TEXT SEARCH DICTIONARY ispell (Template=ispell,
>> DictFile=ispell_sample, AffFile=ispell_sample);
>> ALTER TEXT SEARCH CONFIGURATION ispell_tst ALTER MAPPING FOR word, word
>> WITH ispell;
>> ERROR: tuple already updated by self
>>
>
> Yes, this is another issue that DDL operation on an same object twice.
> Maybe we can report error on Parse phase.
> But as you say, all the other DDL commands run OK, so maybe it's better to
> process this case in execution phase.
> I will send a patch later.
>
>
> Thank you for working on this!
>
> As these two cases look like exceptions to the common behavior, I wonder
> whether we need some extra functions to deal with duplicates.
> (I haven't look yet how such duplicates processed for other object types.)
>
For example, drop extension pageinspect ,pageinspect;
The duplicates will be done in object_address_present_add_flags(), some
codes as below:
....
if (object->classId == thisobj->classId &&
object->objectId == thisobj->objectId)
{
if (object->objectSubId == thisobj->objectSubId)
{
ObjectAddressExtra *thisextra = addrs->extras + i;
thisextra->flags |= flags;
result = true;
}
....
The object will not be added to targetObjects if above func returned true.
Your reported two issues have different code path, and duplicates do not
processed.
I am considering whether there is a unified way of handling this, otherwise
each one will have to be dealt with separately.
>
> And I'm just curious that how do you find these issues? Use some tools?
>
>
>
> I discovered these two issues with my kind of fuzzer, just watching out
> for the interesting errors.
>
> Best regards,
> Alexander
>
--
Tender Wang
OpenPie: https://en.openpie.com/
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: BUG #18297: Error when adding a column to a parent table with complex inheritance
@ 2024-01-24 06:12 Michael Paquier <michael@paquier.xyz>
parent: Alexander Lakhin <exclusion@gmail.com>
1 sibling, 0 replies; 12+ messages in thread
From: Michael Paquier @ 2024-01-24 06:12 UTC (permalink / raw)
To: Alexander Lakhin <exclusion@gmail.com>; +Cc: Tender Wang <tndrwang@gmail.com>; Richard Guo <guofenglinux@gmail.com>; pgsql-bugs@lists.postgresql.org
On Thu, Jan 18, 2024 at 12:00:01PM +0300, Alexander Lakhin wrote:
> Thank you for working on this!
Yep, this one is obviously wrong so I have applied and backpatched the
fix for tablecmds.c.
> (Maybe it's possible to slightly modify an existing hierarchy with tables
> a, b, c in inherit.sql for the purpose of the check added.)
I was wondering about that, but decided to keep a separate test as
suggested as the other tests with inheritance are already tightly
linked to their own assumptions, mostly with renames and constraints.
--
Michael
Attachments:
signature.asc (application/pgp-signature, 832B) download
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: BUG #18297: Error when adding a column to a parent table with complex inheritance
@ 2024-01-24 06:20 Michael Paquier <michael@paquier.xyz>
parent: Tender Wang <tndrwang@gmail.com>
0 siblings, 0 replies; 12+ messages in thread
From: Michael Paquier @ 2024-01-24 06:20 UTC (permalink / raw)
To: Tender Wang <tndrwang@gmail.com>; +Cc: Alexander Lakhin <exclusion@gmail.com>; Richard Guo <guofenglinux@gmail.com>; pgsql-bugs@lists.postgresql.org
On Tue, Jan 23, 2024 at 11:19:57AM +0800, Tender Wang wrote:
> Your reported two issues have different code path, and duplicates do not
> processed.
>
> I am considering whether there is a unified way of handling this, otherwise
> each one will have to be dealt with separately.
Yes, I don't agree with what you have posted on [1], inventing a new
way to handle what is basically a way to remove duplicated
ObjectAddresses, and that's what we use in the DropRole() path.
The problem reported on this thread has now been fixed as of
bb812ab0917e, so why not moving the rest to a different thread with a
proper subject? This will attract a better audience for the topic
dealt with.
[1]: https://www.postgresql.org/message-id/CAHewXN=dPJDvaoME0G9vyemUmY-TpDuqQfcHJRWfUvSWX1p=rQ@mail.gmail...
--
Michael
Attachments:
signature.asc (application/pgp-signature, 832B) download
^ permalink raw reply [nested|flat] 12+ messages in thread
end of thread, other threads:[~2024-01-24 06:20 UTC | newest]
Thread overview: 12+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2024-01-16 04:00 BUG #18297: Error when adding a column to a parent table with complex inheritance PG Bug reporting form <noreply@postgresql.org>
2024-01-17 07:48 ` Tender Wang <tndrwang@gmail.com>
2024-01-17 11:54 ` Richard Guo <guofenglinux@gmail.com>
2024-01-17 15:03 ` Tender Wang <tndrwang@gmail.com>
2024-01-18 09:00 ` Alexander Lakhin <exclusion@gmail.com>
2024-01-18 15:42 ` Tender Wang <tndrwang@gmail.com>
2024-01-19 09:00 ` Alexander Lakhin <exclusion@gmail.com>
2024-01-22 10:36 ` Tender Wang <tndrwang@gmail.com>
2024-01-22 17:00 ` Alexander Lakhin <exclusion@gmail.com>
2024-01-23 03:19 ` Tender Wang <tndrwang@gmail.com>
2024-01-24 06:20 ` Michael Paquier <michael@paquier.xyz>
2024-01-24 06:12 ` Michael Paquier <michael@paquier.xyz>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox