Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SQLState #207

Closed
alexzorin opened this issue Jan 21, 2014 · 9 comments
Closed

SQLState #207

alexzorin opened this issue Jan 21, 2014 · 9 comments

Comments

@alexzorin
Copy link

Hi,

Was there a particular reason that the SQLState code is not being used as part of MySQLError? It is currently in the codebase but commented out:

    // SQL State [optional: # + 5bytes string]
    // sqlstate := string(data[pos : pos+6])

Would you be interested in a PR for this? I'd like to use this code if possible instead of MySQL's own error code.

@arnehormann
Copy link
Member

It's left out because we want to keep allocations to a minimum.
Also, it apparently contains a lot less information than the MySQL error code.
Have a look at http://dev.mysql.com/doc/refman/5.1/en/error-messages-server.html and count the occurrences of "HY000".
The driver is MySQL specific, the error type too. As that dependency exists anyway, there's not much need in a less granular error information causing an additional allocation.

Do you still want to use that code given MySQLs excessive mapping to HY000? If not, please close this. If you do, we'll have to discuss it.

@julienschmidt
Copy link
Member

As Arne said, we want to keep the memory footprint small.
But we're always open for input. Convince us why this is useful or even necessary 😉

@alexzorin
Copy link
Author

Thanks for the feedback.

Despite the significantly lower granularity of SQLSTATE, I was hoping to use it to classify broad classes of errors (such as duplicate keys) across different rdbmses - but the drivers would need to report the SQLSTATE otherwise this wouldn't be possible - so I was just canvassing reactions really.

The other idea I had was that this driver could provide symbols for comparing the MySQL error codes. It shouldn't be too hard to do this, http://bazaar.launchpad.net/~mysql/mysql-server/5.7/view/head:/sql/share/errmsg-utf8.txt looks easily parseable. Thoughts? I'm not sure if its idiomatic for a driver to provide this, but it would be convenient.

@xaprb
Copy link

xaprb commented Jan 21, 2014

The errmsg files are release-specific and change not infrequently. I think
that way lies madness.

@arnehormann
Copy link
Member

Spaaaaarta! Sorry, couldn't resist, will let myself out...

@arnehormann
Copy link
Member

... and on a more serious note: I wouldn't use the messages and I don't want that in the driver directly.
We can't use the text messages anyway, they are full of formatting instructions (%s).
But I could imagine a subfolder with a function getting the MySQLError and returning a string with the SQL state and the name of the mysql error constant (for reference when one searches for it). A giant switch-case on the error number.

@xaprb
Copy link

xaprb commented Jan 21, 2014

You can already do that with the perror tool, which also provides helpful access to OS error codes:

$ mysql -uroot -pfoobar
ERROR 1045 (28000): Access denied for user 'root'@'localhost' (using password: YES)

# hmmm, what is error 1045?

$ perror 1045
MySQL error code 1045 (ER_ACCESS_DENIED_ERROR): Access denied for user '%-.48s'@'%-.64s' (using password: %s)

# I feel like grepping some logs today...

080114 11:30:29 mysqld started 
080114 11:30:29 InnoDB: Operating system error number 13 in a file operation.

# what is error 13?

$ perror 13
OS error code  13:  Permission denied

@arnehormann
Copy link
Member

@xaprb Yes, but you can't do it in a program without starting a subprocess. I think that's a bit much if you only want a way to make errors more comparable between different kinds of databases in an ORM solution which @alexzorin wants to do.
Hmm... on the other hand, I never got how you benefit from a tool for multiple kinds of rdbms - except for migrations.

@arnehormann
Copy link
Member

As I'm rereading this, it's a duplicate of #202

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants