We added these ConstrCheck fields for 9.2, but equalTupleDescs() did not get
the memo. I looked for resulting behavior problems, and I found one in
RelationClearRelation() only. Test case:
set constraint_exclusion = on;
drop table if exists ccvalid_test;
create table ccvalid_test (c int);
alter table ccvalid_test add constraint x check (c > 0) not valid;
begin;
-- constraint_exclusion won't use an invalid constraint.
explain (costs off) select * from ccvalid_test where c = 0;
-- Make it valid.
alter table ccvalid_test validate constraint x;
-- Local invalidation rebuilt the Relation and decided the TupleDesc hadn't
-- changed, so we're still not using the constraint.
explain (costs off) select * from ccvalid_test where c = 0;
commit;
-- At COMMIT, we destroyed the then-closed Relation in response to shared
-- invalidation. Now constraint_exclusion sees the valid constraint.
explain (costs off) select * from ccvalid_test where c = 0;
Currently, the damage is limited to later commands in the transaction that
issued ALTER TABLE VALIDATE. Changing ccvalid requires AccessExclusiveLock,
so no other backend will have an affected, open relcache entry to rebuild.
Shared invalidation will make the current backend destroy its affected
relcache entry before starting a new transaction. However, the impact will
not be so limited once we allow ALTER TABLE VALIDATE to run with a mere
ShareUpdateExclusiveLock. (I discovered this bug while reviewing the patch
implementing that very feature.)
I don't see a way to get trouble from the ccnoinherit omission. You can't
change ccnoinherit except by dropping and recreating the constraint, and each
of the drop and create operations would make equalTupleDescs() detect a
change. The same can be said of "ccbin", but equalTupleDescs() does compare
that field. For simplicity, I'll have it compare ccnoinherit.
CreateTupleDescCopyConstr() also skips ccnoinherit. I don't see a resulting
live bug, but it's worth correcting.
Given the minor symptoms in released versions, I lean against a back-patch.
Thanks,
nm
--
Noah Misch
EnterpriseDB http://www.enterprisedb.com
diff --git a/src/backend/access/common/tupdesc.c
b/src/backend/access/common/tupdesc.c
index db8cb82..74cfb64 100644
--- a/src/backend/access/common/tupdesc.c
+++ b/src/backend/access/common/tupdesc.c
@@ -204,6 +204,7 @@ CreateTupleDescCopyConstr(TupleDesc tupdesc)
if (constr->check[i].ccbin)
cpy->check[i].ccbin =
pstrdup(constr->check[i].ccbin);
cpy->check[i].ccvalid =
constr->check[i].ccvalid;
+ cpy->check[i].ccnoinherit =
constr->check[i].ccnoinherit;
}
}
@@ -458,7 +459,9 @@ equalTupleDescs(TupleDesc tupdesc1, TupleDesc tupdesc2)
for (j = 0; j < n; check2++, j++)
{
if (strcmp(check1->ccname, check2->ccname) == 0
&&
- strcmp(check1->ccbin, check2->ccbin) ==
0)
+ strcmp(check1->ccbin, check2->ccbin) ==
0 &&
+ check1->ccvalid == check2->ccvalid &&
+ check1->ccnoinherit ==
check2->ccnoinherit)
break;
}
if (j >= n)
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
