| From: | Dean Rasheed <dean(dot)a(dot)rasheed(at)gmail(dot)com> |
|---|---|
| To: | Zsolt Parragi <zsolt(dot)parragi(at)percona(dot)com> |
| Cc: | pgsql-hackers(at)lists(dot)postgresql(dot)org |
| Subject: | Re: Global temporary tables |
| Date: | 2026-07-13 09:16:58 |
| Message-ID: | CAEZATCUsecTaKTzzPH_rAnfAQmhA_9MtaWXXSYq626j2_nX=sw@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Mon, 6 Jul 2026 at 22:54, Zsolt Parragi <zsolt(dot)parragi(at)percona(dot)com> 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
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Anthonin Bonnefoy | 2026-07-13 09:17:44 | Re: Don't use the deprecated and insecure PQcancel in our frontend tools anymore |
| Previous Message | Dean Rasheed | 2026-07-13 09:15:15 | Re: Global temporary tables |