MDEV-27952 Improve error log messages with descriptive text already present in the code #3058

grooverdan · 2024-02-13T08:22:18Z

The Jira issue number for this PR is: MDEV-27952

Description

…
This is explicitly for the test case:

CREATE TABLE t (c INT) ENGINE=InnoDB;
ALTER TABLE t DISCARD TABLESPACE;
ALTER TABLE t ADD INDEX (c);
SELECT * FROM t;

That had an error:
[ERROR] Got error 194 when reading table './test/t'

194 is the code from HA_ERR_TABLESPACE_MISSING.

As these code are at the handler level, based on the existing code the table name is assumed. There is a built in function my_get_err_msg that translates the number to a format with its argument that can be assumed to be a table string.

The error is now:

2024-02-13 19:08:44 3 [ERROR] Tablespace is missing for table './test/t'

Release Notes

Per title

How can this PR be tested?

Manual test per MDEV

Basing the PR against the correct MariaDB version

This is a new feature and the PR is based against the latest MariaDB development branch.
This is a bug fix and the PR is based against the earliest maintained branch in which the bug can be reproduced.

PR quality check

I checked the CODING_STANDARDS.md file and my PR conforms to this where appropriate.
For any trivial modifications to the PR, I am ok with the reviewer making the changes themselves.

grooverdan · 2024-02-13T08:51:40Z

+    sql_print_error("Error %M", error, table->s->path.str);

Generates:

2024-02-13 19:49:55 4 [ERROR] Error 194 "Tablespace is missing for a table"

which isn't correct because it doesn't have the tablename.

…resent in the code This is explictly for the test case: CREATE TABLE t (c INT) ENGINE=InnoDB; ALTER TABLE t DISCARD TABLESPACE; ALTER TABLE t ADD INDEX (c); SELECT * FROM t; That had an error: [ERROR] Got error 194 when reading table './test/t' 194 is the code from HA_ERR_TABLESPACE_MISSING. As these code are at the handler level, based on the existing code the table name is assumed. There is a built in function my_get_err_msg that translates the number to a format with its argument that can be assumed to be a table string. The error is now: 2024-02-13 19:08:44 3 [ERROR] Tablespace is missing for table './test/t' Occasionally an index gets called with report_error so the path form is still there as opposed to changing format or doing a db.table string. A few tests ajusted for the exact error. Achive storage engine fixed for all its incorrect returning of errno instead of a HA_ERR_{code}. Reported BINLOG messages correctly to opposed to unknown error.

dr-m · 2024-02-16T10:12:01Z

mysql-test/suite/archive/archive.result

 call mtr.add_suppression("Table 't1' is marked as crashed and should be repaired");
+call mtr.add_suppression("Table './test/t1' is marked as crashed and should be repaired");


Can we edit the existing suppressions instead:

call mtr.add_suppression("Table '.*t1' is marked as crashed and should be repaired");

dr-m · 2024-02-16T10:19:13Z

mysql-test/suite/innodb/t/innodb_corrupt_bit.test

@@ -8,6 +8,7 @@

 --disable_query_log
 call mtr.add_suppression("Flagged corruption of.* in table .* in .*");
+call mtr.add_suppression("Index ./test/corrupt_bit_test_@1s is corrupted");


Could we convert the filename-safe table name to the user-specified name test.corrupt_bit_test_ā with something like this?

static void dict_table_open_failed(const table_name_t &name) { my_printf_error(ER_TABLE_CORRUPT, "Table %`.*s.%`s is corrupted." " Please drop the table and recreate.", MYF(ME_ERROR_LOG), int(name.dblen()), name.m_name, name.basename()); }

Or would it require a different format variant to be implemented?

Please do not use &table_name_t ; Please use pointers instead.
If possible, use C syntax cast instead of C++ syntax cast.

Note that when using a table name for error messages inside InnoDB, you have to use conversion functions to change it from filename format to table name format.
You have to use innobase_convert_to_system_charset() when converting filenames to table names.

montywi · 2024-04-08T16:19:19Z

mysql-test/suite/archive/archive.test

@@ -6,6 +6,7 @@

 CALL mtr.add_suppression("Unsafe statement written to the binary log using statement format since BINLOG_FORMAT = STATEMENT");
 call mtr.add_suppression("Table 't1' is marked as crashed and should be repaired");
+call mtr.add_suppression("Table './test/t1' is marked as crashed and should be repaired");


Please ensure that we do not use '/' in the suppression. (Will not work on windows)

montywi

Please fix the commented issues.

montywi · 2024-04-08T16:20:57Z

storage/archive/ha_archive.cc

@@ -287,7 +287,7 @@ static int archive_discover(handlerton *hton, THD* thd, TABLE_SHARE *share)
 if (!(azopen(&frm_stream, az_file, O_RDONLY|O_BINARY)))
 {
 if (errno == EROFS || errno == EACCES)
- DBUG_RETURN(my_errno= errno);
+ DBUG_RETURN(HA_ERR_TABLE_READONLY);


You MUST set my_errno here!