Re: WIP: Deferrable unique constraints

2009/7/28 Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>:
> ... speaking of adding catalog columns, I just discovered that the patch
> adds searches of pg_depend and pg_constraint to BuildIndexInfo.  This
> seems utterly unacceptable on two grounds:
>
> * It's sheer luck that it gets through bootstrap without crashing.
> Those aren't part of the core set of catalogs that we expect to be
> accessed by primitive catalog searches.  I wouldn't be too surprised
> if it gets the wrong answer, and the only reason there's not a visible
> bug is none of the core catalogs have deferrable indexes (so there's
> no pg_depend entry to be found).
>
> * It increases the runtime of BuildIndexInfo by what seems likely to
> be orders of magnitude (note that get_index_constraint is not a
> syscacheable operation).  Did anyone do any performance checks on
> this patch, like seeing if pgbench got slower?
>
> I think we had better add the deferrability state to pg_index
> to avoid this.
>
> I tried to see if we could dispense with storing the flags in IndexInfo
> at all, so as not to have to do that.  It looks to me like the only
> place where it's really needed is in ExecInsertIndexTuples:
>
>        if (is_vacuum || !relationDescs[i]->rd_index->indisunique)
>            uniqueCheck = UNIQUE_CHECK_NO;
> ==>     else if (indexInfo->ii_Deferrable)
>            uniqueCheck = UNIQUE_CHECK_PARTIAL;
>        else
>            uniqueCheck = UNIQUE_CHECK_YES;
>
> Since this code has its hands on the pg_index row already, it definitely
> doesn't need a copy in IndexInfo if the state is in pg_index.  However,
> I also notice that it doesn't particularly care about the deferrability
> state in the sense that the triggers use (ie, whether to check at end of
> statement or end of transaction).  What we want to know here is whether
> to force an old-school immediate uniqueness check in the index AM.  So
> I'm thinking that we only need one bool added to pg_index, not two.
>
> And I'm further thinking about intentionally calling it something other
> than "deferred", to emphasize that the semantics aren't quite like
> regular constraint deferral.  Maybe invert the sense and call it
> "immediate".  Comments?
>
>                        regards, tom lane
>

Yes that makes sense. Sorry I didn't spot this - it was a performance
regression, which I should have spotted with pgbench.

- Dean