Re: Constraint merge and not valid status

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 logical change */
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-disk relations, 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