2009/7/28 Tom Lane <tgl@sss.pgh.pa.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