Peter Geoghegan <pg@heroku.com> writes:
> I attached two revisions - one of my patch (btreelock_insert_on_dup)
> and one of your alternative design (exclusion_insert_on_dup).
I spent a little bit of time looking at btreelock_insert_on_dup. AFAICT
it executes FormIndexDatum() for later indexes while holding assorted
buffer locks in earlier indexes. That really ain't gonna do, because in
the case of an expression index, FormIndexDatum will execute nearly
arbitrary user-defined code, which might well result in accesses to those
indexes or others. What we'd have to do is refactor so that all the index
tuple values get computed before we start to insert any of them. That
doesn't seem impossible, but it implies a good deal more refactoring than
has been done here.
Once we do that, I wonder if we couldn't get rid of the LWLockWeaken/
Strengthen stuff. That scares the heck out of me; I think it's deadlock
problems waiting to happen.
Another issue is that the number of buffer locks being held doesn't seem
to be bounded by anything much. The current LWLock infrastructure has a
hard limit on how many lwlocks can be held per backend.
Also, the lack of any doc updates makes it hard to review this. I can
see that you don't want to touch the user-facing docs until the syntax
is agreed on, but at the very least you ought to produce real updates
for the indexam API spec, since you're changing that significantly.
BTW, so far as the syntax goes, I'm quite distressed by having to make
REJECTS into a fully-reserved word. It's not reserved according to the
standard, and it seems pretty likely to be something that apps might be
using as a table or column name.
regards, tom lane