On Thu, Aug 4, 2016 at 8:14 AM, Aleksander Alekseev <a.aleks...@postgrespro.ru> wrote: > Thanks everyone for your remarks and comments! > > Here is an improved version of a patch. > > Main changes: > * Patch passes `make installcheck` > * Code is fully commented, also no more TODO's > > I wish I sent this version of a patch last time. Now I realize it was > really hard to read and understand. Hope I managed to correct this > flaw. If you believe that some parts of the code are still poorly > commented or could be improved in any other way please let me know. > > And as usual, any other comments, remarks or questions are highly > appreciated!
Three general comments: 1. It's always seemed to me that a huge problem with anything of this sort is dependencies. For example, suppose I create a fast temporary table and then I create a functional index on the fast temporary table that uses some SQL function defined in pg_proc. Then, another user drops the function. Then, I try to use the index. With regular temporary tables, dependencies protect us here, but if there are no catalog entries, I wonder how this can ever be made safe. Similar problems exist for triggers, constraints, RLS policies, and attribute defaults. 2. This inserts additional code in a bunch of really low-level places like heap_hot_search_buffer, heap_update, heap_delete, etc. I think what you've basically done here is create a new, in-memory heap AM and then, because we don't have an API for adding new storage managers, you've bolted it onto the existing heapam layer. That's certainly a reasonable approach for creating a PoC, but I think we actually need a real API here. Otherwise, when the next person comes along and wants to add a third heap implementation, they've got to modify all of these same places again. I don't think this code is reasonably maintainable in this form. 3. Currently, temporary tables are parallel-restricted: a query that references temporary tables can use parallelism, but access to the temporary tables is only possible from within the leader. I suspect, although I'm not entirely sure, that lifting this restriction would be easier with our current temporary table implementation than with this one, because the current temporary table implementation mostly relies on a set of buffers that could be moved from backend-private memory to DSM. On a quick look, this implementation uses a bunch of new data structures that are heavy on pointers, so that gets quite a bit more complicated to make parallel-safe (unless we adopt threads instead of processes!). -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
