mysql
mysql copied to clipboard
Add error codes from MySQL 8
Fixes #2325 Fixes #2402 Fixes #2410
This PR makes the generate-error-constants.js tool able to get all the error codes from the MySQL 8 sources, as the txt file which defines all of them was in a different location compared to MySQL 5.
I've also included the output from the tool.
However, there appears to be one issue. The MySQL 8 sources marks a lot of the MySQL 5 error codes as "obsolete". Since MySQL 5 is going to go EOL in Oct 2023, I'm assuming that these "obsolete" codes shouldn't be marked as such, at least until then. In order to solve this, we can do one of the two things:
- Manually edit
errors.jsto include the non-obsolete definitions for the MySQL 5 error codes. - Allow for
generate-error-constants.jsto be passed 2 directories, one pointing the the MySQL 5 sources, and the other to the MySQL 8 sources. If it detects a MySQL 8 definition which starts withOBSOLETE_, it'll fall back to the MySQL 5 definition.
Please let me know what's preferred so I can make the adjustments.
Thank so much @ARitz-Cracker ! Yea, we definately need some solution to the 5 vs 8 issue, as we cannot just drop this change in as-is with changing all those code to include the word obsolete -- as code that uses this library relies on the code names there for detection of what type of error occurred vs the number, so it would be at least a semver major change for this module (not even considering if it makes sense to lead them with obsolete).
Perhaps, if all the codes are included in 5 sources still, would just having the tool simply remove the OBSOLETE_ prefix be sufficient?
As for your linked issues, I see you say this fixes those two bug reports. Can you list here which code number fixes #2325 and which code number fixes #2350 ? We don't want to close those out without being sure this fixes those issues, and the way you have the description written GitHub will automatically close them once this is merged, so want to be sure it makes sense.
Hello, @dougwilson!
This PR does include ER_WRONG_SRID_FOR_COLUMN which #2325 refers to. I'll remove the reference to #2350 as I can't find the string "Connection was killed" on its own anywhere in the MySQL 8 sources.
As for making the tool simply removing the OBSOLETE_ prefix, I don't think that's a good idea. The most obvious reason being that some 5.x codes are defined as "OBSOLETE_ER_UNUSED" in 8.x. like here
Going down this path, I think making the tool read both the 5.x sources and the 8.x sources is the best solution.
Also, the CI is failing due to linting errors, I'll make sure to run eslint this time. 😆
By the way, you may notice that this PR still replaces some of the generated output with "unused" or "obsolete", this must have been done sometime between 5.7.29 and 5.7.31. So there are still some definitions which both versions mark as obsolete. Should I make the script remove those last remaining obsolete and unused definitions from the list?
@dougwilson one more thing! Since I'm going through the trouble of modifying the error constants generator anyway, I can further extend the tool to read the MariaDB sources as well, and thus solve #1982! the MariaDB 10 sources have them defined in a similar manner as MySQL 5 with some minor tweaks. So it definitely won't be difficult to do.
That would be neat. I think I tried that and ran into issues. There may be a branch in this repo with that work already if you want to take a look.
But regardless, please make sure to do the MariaDB changes as a separate PR from this one.
I have been at work since I first commented and will take a look at the new changes of this PR after work.
Sounds great! I'll create a new branch which extends onto this one. Also, just made one final change to make sure that this tool will always prioritize MySQL 5 definitions over MySQL 8 so that there are no breaking changes.
I've made another branch here, which contains the MariaDB codes and the MySQL 8 codes. I'll submit a PR for that one once this one is approved.
I've just re-written history. I removed the changes to errors.js during my 3 wip commits, and added a final commit where I ran the tool. Things look like they work as expected now!
Hello! I don't know why the AppVeyor build failed. Should I rebase this PR?