Hi, Bharath:
Happy new year.
+ appendStringInfo(&buf, "(%s, %s)", server->servername,
+ entry->invalidated ? "false" : "true");
Is it better to use 'invalidated' than 'false' in the string ?
For the first if block of postgres_fdw_disconnect():
+ * Check if the connection associated with the given foreign server
is
+ * in use i.e. entry->xact_depth > 0. Since we can not close it, so
+ * error out.
+ */
+ if (is_in_use)
+ ereport(WARNING,
since is_in_use is only set in the if (server) block, I think the above
warning can be moved into that block.
Cheers
On Fri, Jan 1, 2021 at 2:04 AM Bharath Rupireddy <
bharath.rupireddyforpostg...@gmail.com> wrote:
> On Thu, Dec 31, 2020 at 8:29 AM Bharath Rupireddy
> <bharath.rupireddyforpostg...@gmail.com> wrote:
> > Right. I meant the "next use" as the select attempt on a foreign table
> > with that foreign server. If no select query is run, then at the end
> > of the current txn that connection gets closed. Yes internally such
> > connection gets closed in pgfdw_xact_callback.
> >
> > If the errdetail("Such connections get closed either in the next use
> > or at the end of the current transaction.") looks confusing, how about
> >
> > 1) errdetail("Such connection gets discarded while closing the remote
> > transaction.")/errdetail("Such connections get discarded while closing
> > the remote transaction.")
> > 2) errdetail("Such connection is discarded at the end of remote
> > transaction.")/errdetail("Such connections are discarded at the end of
> > remote transaction.")
> >
> > I prefer 2) Thoughts?
> >
> > Because we already print a message in pgfdw_xact_callback -
> > elog(DEBUG3, "closing remote transaction on connection %p"
>
> I changed the message to "Such connection is discarded at the end of
> remote transaction.".
>
> I'm attaching v5 patch set i.e. all the patches 0001 ( for new
> functions), 0002 ( for GUC) and 0003 (for server level option). I have
> also made the changes for increasing the version of
> postgres_fdw--1.0.sql from 1.0 to 1.1.
>
> I have no open points from my end. Please consider the v5 patch set
> for further review.
>
> With Regards,
> Bharath Rupireddy.
> EnterpriseDB: http://www.enterprisedb.com
>
