... 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
