Re: BUG #18500: Detaching a partition with an index manually attached to the parent's index triggers Assert - Mailing list pgsql-bugs
From | Alvaro Herrera |
---|---|
Subject | Re: BUG #18500: Detaching a partition with an index manually attached to the parent's index triggers Assert |
Date | |
Msg-id | 202407031821.2grjgjjcqovc@alvherre.pgsql Whole thread Raw |
In response to | Re: BUG #18500: Detaching a partition with an index manually attached to the parent's index triggers Assert (Alvaro Herrera <alvherre@alvh.no-ip.org>) |
Responses |
Re: BUG #18500: Detaching a partition with an index manually attached to the parent's index triggers Assert
Re: BUG #18500: Detaching a partition with an index manually attached to the parent's index triggers Assert |
List | pgsql-bugs |
Did anybody figure out a way to repair the problematic situation without having to recreate the whole index hierarchy? If so, please let me know. For full context, see below. On 2024-Jun-28, Alvaro Herrera wrote: > Here's a proposed patch for master only. It turns all three situations > being reported into ereport(ERROR); in one case I have an XXX comment, > because we have an alternative when attaching a partition that already > has a PK to a partitioned table that has a non-PK index: just create a > separate index in the partition. But that would cause slowness, which > is probably undesirable. I'm inclined to just remove the XXX comment, > but if anyone has other thoughts, they are welcome. > > I add an errhint() to one of these new errors, but I'm undecided about > that idea --- should we add errhint() to all three messages, or none? > > We can't do this in back branches, of course (incl. 17 at this point), > because that might break existing applications, which means we need to > adjust behavior in some other way, probably similar to what Tender Wang > suggested, namely just don't crash on detach, or something like that. So here are three patches. 0001 adds a check at DETACH time so that if a constraint has the form that we don't want to see, it copes without crashing in the way Tender suggested. The code is different though, mainly because we don't need his proposed has_superclass() function. It also adds the test cases, and leaves one of the tables from it so that pg_upgrade is tested. 0002 is the code that throws an error in any of the three problem cases; roughly the same as the patch I posted as 0001 upthread. This is incomplete in the sense that the regression tests would fail with it, because I didn't modify the results. (I think we'd also remove the DETACH line from those tests, because it'd be useless). 0003 adds the pg_upgrade check I mentioned: it scans the databases in the source cluster for any constraints of the bogus form and fails the upgrade if any exist. I tested this one very lightly (that is to say, only with the database state left by the tests in 0001). After staring at this third patch for a little bit, I'm not so sure about rejecting the situation after all. The problem is that it is quite tricky to get rid of the problem objects: you may have to accept a length of time during which your partitioned table has no unique index and the partitions have no primary keys, and you need to have a lot of time and effort to recreate essentially those very same things, just to get the PK created at the partitioned-table level instead of at the partitions level. From a user's point of view this sounds rather insane because of how inconvenient it is. If we did have `ALTER TABLE partitioned ADD PRIMARY KEY USING INDEX`, then it wouldn't be a problem, because you can repair the situation cheaply and easily. So maybe a better plan is to retain the lenient behavior for now (that is, push patch 0001 only), then add ALTER TABLE ADD PRIMARY KEY USING INDEX (to 18 only of course), and only _then_ get patches 0002 and 0003 pushed (to 18 only). If somebody does see a cheap way to repair an existing database without having to recreate the indexes, then my opinion would switch 180 degrees -- so please speak up if you do. BTW I didn't actually try the cases where the constraint is UNIQUE rather than PRIMARY KEY (because that only occurred to me while I was writing this email), but I suspect it should be more or less the same, if not exactly identical. Also, I should disclaim that I didn't try these scenarios on a server compiled without assertions. I'll give them a run tomorrow, in case there are further problems hidden behind the assertion failures. -- Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/
pgsql-bugs by date: