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




-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to