I've been looking into Gordon Shannon's crash report here: http://archives.postgresql.org/pgsql-general/2010-12/msg01030.php After some groveling around in the core dump (thanks to Gordon for making that available), I figured out the cause of the problem.
The missing piece of information that prevented anyone from reproducing the crash was that Gordon's master table, and many of its inheritance children, had a dropped column --- but the most-recently-added children did not. This meant that the child plans of the Update node didn't all have the same targetlists, which is expected. But then the resjunk columns added to track row identities for sharelocking didn't occur at the same column numbers in every subplan. The PlanRowMark representation built by the planner fails to account for this possibility, since it sends predetermined column numbers (ctidAttNo, toidAttNo, wholeAttNo) to the executor in the ModifyTable's rowMarks list. This would fail not only in the case of dropped columns, but any scenario where the child tables don't all have identical column sets. The problem only manifests when EvalPlanQual is run, though, and that requires concurrent updates to the same row in READ COMMITTED mode. (Which makes it impractical to test in the current regression test framework :-() I can see two basic ways of attacking this. Clearly we need to track the resjunk column numbers separately for each subplan. We could convert ModifyTable's rowMarks list into a list-of-lists of PlanRowMark, one per child plan. I'm thinking though that determining those column numbers at plan time might have been an overly cute idea. It might be better to remove the column numbers from PlanRowMark, restoring it to a form that's not dependent on the particular subplan. Then ModifyTable would need to build a list of ExecRowMark nodes for each child plan from the PlanRowMark representation, doing the lookup for resjunk column names during plan startup. This would take a few more cycles than the current way but it seems more robust. (Another possible objection is that we'd have to put back into the executor knowledge of the resjunk column naming conventions, something I'd been trying to decouple by adding these attno fields. But if it doesn't work, it doesn't work...) Another point that has to be considered is that currently the executor maintains a global list of ExecRowMarks (estate->es_rowMarks). In SELECT FOR UPDATE/SHARE that list is needed so that execCurrent.c (WHERE CURRENT OF) can pull out the current locked TID of a SELECT FOR UPDATE cursor. However, we don't support any such thing for ModifyTable. The other use of the global list is to keep track of which relations have been opened with a lock type higher than AccessShareLock. On the whole it seems that it might be best to split the data structure into two types of structs: * a global list carrying the same fields as current ExecRowMark except for the attno fields. * private lists in LockRows and ModifyTable nodes carrying attribute numbers plus pointers to the corresponding global-list entry. In ModifyTable we'd need such a list per child plan. Comments? regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers