MDEV-35602: Wrong table column count mentioning corruption harmful #3685
Conversation
grooverdan
force-pushed
the
MDEV-35602-wrong-count-isnt-corruption
branch
from
34887e9 to
6d9b26b
Compare
|
Hi Daniel! I don't agree with your proposed wording. If you intend the error message to steer users towards a solution, you should change the error messages to actually say that. What you have done now is leave users in an even more confused state. From a user's POV: |
There was a problem hiding this comment.
Please explain the problem you are fixing in the MDEV properly.
- What is the user story behind this?
- Where does this problem come from?
- What kind of use cases lead to this error?
Because if users run into this issue, there must be a "tutorial" somewhere that tells them to shoot themselves in the foot.
If you want users to have an easier time, make sure that the errors clearly explain what the problem is and what to look at in order to resolve things. With your suggested changes you do not explain to the user anything extra.
cvicentiu
added
the
MariaDB Foundation
The calls to Proc_table::report_error from various code paths populate the error and code related to the error. For the mysql.proc table, all this information is ignored and a common ER_CANNOT_LOAD_FROM_TABLE_V2 error is thrown. We correct this by using the error provided. The documented purpose of the Proc_table_intact::report_error function was to report the error only ony once. With a code==0 passed to the function it fails to achieve this objective. This behaviour was preserved anyway as otherwise some to preserve some of the current messages. SHOW PROCEDURE STATUS got an explict error of ER_SP_PROC_TABLE_CORRUPT pushed outside the transaction in fill_schema_proc. In the implementation of Proc_table::report_error, Rather than doing a write to buffer implementation of this using my_printv_error as a common implementation that takes a varglist and the same implementation as the current my_message call. The sp-destruct test case, as expected, started to produce different results. The more general exposure of ER_SP_PROC_TABLE_CORRUPT highlighted "Internal code" as a reason, expanded this to text to make to more readable (but not great).
grooverdan
force-pushed
the
MDEV-35602-wrong-count-isnt-corruption
branch
from
6d9b26b to
33aba50
Compare
Description
ER_COL_COUNT_DOESNT_MATCH_CORRUPTED_V2 mentioning the table is possibly corrupted leaves users with no avenue as to what action to take.
The simpler version of just saying the column count isn't matching leaves a manual reconciliation and mariadb-upgrade paths as something that can be attempted.
Release Notes
"Possible corruption" notices around system table with the wrong column count was unlikely to be corruption, just a non-standard upgrade path.
How can this PR be tested?
TODO: modify the automated test suite to verify that the PR causes MariaDB to behave as intended.
Consult the documentation on "Writing good test cases".
If the changes are not amenable to automated testing, please explain why not and carefully describe how to test manually.
Basing the PR against the correct MariaDB version
mainbranch.PR quality check