Looks good overall. I attached a new version with a few small changes: > * Changed store_conn_addrinfo to return int like how all the functions > dealing with addrinfo does. Also moved the error reporting to inside > there > where the error happened.
I don't feel strong about the int vs bool return type. The existing static libpq functions are a bit of a mixed bag around this, so either way seems fine to me. And moving the log inside the function seems fine too. But it seems you accidentally removed the "goto error_return" part as well, so now we're completely ignoring the allocation failure. The attached patch fixes that. >+ok($node1_occurences > 1, "expected at least one execution on node1, found >none"); >+ok($node2_occurences > 1, "expected at least one execution on node2, found >none"); >+ok($node3_occurences > 1, "expected at least one execution on node3, found >none"); I changed the message to be a description of the expected case, instead of the failure case. This is in line with the way these messages are used in other tests, and indeed seems like the correct way because you get output from "meson test -v postgresql:libpq / libpq/003_load_balance_host_list" like this: ▶ 6/6 - received at least one connection on node1 OK ▶ 6/6 - received at least one connection on node2 OK ▶ 6/6 - received at least one connection on node3 OK ▶ 6/6 - received 50 connections across all nodes OK Finally, I changed a few small typos in your updated commit message (some of which originated from my earlier commit messages)
v16-0001-Copy-and-store-addrinfo-in-libpq-owned-private-m.patch
Description: Binary data
v16-0002-Support-connection-load-balancing-in-libpq.patch
Description: Binary data
