Provide SQLSTATE

[eulerto]

PgBouncer currently uses SQLSTATE 08P01 (protocol_violation) as default error code to send error messages to client. There are cases that this SQLSTATE is different from what Postgres provides. Since the goal is to be as transparent as possible, PgBouncer should report the SQLSTATE provided by Postgres. Issue #778 exposes one case that if you connect to a database that doesn't exist, Postgres reports 3D000 (invalid_catalog_name) but PgBouncer reports 08P01. The npgsql driver relies on the SQLSTATE to check if the database already exists and this behavior breaks it. PgBouncer will use the SQLSTATE that Postgres provides.

Fix issue #778.

[JelteF]

I think this should be moved to disconnect_client_sqlstate.

[eulerto]

Are you referring to now variable? It was moved to disconnect_client_sqlstate. I checked a few times after reading the diff. diff seems confusing. now is in line 999.

[JelteF]

no I meant the if (reason) { block

[JelteF]

code looks good overall. But I think this should include a regression test. Also there's some compilation warning in CI that should be fixed.

[eulerto]

Thanks for checking. I noticed the failure after creating this PR. I will fix that error and create a test case for this issue.

[eulerto]

I tried to come up with a test case but it seems psycopg decided that it doesn't use the Invalid Catalog Name (3D000) for the 'database "foo" does not exist' case. According to psycopg2.errors, class 3D belongs to ProgrammingError exception but a single connection to a database that doesn't exist returns OperationalError exception.

Traceback (most recent call last):
  File "/tmp/c.py", line 13, in <module>
    con2 = psycopg2.connect("host=localhost port=9915 dbname=nonexist")
  File "/usr/lib/python3/dist-packages/psycopg2/__init__.py", line 127, in connect
    conn = _connect(dsn, connection_factory=connection_factory, **kwasync)
psycopg2.OperationalError: FATAL:  database "nonexist" does not exist

Unless, I'm missing some trick, I think a test case won't be possible.

[JelteF]

All of the pscopg error types have an sqlstate field. If you check that manually to be the expected error code, then I think we have tested it well enough.

[JelteF]

Okay I tried to create the test that I had in my mind. But indeed it's not possible. The error code is not exposed to us. And the reason for that is actually because the error result is not exposed to psycopg by libpq.

Did you manually test that this fixes the problem with npgsql? If so, I think we should just merge this. The code looks good, but someone should at least test that this fixes the issue manually.

[eulerto]

I manually inspected the messages but didn't give it a try with Npgsql. I pinged the other issue to see if someone tested it or can test it.

[JelteF]

If you manually inspected the result that's good enough for me. I'll press the merge button now. In the unlikely case that people on the issue still reach out that it didn't fix the problem, then it's probably a minor change.