On Sat, Jan 18, 2014 at 5:28 AM, Robert Haas <robertmh...@gmail.com> wrote:
>> I was wondering if that might cause deadlocks if an existing index is
>> changed from unique to non-unique, or vice versa, as the ordering would
>> change. But we don't have a DDL command to change that, so the question is
>> moot.
>
> It's not hard to imagine someone wanting to add such a DDL command.
Perhaps, but the burden of solving that problem ought to rest with
whoever eventually propose the command. Certainly, if someone did so
today, I would object on the grounds that their patch precluded us
from ever prioritizing unique indexes, to get them out of the way
during insertion, so I am not actually making such an effort more
difficult than it already is. Moreover, avoiding entirely predictable
index bloat is more important than making supporting this yet to be
proposed feature's implementation easier. I was surprised when I
learned that things didn't already work this way.
Attached patch, broken off from my patch has relcache sort indexes by
(!indisunique, relindexid).
--
Peter Geoghegan
*** a/src/backend/utils/cache/relcache.c
--- b/src/backend/utils/cache/relcache.c
*************** typedef struct relidcacheent
*** 108,113 ****
--- 108,125 ----
Relation reldesc;
} RelIdCacheEnt;
+ /*
+ * Representation of indexes for sorting purposes
+ *
+ * We use this to sort indexes globally by a specific sort order, per
+ * RelationGetIndexList().
+ */
+ typedef struct relidunq
+ {
+ bool indisunique;
+ Oid relindexid;
+ } relidunq;
+
static HTAB *RelationIdCache;
/*
*************** static TupleDesc GetPgClassDescriptor(vo
*** 246,252 ****
static TupleDesc GetPgIndexDescriptor(void);
static void AttrDefaultFetch(Relation relation);
static void CheckConstraintFetch(Relation relation);
! static List *insert_ordered_oid(List *list, Oid datum);
static void IndexSupportInitialize(oidvector *indclass,
RegProcedure *indexSupport,
Oid *opFamily,
--- 258,264 ----
static TupleDesc GetPgIndexDescriptor(void);
static void AttrDefaultFetch(Relation relation);
static void CheckConstraintFetch(Relation relation);
! static int relidunq_cmp(const void *a, const void *b);
static void IndexSupportInitialize(oidvector *indclass,
RegProcedure *indexSupport,
Oid *opFamily,
*************** CheckConstraintFetch(Relation relation)
*** 3445,3455 ****
* Such indexes are expected to be dropped momentarily, and should not be
* touched at all by any caller of this function.
*
! * The returned list is guaranteed to be sorted in order by OID. This is
! * needed by the executor, since for index types that we obtain exclusive
! * locks on when updating the index, all backends must lock the indexes in
! * the same order or we will get deadlocks (see ExecOpenIndices()). Any
! * consistent ordering would do, but ordering by OID is easy.
*
* Since shared cache inval causes the relcache's copy of the list to go away,
* we return a copy of the list palloc'd in the caller's context. The caller
--- 3457,3471 ----
* Such indexes are expected to be dropped momentarily, and should not be
* touched at all by any caller of this function.
*
! * The returned list is guaranteed to be in (!indisunique, OID) order. This is
! * needed by the executor, since for index types that we obtain exclusive locks
! * on when updating the index, all backends must lock the indexes in the same
! * order or we will get deadlocks (see ExecOpenIndices()). For most purposes
! * any consistent ordering would do, but there is further consideration, which
! * is why we put unique indexes first: it is generally useful to get insertion
! * into unique indexes out of the way, since unique violations are the cause of
! * many aborted transactions. We can always avoid bloating non-unique indexes
! * of the same slot.
*
* Since shared cache inval causes the relcache's copy of the list to go away,
* we return a copy of the list palloc'd in the caller's context. The caller
*************** RelationGetIndexList(Relation relation)
*** 3469,3475 ****
SysScanDesc indscan;
ScanKeyData skey;
HeapTuple htup;
! List *result;
char replident = relation->rd_rel->relreplident;
Oid oidIndex = InvalidOid;
Oid pkeyIndex = InvalidOid;
--- 3485,3495 ----
SysScanDesc indscan;
ScanKeyData skey;
HeapTuple htup;
! relidunq *indexTypes;
! int nIndexType;
! int i;
! Size szIndexTypes;
! List *result = NIL;
char replident = relation->rd_rel->relreplident;
Oid oidIndex = InvalidOid;
Oid pkeyIndex = InvalidOid;
*************** RelationGetIndexList(Relation relation)
*** 3486,3494 ****
* list into the relcache entry. This avoids cache-context memory leakage
* if we get some sort of error partway through.
*/
- result = NIL;
oidIndex = InvalidOid;
/* Prepare to scan pg_index for entries having indrelid = this rel. */
ScanKeyInit(&skey,
Anum_pg_index_indrelid,
--- 3506,3517 ----
* list into the relcache entry. This avoids cache-context memory leakage
* if we get some sort of error partway through.
*/
oidIndex = InvalidOid;
+ nIndexType = 0;
+ szIndexTypes = 5;
+ indexTypes = palloc(sizeof(relidunq) * szIndexTypes);
+
/* Prepare to scan pg_index for entries having indrelid = this rel. */
ScanKeyInit(&skey,
Anum_pg_index_indrelid,
*************** RelationGetIndexList(Relation relation)
*** 3504,3509 ****
--- 3527,3533 ----
Form_pg_index index = (Form_pg_index) GETSTRUCT(htup);
Datum indclassDatum;
oidvector *indclass;
+ relidunq indexType;
bool isnull;
/*
*************** RelationGetIndexList(Relation relation)
*** 3515,3522 ****
if (!IndexIsLive(index))
continue;
! /* Add index's OID to result list in the proper order */
! result = insert_ordered_oid(result, index->indexrelid);
/*
* indclass cannot be referenced directly through the C struct,
--- 3539,3555 ----
if (!IndexIsLive(index))
continue;
! /* For sort ordering */
! indexType.indisunique = index->indisunique;
! indexType.relindexid = index->indexrelid;
!
! /* Append to array of index oids */
! if (nIndexType >= szIndexTypes)
! {
! szIndexTypes = Max(nIndexType + 1, szIndexTypes * 2);
! indexTypes = repalloc(indexTypes, sizeof(relidunq) * szIndexTypes);
! }
! indexTypes[nIndexType++] = indexType;
/*
* indclass cannot be referenced directly through the C struct,
*************** RelationGetIndexList(Relation relation)
*** 3571,3576 ****
--- 3604,3618 ----
heap_close(indrel, AccessShareLock);
+ /* sort returned values consistently */
+ qsort(indexTypes, nIndexType, sizeof(relidunq), relidunq_cmp);
+ /* caller expects simple list of Oids */
+ for (i = 0; i < nIndexType; i++)
+ result = lappend_oid(result, indexTypes[i].relindexid);
+
+ /* free temp memory */
+ pfree(indexTypes);
+
/* Now save a copy of the completed list in the relcache entry. */
oldcxt = MemoryContextSwitchTo(CacheMemoryContext);
relation->rd_indexlist = list_copy(result);
*************** RelationGetIndexList(Relation relation)
*** 3582,3617 ****
}
/*
! * insert_ordered_oid
! * Insert a new Oid into a sorted list of Oids, preserving ordering
*
! * Building the ordered list this way is O(N^2), but with a pretty small
! * constant, so for the number of entries we expect it will probably be
! * faster than trying to apply qsort(). Most tables don't have very many
! * indexes...
*/
! static List *
! insert_ordered_oid(List *list, Oid datum)
{
! ListCell *prev;
! /* Does the datum belong at the front? */
! if (list == NIL || datum < linitial_oid(list))
! return lcons_oid(datum, list);
! /* No, so find the entry it belongs after */
! prev = list_head(list);
! for (;;)
! {
! ListCell *curr = lnext(prev);
! if (curr == NULL || datum < lfirst_oid(curr))
! break; /* it belongs after 'prev', before 'curr' */
! prev = curr;
! }
! /* Insert datum into list after 'prev' */
! lappend_cell_oid(list, prev, datum);
! return list;
}
/*
--- 3624,3652 ----
}
/*
! * relidunq_cmp
! * Qsort style comparator for index relations from relcache
*
! * The comparator sorts by (!indisunique, OID).
*/
! static int
! relidunq_cmp(const void *a, const void *b)
{
! relidunq l_idx = (*(relidunq *) a);
! relidunq r_idx = (*(relidunq *) b);
! /* First sort by !indisunique */
! if (l_idx.indisunique < r_idx.indisunique)
! return +1;
! else if (l_idx.indisunique > r_idx.indisunique)
! return -1;
! if (l_idx.relindexid < r_idx.relindexid)
! return -1;
! else if (l_idx.relindexid > r_idx.relindexid)
! return +1;
! return 0;
}
/*
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
