On 24.03.25 21:33, Matheus Alcantara wrote:
I'm a bit confused about the refactoring patch 0001. There are some
details there that don't seem right. For example, you write that the
pfree(rconn) calls are no longer necessary, but AFAICT, it would still
be needed in dblink_get_conn(). Also, there appear to be some possible
behavior changes, or at least it's not fully explained, like
connect_pg_server() doing foreign-server name resolution, which
dblink_get_conn() did not do before.
But it's actually not clear to me how the refactoring in 0001
contributes to making the patch 0002 better, since patch 0002 barely
touches the code touched by 0001.
How would patch 0002 look without 0001 before it? Which code would need
to be duplicated or what other awkward changes are you trying to avoid?
You are right, I think that the refactor was needed on the initial
versions of the patch because it was referencing the UseScramPassthrough
function in multiple places, so the refactor was needed to accomplish the
parameters of the function.
Since we now assume that the UseScramPassthrough is already checked on
some parts of the code I agree that this refactor is not required
anymore. Attached v11 without the refactor patch.
Committed.
I cut down the documentation a bit and instead linked to postgres_fdw
for some of the details. I think that's better than having to maintain
that text in two different places.
