Hi, Tom Lane <t...@sss.pgh.pa.us> writes: > I'm afraid that this patch is likely to be, if not completely broken, > at least very much less useful than one could wish by the time we get > done closing the holes discussed in this other thread: > > https://www.postgresql.org/message-id/flat/5d910e2e-0db8-ec06-dd5f-baec420513c3%40imap.cc
Thanks for the review here Tom, and for linking with the other discussion (Álvaro did that too, thanks!). I've been reviewing it too. I didn't think about the pg_temp_NN namespaces in my approach, and I think it might be possible to make it work, but it's getting quite involved now. One idea would be that if every temp table in the session belongs to the transaction, and their namespace too (we could check through pg_depend that the namespace doesn't contain anything else beside the transaction's tables), then we could dispose of the temp schema and on-commit-drop tables at PREPARE COMMIT time. Otherwise, as before, prevent the transaction to be a 2PC one. > For instance, if we're going to have to reject the case where the > session's temporary schema was created during the current transaction, > then that puts a very weird constraint on whether this case works. Yeah. The goal of my approach is to transparently get back temp table support in 2PC when that makes sense, which is most use cases I've been confronted to. We use 2PC in Citus, and it would be nice to be able to use transaction local temp tables on worker nodes when doing data injestion and roll-ups. > Also, even without worrying about new problems that that discussion > may lead to, I don't think that the patch works as-is. The function > every_on_commit_is_on_commit_drop() does what it says, but that is > NOT sufficient to conclude that every temp table the transaction has > touched is on-commit-drop. This logic will successfully reject cases > with on-commit-delete-rows temp tables, but not cases where the temp > table(s) lack any ON COMMIT spec at all. Thanks! I missed that the lack of ON COMMIT spec would have that impact in the code. We could add tracking of that I suppose, and will have a look at how to implement it provided that the other points find an acceptable solution. Regards, -- dim
