Gurjeet Singh <singh.gurjeet@gmail.com> writes:
> On Sat, Jun 16, 2012 at 9:50 AM, Dean Rasheed <dean.a.rasheed@gmail.com>wrote:
> I find it interesting that 'actual time' for top level 'Update on fk_table'
> is always higher in patched versions, and yet the 'Total runtime' is lower
> for the patched versions. I would've expected 'Total runtime' to be
> proportional to the increase in top-level row-source's 'actual time'.
> Even the time consumed by Seq scans is higher in patched version, so I
> think the patch's affect on performance needs to be evaluated.
AFAICS, the only way that the given patch could possibly make anything
slower is that if the old value of some tested attribute is NULL, the
comparison routines used to fall out immediately; now, they will do an
additional SPI_getbinval call to extract the new value before making
any decision. So that would account for some small increase in the
ModifyTable runtime in cases where there are a lot of null keys in FK
rows being updated, which accurately describes Dean's test case, if not
so much the real world. I don't have a big problem with it, since the
point of the patch is to possibly save a great deal more work in exactly
these cases.
It strikes me though that we are still leaving some money on the table.
The SQL spec says clearly that no RI action need be taken when a null
PK key value is updated to non-null, and I think this is right because
there cannot possibly be any FK rows that are considered to match the
old value. (Note that both the spec and our FK code treat the RI
equality operators as strict, even if the underlying functions aren't
really.) So we ought to have asymmetric logic in there when making
checks on PK rows, such that null->non-null is not considered an
interesting change. If done properly this would remove the above-
described slowdown in the PK case.
Conversely, if an FK value is changed from non-null to null, that is
either always OK (if MATCH SIMPLE, or if MATCH FULL and all the FK
columns went to null) or a certain failure (if MATCH FULL and we
have a mix of nulls and non-nulls). There's no need to queue a
trigger event in the "always OK" cases, so I think we need some
asymmetric logic in the FK case as well.
regards, tom lane