On Mon, 6 Jul 2026 at 22:54, Zsolt Parragi <[email protected]> wrote:
>
> This is a great feature, I did some testing of it.
Thanks for testing!
> 1. If a table has an ON COMMIT clause:
>
> CREATE GLOBAL TEMPORARY TABLE gtt_del (id int) ON COMMIT DELETE ROWS
>
> Then pg_dump forgets that.
Fixed in v9.
> 2. Even an aborted transaction pins frozenxid:
>
> CREATE GLOBAL TEMPORARY TABLE gtt (id int);
>
> In one session
>
> BEGIN; SELECT count(*) FROM gtt; ABORT;
> -- and keep session alive
>
> Then in another session,
> SELECT datfrozenxid FROM pg_database WHERE datname=current_database();
> -- do some work
> VACUUM (FREEZE);
> SELECT datfrozenxid FROM pg_database WHERE datname=current_database();
> -- ...
>
> And repeat the above a few times for the second session, datfrozenxid
> will remain the same until the first session is closed.
Fixed in v9.
> 3. in AtEOSubXact_UsageCleanup and also in AtEOSubXact_StorageCleanup
>
> + else
> + gtr_remove_usage(entry->relid);
> + }
> +
> + /* Update the usage stopped subid */
> + if (entry->stopped_subid == mySubid)
> + {
> + if (isCommit)
> + entry->stopped_subid = parentSubid;
> + else
> + entry->stopped_subid = InvalidSubTransactionId;
> + }
>
> if we follow the else branch with gtr_remove_usage, shouldn't the
> function immediately return?
OK, fixed in v9. It wasn't actually broken, in that "entry" is still
valid memory after removing it from the hash table (it's on a
freelist), but I agree that it's neater to return immediately.
> 4. in AtEOSubXact_PendingInsertCleanup
>
> + else
> + hash_search(pending_inserts, &entry->relid,
> HASH_REMOVE, NULL);
> + }
> +
> + /* Update the inserted subid (may rollback flush) */
> + if (entry->inserted_subid == mySubid)
> + {
> + if (isCommit)
> + entry->inserted_subid = parentSubid;
> + else
> + {
> + entry->inserted_subid = InvalidSubTransactionId;
> + have_inserts_to_flush = true;
> + }
> + }
>
> same question, isn't the else branch missing a return?
As above, fixed in v9.
> 5. in gtr_remove_usage / gtr_remove_all_usage_on_exit
>
> + shared_entry = dshash_find(gtr_shared_usage, &key, true);
> +
> + if (shared_entry->usage_count > 1)
>
> and in gtr_record_usage
>
> + /* Flag the usage entry for eoxact cleanup */
> + EOXactUsageListAdd(relid);
> +
> + /* Add/update shared usage entry */
> + key.dbid = MyDatabaseId;
> + key.relid = relid;
> + shared_entry = dshash_find_or_insert(gtr_shared_usage, &key, &found);
>
> Can't this crash with a null pointer access in the above if
> dshash_find_or_insert fails with OOM?
No, because it would throw an error, so it will never try to access
"shared_entry" if it couldn't insert it. I checked against other code
using dshash_find_or_insert(), and it's the same.
Regards,
Dean
