No reuse of hash_entry

[tserr]

During our research for the memory “leak“ issue (https://github.com/arkhipov/temporal_tables/issues/24) we found out, that „hash_entry“ will not be found in function “insert_history_row” in some cases, leading to refilling “hash_entry” on every call of the function.

This issue occurs on tables with e.g. defined constraints or default values. In function “insert_history_row” the check

                if (hash_entry->natts == -1 ||
                        RelationGetRelid(history_relation) != hash_entry->history_relid ||
                        !equalTupleDescs(tupdesc, hash_entry->tupdesc) ||
                        !equalTupleDescs(history_tupdesc, hash_entry->history_tupdesc))

always evaluates to true as equalTupleDescs(*tupdesc, hash_entry->tupdesc) and equalTupleDescs(history_tupdesc, hash_entry->history_tupdesc) evaluate to false. In our cases this was because in function equalTupleDesc the check if (attr1->attnotnull != attr2->attnotnull) evaluates to false as “attr2->attnotnull” is 0, even for columns with “NOT NULL“ set.

As far as we could find out the cause seem to be the way the tuple description is copied into the “hash_entry” in function “fill_versioning_hash_entry”: “CreateTupleDescCopy” does explicitly not copy default values and constraints. Using “CreateTupleDescCopyConstr” instead does. Using “CreateTupleDescCopy” instead fix this for us:

diff --git a/versioning.c b/versioning.c
index 2497c2f..41ffcdd 100644
--- a/versioning.c
+++ b/versioning.c
@@ -561,8 +561,8 @@ fill_versioning_hash_entry(VersioningHashEntry *hash_entry,
                oldcontext = MemoryContextSwitchTo(TopMemoryContext);

                hash_entry->history_relid = RelationGetRelid(history_relation);
-               hash_entry->tupdesc = CreateTupleDescCopy(tupdesc);
-               hash_entry->history_tupdesc = CreateTupleDescCopy(history_tupdesc);
+               hash_entry->tupdesc = CreateTupleDescCopyConstr(tupdesc);
+               hash_entry->history_tupdesc = CreateTupleDescCopyConstr(history_tupdesc);

                hash_entry->attnums = palloc(natts * sizeof(int));
                memcpy(hash_entry->attnums, attnums, natts * sizeof(int));

[arkhipov]

It should be working now. Thank you for the patch.