psycopg / psycopg2

PostgreSQL database adapter for the Python programming language
https://www.psycopg.org/
Other
3.31k stars 503 forks source link

Add connection.get_host_addr() #1682

Closed exg closed 6 months ago

exg commented 6 months ago

In some use cases connection.cancel() is not usable because it may block the interpreter for a long time (as it does not release the GIL before calling PQcancel and does not support setting the tcp timeout for the cancel connection). An alternative approach to (asynchronously) cancel a query is to call pg_cancel_backend over a new connection. For this to work reliably, the connection should be established against the server IP address associated to the connection where the query is running. To this end, this PR adds a connection method to get the info using PQhostaddr.

dvarrazzo commented 6 months ago

What's wrong with connection.info.host?

dvarrazzo commented 6 months ago

I also don't see why you need the host... you need the whole set of connection parameters in order to establish a connection and to issue a pg_cancel_backend().

exg commented 6 months ago

What's wrong with connection.info.host?

In the case where host is a domain name, we need the resolved IP address, as the resolution may yield a different IP address (and different database) at cancellation time. Note that this is analogous to PQcancel:

https://github.com/postgres/postgres/blob/REL_16_2/src/interfaces/libpq/fe-connect.c#L4718

https://github.com/postgres/postgres/blob/REL_16_2/src/interfaces/libpq/fe-connect.c#L4919

dvarrazzo commented 6 months ago

Ah, I see: you are after a PQhostaddr wrapper, not PQhost. So if any you should add a info.hostaddr method, right?

However, I am reluctant to add new interfaces to psycopg2. PQhostaddr was introduced only in PG 12 and I can see an entire generation of bugs arising because people might have old libpq libraries still on their system. StackOverflow is still littered of the corpses of such type of mismatches with previous optionally-compiled libpq functions.

The function you need is already exposed by psycopg3, which is implemented in a way to manage gracefully the cases in which libpq 12 wasn't available at compile time or is not available at runtime, see the low-level PQhostaddr wrapper.

So, all in all, I suggest you to move to psycopg 3.

exg commented 6 months ago

Ah, I see: you are after a PQhostaddr wrapper, not PQhost. So if any you should add a info.hostaddr method, right?

You are right, I revised the change.

However, I am reluctant to add new interfaces to psycopg2. PQhostaddr was introduced only in PG 12 and I can see an entire generation of bugs arising because people might have old libpq libraries still on their system. StackOverflow is still littered of the corpses of such type of mismatches with previous optionally-compiled libpq functions.

I understand your concern. However, wouldn't it be enough to check the version at runtime to avoid such issues? PQlibVersion was introduced in PG 9.1. The revised change includes this guard.

The function you need is already exposed by psycopg3, which is implemented in a way to manage gracefully the cases in which libpq 12 wasn't available at compile time or is not available at runtime, see the low-level PQhostaddr wrapper.

So, all in all, I suggest you to move to psycopg 3.

That would be ideal. However, due to time constraints, I need to stick to psycopg2 for the time being. Is there any chance you could reconsider?

dvarrazzo commented 6 months ago

I understand your concern. However, wouldn't it be enough to check the version at runtime to avoid such issues? PQlibVersion was introduced in PG 9.1. The revised change includes this guard.

Unfortunately C is not so forgiving and problems can happen with system where pg_config belongs to a newer version than the library that will be found at runtime. Not quite DLhell, but close.

So, all in all, I suggest you to move to psycopg 3.

That would be ideal. However, due to time constraints, I need to stick to psycopg2 for the time being. Is there any chance you could reconsider?

Not for free. Asking for development at a level that goes out of the lifetime of a free software project is exactly one of the ways a company can contribute to free software maintenance.

But I have a hack for you. The conn.pgconn_ptr was added for cases like this. You can use ctypes to access the libpq function and pass it this pointer.

# test-hostaddr.py 

import ctypes
import ctypes.util

import psycopg2
conn = psycopg2.connect("host=localhost")

libname = ctypes.util.find_library('pq')
assert libname
libpq = ctypes.pydll.LoadLibrary(libname)
assert libpq

libpq.PQhostaddr.argtypes = [ctypes.c_void_p]
libpq.PQhostaddr.restype = ctypes.c_char_p
print("hostaddr", libpq.PQhostaddr(conn.pgconn_ptr))
$ python test-hostaddr.py 
hostaddr b'127.0.0.1'

Note that this only works if you use psycopg2, not psycopg2-binary. In the latter case I assume you must load the libpq library bundled with the python package. I'll leave that as an exercise (I wasted a decent amount of time figuring this out the hard way).

exg commented 6 months ago

But I have a hack for you. The conn.pgconn_ptr was added for cases like this. You can use ctypes to access the libpq function and pass it this pointer.

I see, thank you for the help.