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.