Re: patch for check constraints using multiple inheritance
От | Robert Haas |
---|---|
Тема | Re: patch for check constraints using multiple inheritance |
Дата | |
Msg-id | AANLkTi=Gax=kTFFG2us3xhnD=oBURF_T9U1GSme=6faq@mail.gmail.com обсуждение исходный текст |
Ответ на | Re: patch for check constraints using multiple inheritance (Tom Lane <tgl@sss.pgh.pa.us>) |
Ответы |
Re: patch for check constraints using multiple inheritance
(Robert Haas <robertmhaas@gmail.com>)
|
Список | pgsql-hackers |
On Fri, Jul 30, 2010 at 10:46 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Robert Haas <robertmhaas@gmail.com> writes: >> On Fri, Jul 30, 2010 at 10:23 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote: >>> Uh, full stop there. If you think that the multiply-inherited column >>> logic is wrong, it's you that is mistaken --- or at least you're going >>> to have to do a lot more than just assert that you don't like it. >>> We spent a *lot* of time hashing that behavior out, back around 7.3. > >> Since the output in the previous email apparently wasn't sufficient >> for you to understand what the problem is, here it is in more detail. >> ... >> Adding a column to the toplevel parent of the inheritance hierarchy >> and then dropping it again shouldn't leave a leftover copy of the >> column in the grandchild. > > Actually, it probably should. The inheritance rules were intentionally > designed to avoid dropping inherited columns that could conceivably > still contain valuable data. There isn't enough information in the > inhcount/islocal data structure to recognize that a multiply-inherited > column is ultimately derived from only one distant ancestor, but it was > agreed that an exact tracking scheme would be more complication than it > was worth. Instead, the mechanism is designed to ensure that no column > will be dropped if it conceivably is still wanted --- not that columns > might not be left behind and require another drop step. While I certainly agree that accidentally dropping a column that should be retained is a lot worse than accidentally retaining a column that should be dropped, that doesn't make the current behavior correct. I don't think that's really an arguable point. Another drop step doesn't help: the leftover column is undroppable short of nuking the whole table. So let's focus on what the actual problem is here, what is required to fix it, and whether or not and how far the fix can be back-patched. There are two separate bugs here, so we should consider them individually. In each case, the problem is that with a certain (admittedly rather strange) inheritance hierarchy, adding a column to the top-level parent results in the inheritance count in the bottom-most child ending up as 3 rather than 2. 2 is the correct value because the bottom-most child has 2 immediate parents. The consequence of ending up with a count of 3 rather than 2 is that if the constraint/column added at the top-level is subsequent dropped, necessarily also from the top-level, the bottom-level child ends up with the constraint/column still in existence and with an inheritance count of 1. This isn't the correct behavior, and furthermore the child object can't be dropped, because it still believes that it's inherited (even though its parents no longer have a copy to inherit). In the case of coninhcount, I believe that the fix actually is quite simple, although of course it's possible that I'm missing something. Currently, ATAddConstraint() first calls AddRelationNewConstraints() to add the constraint, or possibly merge it into an existing, inherited constraint; it then adds the constraint to the phase-3 work queue so that it will be checked; and finally it recurses to its own children. It seems to me that it should only recurse to its children if the constraint was really added, rather than merged into an existing constraint, because if it was merged into an existing constraint, then the children already have it. I'm still trying to figure out why this bug doesn't show up in simpler cases, but I think that's the root of the issue. attinhcount exhibits analogous misbehavior, but if you're saying the fix for that case is going to be a lot more complicated, I think I agree with you. In that case, it seems that we traverse the inheritance hierarchy during the prep phase, at which point we don't yet know whether we're going to end up merging anything. It might be that a fix for this part of the problem ends up being too complex to back-patch. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise Postgres Company
В списке pgsql-hackers по дате отправления:
Предыдущее
От: Yeb HavingaДата:
Сообщение: Re: patch for check constraints using multiple inheritance