From 6eb71ac5527a94be443bc66e68b47b04979906e4 Mon Sep 17 00:00:00 2001
From: Robert Haas <rhaas@postgresql.org>
Date: Wed, 25 Jan 2012 15:28:07 -0500
Subject: [PATCH] Make CheckIndexCompatible simpler and more bullet-proof.
This gives up the "don't rewrite the index" behavior in a couple of
relatively unimportant cases, such as changing between an array type
and an unconstrained domain over that array type, in return for
making this code more future-proof.
Noah Misch
---
src/backend/commands/indexcmds.c | 105 +++++++++++-----------
src/test/regress/expected/alter_table.out | 9 ++
src/test/regress/sql/alter_table.sql | 8 ++
3 files changed, 67 insertions(+), 55 deletions(-)
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index 712b0b0eba8..1bf1de56f31 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -23,6 +23,7 @@
#include "catalog/pg_opclass.h"
#include "catalog/pg_opfamily.h"
#include "catalog/pg_tablespace.h"
+#include "catalog/pg_type.h"
#include "commands/dbcommands.h"
#include "commands/defrem.h"
#include "commands/tablecmds.h"
@@ -52,6 +53,7 @@
/* non-export function prototypes */
static void CheckPredicate(Expr *predicate);
static void ComputeIndexAttrs(IndexInfo *indexInfo,
+ Oid *typeOidP,
Oid *collationOidP,
Oid *classOidP,
int16 *colOptionP,
@@ -87,18 +89,17 @@ static void RangeVarCallbackForReindexIndex(const RangeVar *relation,
* of columns and that if one has an expression column or predicate, both do.
* Errors arising from the attribute list still apply.
*
- * Most column type changes that can skip a table rewrite will not invalidate
- * indexes. For btree and hash indexes, we assume continued validity when
- * each column of an index would have the same operator family before and
- * after the change. Since we do not document a contract for GIN or GiST
- * operator families, we require an exact operator class match for them and
- * for any other access methods.
- *
- * DefineIndex always verifies that each exclusion operator shares an operator
- * family with its corresponding index operator class. For access methods
- * having no operator family contract, confirm that the old and new indexes
- * use the exact same exclusion operator. For btree and hash, there's nothing
- * more to check.
+ * Most column type changes that can skip a table rewrite do not invalidate
+ * indexes. We ackowledge this when all operator classes, collations and
+ * exclusion operators match. Though we could further permit intra-opfamily
+ * changes for btree and hash indexes, that adds subtle complexity with no
+ * concrete benefit for core types.
+
+ * When a comparison or exclusion operator has a polymorphic input type, the
+ * actual input types must also match. This defends against the possibility
+ * that operators could vary behavior in response to get_fn_expr_argtype().
+ * At present, this hazard is theoretical: check_exclusion_constraint() and
+ * all core index access methods decline to set fn_expr for such calls.
*
* We do not yet implement a test to verify compatibility of expression
* columns or predicates, so assume any such index is incompatible.
@@ -111,6 +112,7 @@ CheckIndexCompatible(Oid oldId,
List *exclusionOpNames)
{
bool isconstraint;
+ Oid *typeObjectId;
Oid *collationObjectId;
Oid *classObjectId;
Oid accessMethodId;
@@ -123,10 +125,10 @@ CheckIndexCompatible(Oid oldId,
int numberOfAttributes;
int old_natts;
bool isnull;
- bool family_am;
bool ret = true;
oidvector *old_indclass;
oidvector *old_indcollation;
+ Relation irel;
int i;
Datum d;
@@ -168,10 +170,12 @@ CheckIndexCompatible(Oid oldId,
indexInfo->ii_ExclusionOps = NULL;
indexInfo->ii_ExclusionProcs = NULL;
indexInfo->ii_ExclusionStrats = NULL;
+ typeObjectId = (Oid *) palloc(numberOfAttributes * sizeof(Oid));
collationObjectId = (Oid *) palloc(numberOfAttributes * sizeof(Oid));
classObjectId = (Oid *) palloc(numberOfAttributes * sizeof(Oid));
coloptions = (int16 *) palloc(numberOfAttributes * sizeof(int16));
- ComputeIndexAttrs(indexInfo, collationObjectId, classObjectId,
+ ComputeIndexAttrs(indexInfo,
+ typeObjectId, collationObjectId, classObjectId,
coloptions, attributeList,
exclusionOpNames, relationId,
accessMethodName, accessMethodId,
@@ -191,12 +195,7 @@ CheckIndexCompatible(Oid oldId,
return false;
}
- /*
- * If the old and new operator class of any index column differ in
- * operator family or collation, regard the old index as incompatible.
- * For access methods other than btree and hash, a family match has no
- * defined meaning; require an exact operator class match.
- */
+ /* Any change in operator class or collation breaks compatibility. */
old_natts = ((Form_pg_index) GETSTRUCT(tuple))->indnatts;
Assert(old_natts == numberOfAttributes);
@@ -208,52 +207,42 @@ CheckIndexCompatible(Oid oldId,
Assert(!isnull);
old_indclass = (oidvector *) DatumGetPointer(d);
- family_am = accessMethodId == BTREE_AM_OID || accessMethodId == HASH_AM_OID;
-
- for (i = 0; i < old_natts; i++)
- {
- Oid old_class = old_indclass->values[i];
- Oid new_class = classObjectId[i];
-
- if (!(old_indcollation->values[i] == collationObjectId[i]
- && (old_class == new_class
- || (family_am && (get_opclass_family(old_class)
- == get_opclass_family(new_class))))))
- {
- ret = false;
- break;
- }
- }
+ ret = (memcmp(old_indclass->values, classObjectId,
+ old_natts * sizeof(Oid)) == 0 &&
+ memcmp(old_indcollation->values, collationObjectId,
+ old_natts * sizeof(Oid)) == 0);
ReleaseSysCache(tuple);
- /*
- * For btree and hash, exclusion operators need only fall in the same
- * operator family; ComputeIndexAttrs already verified that much. If we
- * get this far, we know that the index operator family has not changed,
- * and we're done. For other access methods, require exact matches for
- * all exclusion operators.
- */
- if (ret && !family_am && indexInfo->ii_ExclusionOps != NULL)
+ /* For polymorphic opcintype, column type changes break compatibility. */
+ irel = index_open(oldId, AccessShareLock); /* caller probably has a lock */
+ for (i = 0; i < old_natts && ret; i++)
+ ret = (!IsPolymorphicType(get_opclass_input_type(classObjectId[i])) ||
+ irel->rd_att->attrs[i]->atttypid == typeObjectId[i]);
+
+ /* Any change in exclusion operator selections breaks compatibility. */
+ if (ret && indexInfo->ii_ExclusionOps != NULL)
{
- Relation irel;
Oid *old_operators, *old_procs;
uint16 *old_strats;
- /* Caller probably already holds a stronger lock. */
- irel = index_open(oldId, AccessShareLock);
RelationGetExclusionInfo(irel, &old_operators, &old_procs, &old_strats);
+ ret = memcmp(old_operators, indexInfo->ii_ExclusionOps,
+ old_natts * sizeof(Oid)) == 0;
- for (i = 0; i < old_natts; i++)
- if (old_operators[i] != indexInfo->ii_ExclusionOps[i])
- {
- ret = false;
- break;
- }
+ /* Require an exact input type match for polymorphic operators. */
+ for (i = 0; i < old_natts && ret; i++)
+ {
+ Oid left,
+ right;
- index_close(irel, NoLock);
+ op_input_types(indexInfo->ii_ExclusionOps[i], &left, &right);
+ ret = (!(IsPolymorphicType(left) || IsPolymorphicType(right)) ||
+ irel->rd_att->attrs[i]->atttypid == typeObjectId[i]);
+ }
}
+ index_close(irel, NoLock);
return ret;
}
@@ -315,6 +304,7 @@ DefineIndex(RangeVar *heapRelation,
bool quiet,
bool concurrent)
{
+ Oid *typeObjectId;
Oid *collationObjectId;
Oid *classObjectId;
Oid accessMethodId;
@@ -550,10 +540,12 @@ DefineIndex(RangeVar *heapRelation,
indexInfo->ii_Concurrent = concurrent;
indexInfo->ii_BrokenHotChain = false;
+ typeObjectId = (Oid *) palloc(numberOfAttributes * sizeof(Oid));
collationObjectId = (Oid *) palloc(numberOfAttributes * sizeof(Oid));
classObjectId = (Oid *) palloc(numberOfAttributes * sizeof(Oid));
coloptions = (int16 *) palloc(numberOfAttributes * sizeof(int16));
- ComputeIndexAttrs(indexInfo, collationObjectId, classObjectId,
+ ComputeIndexAttrs(indexInfo,
+ typeObjectId, collationObjectId, classObjectId,
coloptions, attributeList,
exclusionOpNames, relationId,
accessMethodName, accessMethodId,
@@ -980,6 +972,7 @@ CheckPredicate(Expr *predicate)
*/
static void
ComputeIndexAttrs(IndexInfo *indexInfo,
+ Oid *typeOidP,
Oid *collationOidP,
Oid *classOidP,
int16 *colOptionP,
@@ -1108,6 +1101,8 @@ ComputeIndexAttrs(IndexInfo *indexInfo,
}
}
+ typeOidP[attn] = atttype;
+
/*
* Apply collation override if any
*/
diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out
index e9925495042..41bfa857ddd 100644
--- a/src/test/regress/expected/alter_table.out
+++ b/src/test/regress/expected/alter_table.out
@@ -1665,6 +1665,15 @@ where oid = 'test_storage'::regclass;
t
(1 row)
+-- SET DATA TYPE without a rewrite
+CREATE DOMAIN other_textarr AS text[];
+CREATE TABLE norewrite_array(c text[] PRIMARY KEY);
+NOTICE: CREATE TABLE / PRIMARY KEY will create implicit index "norewrite_array_pkey" for table "norewrite_array"
+SET client_min_messages = debug1;
+ALTER TABLE norewrite_array ALTER c TYPE text[]; -- no work
+ALTER TABLE norewrite_array ALTER c TYPE other_textarr; -- rebuild index
+DEBUG: building index "norewrite_array_pkey" on table "norewrite_array"
+RESET client_min_messages;
--
-- lock levels
--
diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql
index d9bf08d6d50..494c1504d78 100644
--- a/src/test/regress/sql/alter_table.sql
+++ b/src/test/regress/sql/alter_table.sql
@@ -1197,6 +1197,14 @@ select reltoastrelid <> 0 as has_toast_table
from pg_class
where oid = 'test_storage'::regclass;
+-- SET DATA TYPE without a rewrite
+CREATE DOMAIN other_textarr AS text[];
+CREATE TABLE norewrite_array(c text[] PRIMARY KEY);
+SET client_min_messages = debug1;
+ALTER TABLE norewrite_array ALTER c TYPE text[]; -- no work
+ALTER TABLE norewrite_array ALTER c TYPE other_textarr; -- rebuild index
+RESET client_min_messages;
+
--
-- lock levels
--
--
GitLab