On Thu, Jan 25, 2024 at 04:00:01AM +0000, PG Bug reporting form wrote: > The following query: > CREATE ROLE u; > DROP ROLE u, u; > fails with > ERROR: tuple already updated by self
Pas glop.
> Whilst all the other DROP commands, accepting a list of objects (namely, > AGGREGATE, DOMAIN, EXTENSION, FOREIGN DATA WRAPPER, FOREIGN TABLE, > FUNCTION, INDEX, MATERIALIZED VIEW, OPERATOR, PROCEDURE, ROUTINE, > SEQUENCE, SERVER, STATISTICS, TABLE, TYPE, VIEW), handle such duplicates > with no error.
DROP ROLE has it own parsing node and its own dropping logic because of ACL checks and such, while the others used the unified route with dependency.c & friends.
For this job, I think user.c is just overengineered and could be simplified to use a list of OIDs. There is no need for ObjectAddress or even ObjectAddresses: all of them are created with AuthIdRelationId as class ID, and the second loop just fetches the role IDs retrieved from the ObjectAddresses created in the first loop. We don't use the individual ObjectAddress either. On top of all that, it makes the code a bit easier to follow.
Agreed. +1
> Also, the following ALTER query: > 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; > fails with the same > ERROR: tuple already updated by self > > Again, I re-checked all the other ALTER commands, that accept a list of > objects, and could not find other ones producing the same error.
This one comes down to the list of manipulation of the list for token names, where tsearchcmds.c has a few assumptions about the way to do its job across multiple catalog scans. We could adjust the code so as the list of String nodes is rebuilt with some list_append_unique(), but I cannot convince myself that this is worth the complication. -- Michael
