Thread: BUG #5246: Misleading/inconsistent SQLSTATE behavior
The following bug has been logged online: Bug reference: 5246 Logged by: Chris Travers Email address: chris.travers@gmail.com PostgreSQL version: 8.1.18 Operating system: Fedora Linux 12 Description: Misleading/inconsistent SQLSTATE behavior Details: Hi all; I am noticing that that a failed database connection results in an unusable SQLSTATE in libpq, and a very different SQLSTATE than the backend registers. For example, if a connection fails due to a database not found, the backend registers 3D000 as a SQL state, but the front-end registers 25P01. If a login fails, the back-end registers 28000 but the front-end registers 25P01 again. 25P01 is "no_active_sql_transaction" and provides little information to the programmer as to how to handle the error. I may be missing something but the error looks to be entirely meaningless as it relates to a failed connection attempt as I, as a programmer, am loathe to trust that a generic transaction-related status message would be only used to track connection problems. From a programming perspective, it would be ideal for the same SQLSTATE triggered on the back-end to be available to the front-end. This leads to a number of very substandard workarounds. This might not be addressable within stable versions, but it would be very nice to see it fixed. Best Wishes, Chris Travers
"Chris Travers" <chris.travers@gmail.com> wrote: > I am noticing that that a failed database connection results in an > unusable SQLSTATE in libpq, and a very different SQLSTATE than the > backend registers. Well, if the client fails to connect to the server, I'm not sure how the server could communicate its SQLSTATE to the client, in order to force them to match. > For example, if a connection fails due to a database not found, > ... the front-end registers 25P01. I would have expected something in the 08 class, like maybe 08001 or 08004. > login fails, ... the front-end registers 25P01 again. That definitely sounds like it should use 08001. -Kevin
On Wed, Dec 16, 2009 at 12:35 PM, Kevin Grittner <Kevin.Grittner@wicourts.gov> wrote: > "Chris Travers" <chris.travers@gmail.com> wrote: > >> I am noticing that that a failed database connection results in an >> unusable SQLSTATE in libpq, and a very different SQLSTATE than the >> backend registers. > > Well, if the client fails to connect to the server, I'm not sure how > the server could communicate its SQLSTATE to the client, in order to > force them to match. It does send an error message. Currently I end up parsing that error message and checking to see if a connection is active. Unfortunately this becomes annoying where the locale of the PostgreSQL instance could change the messages received. It would be nice to have at least an option for software to pick up a code as to why a connection request fails instead of having to try to deal with feedback intended for a human, but I would settle for at least a SQLSTATE that indicated a connection problem instead of a transaction issue (08001 or 08004) since these would be a lot less ambiguous on the program interface side. Best Wishes, Chris Travers
"Chris Travers" <chris.travers@gmail.com> writes:
> I am noticing that that a failed database connection results in an unusable
> SQLSTATE in libpq, and a very different SQLSTATE than the backend
> registers.
> For example, if a connection fails due to a database not found, the backend
> registers 3D000 as a SQL state, but the front-end registers 25P01. If a
> login fails, the back-end registers 28000 but the front-end registers 25P01
> again.
Exactly what "frontend" are you talking about here? Because what this
sounds like to me is a client-side programming error. It's certainly
not the backend's fault, and I doubt it is libpq's either.
regards, tom lane
trying again. Sorry for the duplicate. On Wed, Dec 16, 2009 at 1:12 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > "Chris Travers" <chris.travers@gmail.com> writes: >> I am noticing that that a failed database connection results in an unusa= ble >> SQLSTATE in libpq, and a very different SQLSTATE than the backend >> registers. > >> For example, if a connection fails due to a database not found, the back= end >> registers 3D000 as a SQL state, but the front-end registers 25P01. =A0If= a >> login fails, the back-end registers 28000 but the front-end registers 25= P01 >> again. > > Exactly what "frontend" are you talking about here? =A0Because what this > sounds like to me is a client-side programming error. =A0It's certainly > not the backend's fault, and I doubt it is libpq's either. I am using DBD::Pg. I asked about it on #postgresql and was told this was the SQLSTATE code passed up from libpq (by the author of DBD::Pg). Several other folks there seemed to concur. It is certainly not a backend error. The back-end logs correct errors when logging is set to verbose. Best Wishes, Chris Travers
Chris Travers <chris@metatrontech.com> writes:
> trying again. Sorry for the duplicate.
> On Wed, Dec 16, 2009 at 1:12 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Exactly what "frontend" are you talking about here? Because what this
>> sounds like to me is a client-side programming error. It's certainly
>> not the backend's fault, and I doubt it is libpq's either.
> I am using DBD::Pg.
> I asked about it on #postgresql and was told this was the SQLSTATE code
> passed up from libpq (by the author of DBD::Pg). Several other folks
> there seemed to concur.
Could you provide a self-contained test case? And what versions of
DBD::Pg etc are we talking about?
regards, tom lane
On Wed, Dec 16, 2009 at 1:39 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Chris Travers <chris@metatrontech.com> writes: >> trying again. =A0Sorry for the duplicate. >> On Wed, Dec 16, 2009 at 1:12 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: >>> Exactly what "frontend" are you talking about here? =A0Because what this >>> sounds like to me is a client-side programming error. =A0It's certainly >>> not the backend's fault, and I doubt it is libpq's either. > >> I am using DBD::Pg. > >> I asked about it on #postgresql and was told this was the SQLSTATE code >> passed up from libpq (by the author of DBD::Pg). =A0Several other folks >> there seemed to concur. > > Could you provide a self-contained test case? =A0And what versions of > DBD::Pg etc are we talking about? Just to weed out possibilities of DBD::Pg behavior causing this, I am going to try to create a straight C test case. If I can't I will end the perl scripts I was using to test. Best Wishes, Chris Travers > > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0regards, tom lane >
Hi Tom and everyone else; After significant additional research this is what I have turned up: 1) The problem was a problem in DBD::Pg which didn't quite succeed in setting the connection state to 08006. I have submitted a patch for that to the DBD::Pg project. 2) As of 8.1, tshark shows that the server does send the SQLSTATE to the client regarding why the login fails (for example 3D000 in the case of bad db name). Libpq as far as I can tell (from reading the code) doesn't do anything with this code. Certainly there seems to be no exposure of the SQLSTATE to anything as it relates to a failed connection attempt. I could be missing something because I am not extremely familiar with the libpq codebase, but it seems that the value is just discarded. Are there any plans to expose the SQLSTATE from a failed connection attempt upwards through the library? (I would be happy to try to write a patch but you probably don't want my C code in your library.) Best Wishes, Chris Travers
Chris Travers <chris@metatrontech.com> writes:
> 2) As of 8.1, tshark shows that the server does send the SQLSTATE to
> the client regarding why the login fails (for example 3D000 in the
> case of bad db name). Libpq as far as I can tell (from reading the
> code) doesn't do anything with this code. Certainly there seems to be
> no exposure of the SQLSTATE to anything as it relates to a failed
> connection attempt. I could be missing something because I am not
> extremely familiar with the libpq codebase, but it seems that the
> value is just discarded.
Yeah. The problem is that the only infrastructure libpq has for returning
individual error message fields (like the SQLSTATE) is associated with a
PGresult, and there's no PGresult for a connection failure. I see no
easy way to fix that without incompatible changes in libpq's API.
This is related to the fact that errors detected internally in libpq
generally lack SQLSTATEs. Part of the reason that fixing that has been
so low-priority is that in many cases there's no existing API whereby
they could be returned anyhow. It's been on the TODO list since 7.4,
but nobody has cared to tackle it.
regards, tom lane
Just to be clear (before I consider attempting a patch maybe I can hand off to someone else to double-check)... On Wed, Dec 16, 2009 at 5:31 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Yeah. =A0The problem is that the only infrastructure libpq has for return= ing > individual error message fields (like the SQLSTATE) is associated with a > PGresult, and there's no PGresult for a connection failure. =A0I see no > easy way to fix that without incompatible changes in libpq's API. Looking through the code it looks like this is for two reasons: 1) The current API assumes a PGResult rather than a connection being hande= d in. 2) There is no place in the struct for a connection to handle the message fields. > > This is related to the fact that errors detected internally in libpq > generally lack SQLSTATEs. =A0Part of the reason that fixing that has been > so low-priority is that in many cases there's no existing API whereby > they could be returned anyhow. =A0It's been on the TODO list since 7.4, > but nobody has cared to tackle it. I do a lot with SQLSTATEs in my Perl code and having access to this would be really quite helpful. (More info below but don't want to crowd out my questions.) It looks like this could be added without a disruption to programmer interfaces, but it seems like any major change in this area would create binary compatibility issues (i.e. require recompile of linked software). Is this correct in what you mean by API incompatibility? A quick review suggests to me it shouldn't be too bad to add this, but at the same time my C code is not the best out there. I might still be willing to give it a shot. As for more info: I use the SQLSTATE field quite heavily for error handling and while it isn't always sufficient by itself, it is quite helpful in detecting errors and providing more helpful messages to end users. The application I am working on at the moment uses database roles as application roles and enforces security in the database. On login, the user has to enter username, password, and database name in order to access the application. The problem I was running into is that if the user enters a non-existant database, the program would prompt for username/password instead of letting them know the database was wrong. My workaround at the moment is to check the error message against a configurable value to see if it represents a missing database. It sucks because it means that foreign locale users must go through extra configuration steps. If I had the SQLSTATE data it would be easy to set up so that wouldn't be needed.
Chris Travers <chris@metatrontech.com> writes:
> It looks like this could be added without a disruption to programmer
> interfaces, but it seems like any major change in this area would
> create binary compatibility issues (i.e. require recompile of linked
> software). Is this correct in what you mean by API incompatibility?
No, I'm concerned about the programmer interface at the moment. What
have you got in mind?
> ... The problem I was running into is that if the
> user enters a non-existant database, the program would prompt for
> username/password instead of letting them know the database was wrong.
> My workaround at the moment is to check the error message against a
> configurable value to see if it represents a missing database. It
> sucks because it means that foreign locale users must go through extra
> configuration steps. If I had the SQLSTATE data it would be easy to
> set up so that wouldn't be needed.
We do have a workaround for distinguishing "password required" from
other errors without any locale-specific tests. It is surely a crock,
but you'd want to use that in the near term anyway. Any real fix here
could not appear before 8.5 at the earliest.
regards, tom lane
On Wed, Dec 16, 2009 at 7:07 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Chris Travers <chris@metatrontech.com> writes: >> It looks like this could be added without a disruption to programmer >> interfaces, but it seems like any major change in this area would >> create binary compatibility issues (i.e. require recompile of linked >> software). =A0Is this correct in what you mean by API =A0incompatibility? > > No, I'm concerned about the programmer interface at the moment. =A0What > have you got in mind? My thinking (rather hackish) was to do as follows: (Rough version) 1) Add an errFields member to pg_conn similar to what exists in pg_result 2) create a "dummy" pgResult instance inside the connection object to track the messages (thus use internally any functions to store errorfields there) 3) Store error message fields using the errFields section of the pgResult using whatever standard methods currently used for pgResult structs. 4) Copy over to connection struct 5) Create a new function to expose this to the application. Maybe named PQresultErrorField. Then test this and make sure it works.... Then refine as follows: 1) Refactor any errfields/Alloc-type functions as possible to get rid of the necessity to go through a dummy result struct. Remove the dummy result struct and copy fields manually. Maybe refactor the field checking to centralize it between the result and connection functions? Then test this to make sure it works again. Then see if I can get someone else to further comment on/review/refine the patch. > >> ... The problem I was running into is that if the >> user enters a non-existant database, the program would prompt for >> username/password instead of letting them know the database was wrong. >> My workaround at the moment is to check the error message against a >> configurable value to see if it represents a missing database. =A0It >> sucks because it means that foreign locale users must go through extra >> configuration steps. =A0If I had the SQLSTATE data it would =A0be easy to >> set up so that wouldn't be needed. > > We do have a workaround for distinguishing "password required" from > other errors without any locale-specific tests. =A0It is surely a crock, > but you'd want to use that in the near term anyway. =A0Any real fix here > could not appear before 8.5 at the earliest. What sort of current workarounds are there? Best Wishes, Chris Travers > > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0regards, tom lane >