On 2020-12-30 09:10, Bharath Rupireddy wrote:
> Hi,
>
> I'm posting a v4-0001 patch for the new functions
> postgres_fdw_get_connections() and postgres_fdw_disconnect(). In this
> patch, I tried to address the review comments provided upthread.
>
> Thoughts?
>
I still have some doubts that it is worth of allowing to call
postgres_fdw_disconnect() in the explicit transaction block, since it
adds a lot of things to care and check for. But otherwise current logic
looks solid.
+ errdetail("Such connections get closed either in the next use or
at the end of the current transaction.")
+ : errdetail("Such connection gets closed either in the next use or
at the end of the current transaction.")));
Does it really have a chance to get closed on the next use? If foreign
server is dropped then user mapping should be dropped as well (either
with CASCADE or manually), but we do need user mapping for a local cache
lookup. That way, if I understand all the discussion up-thread
correctly, we can only close such connections at the end of xact, do we?
+ * This function returns false if the cache doesn't exist.
+ * When the cache exists:
I think that this will be corrected later by pg_indent, but still. In
this comment section following points 1) and 2) have a different
combination of tabs/spaces.
Regards
--
Alexey Kondratov
Postgres Professional https://www.postgrespro.com
Russian Postgres Company