Hello,
On 2016/07/26 11:05, Kyotaro HORIGUCHI wrote:
> At Mon, 25 Jul 2016 18:21:27 +0900, Amit Langote wrote:
>
>> 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.
>
> Hmm. It should resolve the problem. But the two comparisons seem
> to be separate things. Constraints is not a part of tuple
> description. relcache seems to be making misusage of the equality
> of tuple descriptors.
>
> So, how about splitting the original equalTupleDescs into
> equalTupleDescs and equalTupleConstraints ?
Actually TupleConstr is *part* of the TupleDesc struct, which the
relcache.c tries to preserve in *whole* across a relcache flush, so it
seems perhaps cleaner to keep the decision centralized in one function:
keep_tupdesc = equalTupleDescs(relation->rd_att, newrel->rd_att); ... /* preserve old tupledesc and rules if no
logicalchange */ if (keep_tupdesc) SWAPFIELD(TupleDesc, rd_att);
Also:
/* * This struct is passed around within the backend to describe the * structure of tuples. For tuples coming from
on-diskrelations, the * information is collected from the pg_attribute, pg_attrdef, and * pg_constraint catalogs.
...typedef struct tupleDesc {
It would perhaps have been clear if there was a rd_constr field that
relcache.c independently managed.
OTOH, I tend to agree that other places don't quite need the whole thing
(given also that most other callers except acquire_inherit_sample_rows
compare transient row types)
Thanks,
Amit