No reuse of hash_entry
tserr opened this issue · 1 comments
During our research for the memory “leak“ issue (#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));
It should be working now. Thank you for the patch.