Attached are 3 patches that address the feedback from Andres about code duplication and splitting up commits. I completely removed internal_cancel now, since it only had one caller at this point.
> IMO, this is a new feature not a bug fix IMO this is definitely a bugfix. Nowhere in the libpq docs it stated that the connection options in question do not apply to connections that are opened for cancellations. So as a user I definitely expect that any connections that libpq opens would use these options. Which is why I definitely consider it a bug that they are currently not honoured for cancel requests. However, even though I think it's a bugfix, I can understand the being hesitant to backport this. IMHO in that case at least the docs should be updated to explain this discrepancy. I attached a patch to do so against the docs on the REL_14_STABLE branch. > To me it seems a bit problematic to introduce a divergence between windows / > everything else here. Isn't that just going to lead to other complaints just > like this thread, where somebody discovered the hard way that there's platform > dependent behaviour here? Of course, fixing this also for windows would be much better. There's two problems: 1. I cannot find any clear documentation on which functions are signal safe in Windows and which are not. The only reference I can find is this: https://docs.microsoft.com/en-us/cpp/c-runtime-library/reference/signal?view=msvc-170 However, this says that you should not use any function that generates a system call. PQcancel is obviously already violating that when calling "connect", so this is not very helpful. 2. My Windows C experience is non existent, so I don't think I would be the right person to write this code. IMO blocking this bugfix, because it does not fix it for Windows, would be an example of perfect becoming the enemy of good. One thing I could do is add a note to the docs that these options are not supported on Windows for cancellation requests (similar to my proposed doc change for PG14 and below).
0001-Refactor-to-remove-internal_cancel-helper.patch
Description: 0001-Refactor-to-remove-internal_cancel-helper.patch
0002-Use-timeout-socket-options-for-cancel-connections.patch
Description: 0002-Use-timeout-socket-options-for-cancel-connections.patch
0003-Honor-connect_timeout-when-connecting-with-PQcancel.patch
Description: 0003-Honor-connect_timeout-when-connecting-with-PQcancel.patch
0001-Doc-explain-that-connection-options-don-t-apply-to-c.patch
Description: 0001-Doc-explain-that-connection-options-don-t-apply-to-c.patch
