Обсуждение: BUG #18297: Error when adding a column to a parent table with complex inheritance

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.

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

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

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

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

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.com
--
Michael