Hello,
On 2016/07/25 17:18, Kyotaro HORIGUCHI wrote:
>
> - Remove ccvalid condition from equalTupleDescs() to reduce
> unnecessary cache invalidation or tuple rebuilding.
The following commit introduced the ccvalid check:
"""
commit c31305de5f5a4880b0ba2f5983025ef0210a3b2a
Author: Noah Misch <n...@leadboat.com>
Date: Sun Mar 23 02:13:43 2014 -0400
Address ccvalid/ccnoinherit in TupleDesc support functions.
equalTupleDescs() neglected both of these ConstrCheck fields, and
CreateTupleDescCopyConstr() neglected ccnoinherit. At this time, the
only known behavior defect resulting from these omissions is constraint
exclusion disregarding a CHECK constraint validated by an ALTER TABLE
VALIDATE CONSTRAINT statement issued earlier in the same transaction.
Back-patch to 9.2, where these fields were introduced.
"""
So, apparently RelationClearRelation() does intend to discard a cached
TupleDesc if ccvalid changed in a transaction. Whereas,
acquire_inherited_sample_rows() does not seem to depend on ccvalid being
equal or not (or any member of TupleConstr for that matter). So maybe,
instead of simply discarding the check (which does serve a purpose), we
could make equalTupleDescs() parameterized on whether we want TupleConstr
equality check to be performed or not. RelationClearRelation() can ask
for the check to be performed by passing true for the parameter whereas
acquire_inherited_sample_rows() and other callers can pass false. Perhaps
something like the attached.
Thanks,
Amit
diff --git a/src/backend/access/common/tupdesc.c b/src/backend/access/common/tupdesc.c
index b56d0e3..3dcb656 100644
--- a/src/backend/access/common/tupdesc.c
+++ b/src/backend/access/common/tupdesc.c
@@ -348,7 +348,7 @@ DecrTupleDescRefCount(TupleDesc tupdesc)
* We don't compare tdrefcount, either.
*/
bool
-equalTupleDescs(TupleDesc tupdesc1, TupleDesc tupdesc2)
+equalTupleDescs(TupleDesc tupdesc1, TupleDesc tupdesc2, bool compare_constr)
{
int i,
j,
@@ -411,6 +411,9 @@ equalTupleDescs(TupleDesc tupdesc1, TupleDesc tupdesc2)
/* attacl, attoptions and attfdwoptions are not even present... */
}
+ if (!compare_constr)
+ return true;
+
if (tupdesc1->constr != NULL)
{
TupleConstr *constr1 = tupdesc1->constr;
diff --git a/src/backend/catalog/pg_proc.c b/src/backend/catalog/pg_proc.c
index c1d1505..7ce2da5 100644
--- a/src/backend/catalog/pg_proc.c
+++ b/src/backend/catalog/pg_proc.c
@@ -434,7 +434,7 @@ ProcedureCreate(const char *procedureName,
if (olddesc == NULL && newdesc == NULL)
/* ok, both are runtime-defined RECORDs */ ;
else if (olddesc == NULL || newdesc == NULL ||
- !equalTupleDescs(olddesc, newdesc))
+ !equalTupleDescs(olddesc, newdesc, false))
ereport(ERROR,
(errcode(ERRCODE_INVALID_FUNCTION_DEFINITION),
errmsg("cannot change return type of existing function"),
diff --git a/src/backend/commands/analyze.c b/src/backend/commands/analyze.c
index 5fcedd7..f96333d 100644
--- a/src/backend/commands/analyze.c
+++ b/src/backend/commands/analyze.c
@@ -1417,7 +1417,8 @@ acquire_inherited_sample_rows(Relation onerel, int elevel,
/* We may need to convert from child's rowtype to parent's */
if (childrows > 0 &&
!equalTupleDescs(RelationGetDescr(childrel),
- RelationGetDescr(onerel)))
+ RelationGetDescr(onerel),
+ false))
{
TupleConversionMap *map;
diff --git a/src/backend/utils/cache/plancache.c b/src/backend/utils/cache/plancache.c
index f42a62d..d3b0d6c 100644
--- a/src/backend/utils/cache/plancache.c
+++ b/src/backend/utils/cache/plancache.c
@@ -711,7 +711,7 @@ RevalidateCachedQuery(CachedPlanSource *plansource)
/* OK, doesn't return tuples */
}
else if (resultDesc == NULL || plansource->resultDesc == NULL ||
- !equalTupleDescs(resultDesc, plansource->resultDesc))
+ !equalTupleDescs(resultDesc, plansource->resultDesc, false))
{
/* can we give a better error message? */
if (plansource->fixed_result)
diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c
index 8d2ad01..228f1f8 100644
--- a/src/backend/utils/cache/relcache.c
+++ b/src/backend/utils/cache/relcache.c
@@ -2240,7 +2240,7 @@ RelationClearRelation(Relation relation, bool rebuild)
elog(ERROR, "relation %u deleted while still in use", save_relid);
}
- keep_tupdesc = equalTupleDescs(relation->rd_att, newrel->rd_att);
+ keep_tupdesc = equalTupleDescs(relation->rd_att, newrel->rd_att, true);
keep_rules = equalRuleLocks(relation->rd_rules, newrel->rd_rules);
keep_policies = equalRSDesc(relation->rd_rsdesc, newrel->rd_rsdesc);
diff --git a/src/backend/utils/cache/typcache.c b/src/backend/utils/cache/typcache.c
index ea6f787..b4578f5 100644
--- a/src/backend/utils/cache/typcache.c
+++ b/src/backend/utils/cache/typcache.c
@@ -1341,7 +1341,7 @@ assign_record_type_typmod(TupleDesc tupDesc)
foreach(l, recentry->tupdescs)
{
entDesc = (TupleDesc) lfirst(l);
- if (equalTupleDescs(tupDesc, entDesc))
+ if (equalTupleDescs(tupDesc, entDesc, false))
{
tupDesc->tdtypmod = entDesc->tdtypmod;
return;
diff --git a/src/include/access/tupdesc.h b/src/include/access/tupdesc.h
index de18f74..28a85f4 100644
--- a/src/include/access/tupdesc.h
+++ b/src/include/access/tupdesc.h
@@ -110,7 +110,7 @@ extern void DecrTupleDescRefCount(TupleDesc tupdesc);
DecrTupleDescRefCount(tupdesc); \
} while (0)
-extern bool equalTupleDescs(TupleDesc tupdesc1, TupleDesc tupdesc2);
+extern bool equalTupleDescs(TupleDesc tupdesc1, TupleDesc tupdesc2, bool compare_constr);
extern void TupleDescInitEntry(TupleDesc desc,
AttrNumber attributeNumber,
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
