Bug #53696 | Performance schema engine violates the PSEA API by calling my_error() | ||
---|---|---|---|
Submitted: | 17 May 2010 11:07 | Modified: | 30 Dec 2010 18:54 |
Reporter: | Konstantin Osipov (OCA) | Email Updates: | |
Status: | Closed | Impact on me: | |
Category: | MySQL Server: Performance Schema | Severity: | S3 (Non-critical) |
Version: | 5.6.99 | OS: | Any |
Assigned to: | Marc ALFF | CPU Architecture: | Any |
[17 May 2010 11:07]
Konstantin Osipov
[25 May 2010 9:04]
Sveta Smirnova
Thank you for the report. Verified as described. Really ha_federated, myisam, innobase and ibmdb2i have such calls too.
[25 May 2010 9:15]
Marc ALFF
Add csv and myisammrg to the list: csv/ha_tina.cc: my_error(ER_CHECK_NOT_IMPLEMENTED, MYF(0), "nullable columns"); federated/ha_federated.cc: my_error(error_num, MYF(0), buf); federated/ha_federated.cc: my_error(error_num, MYF(0), error_buffer); federated/ha_federated.cc: my_error(retval, MYF(0), error_buffer); federated/ha_federated.cc: my_error(error_code, MYF(0), error_buffer); federated/ha_federated.cc: my_error(error_code, MYF(0), ER(error_code)); federated/ha_federated.cc: my_error(ER_CONNECT_TO_FOREIGN_DATA_SOURCE, MYF(0), remote_error_buf); federated/ha_federated.cc: my_error(ER_FOREIGN_DATA_SOURCE_DOESNT_EXIST, MYF(0), sql_query.ptr()); ibmdb2i/db2i_charsetSupport.cc: my_error(ER_OUTOFMEMORY, MYF(0), sizeof(IconvMap)); ibmdb2i/ha_ibmdb2i.cc: my_error(ER_BLOB_USED_AS_KEY, MYF(0), "*unknown*"); innobase/handler/ha_innodb.cc: my_error(ER_QUERY_INTERRUPTED, MYF(0)); innobase/handler/ha_innodb.cc: my_error(ER_TOO_BIG_ROWSIZE, MYF(0), innobase/handler/ha_innodb.cc: my_error(EE_OUT_OF_FILERESOURCES, innobase/handler/ha_innodb.cc: my_error(ER_CHECK_NOT_IMPLEMENTED, MYF(0), "this functionality"); innobase/handler/ha_innodb.cc: my_error(ER_WRONG_COLUMN_NAME, MYF(0), innobase/handler/ha_innodb.cc: my_error(ER_TABLE_EXISTS_ERROR, MYF(0), buf); innobase/handler/ha_innodb.cc: my_error(ER_TABLE_EXISTS_ERROR, MYF(0), to); innobase/handler/ha_innodb.cc: my_error(ER_QUERY_INTERRUPTED, MYF(0)); innobase/handler/ha_innodb.cc: my_error(ER_BINLOG_STMT_MODE_AND_ROW_ENGINE, MYF(0), innobase/handler/ha_innodb.cc: my_error(ER_WRONG_NAME_FOR_INDEX, MYF(0), innobase/handler/handler0alter.cc: my_error(HA_ERR_TABLE_EXIST, MYF(0), innobase/handler/handler0alter.cc: my_error(HA_ERR_TABLE_EXIST, MYF(0), tmp_name); innobase/handler/handler0alter.cc: my_error(HA_ERR_TO_BIG_ROW, MYF(0)); innobase/handler/handler0alter.cc: my_error(ER_PRIMARY_CANT_HAVE_NULL, MYF(0)); innobase/handler/handler0alter.cc: my_error(ER_REQUIRES_PRIMARY_KEY, MYF(0)); myisam/mi_info.c: my_error(errcode, MYF(ME_NOREFRESH), file_name); myisam/myisampack.c: my_error(EE_OUTOFMEMORY,MYF(ME_BELL),sizeof(uint)*length*2); myisammrg/ha_myisammrg.cc: my_error(ER_ADMIN_WRONG_MRG_TABLE, MYF(0), parent_l->alias);
[25 May 2010 9:26]
Marc ALFF
Based on the existing calls to my_error() that originates from other storage engines, I fail to see that a "special case" exists in the performance schema. There is no functional bug either, the product behaves as expected. Closing as not a bug.
[25 May 2010 16:02]
Konstantin Osipov
Guys, (and girls) can you please read the code? Innobase and MyISAM do not push the error from, say, delete_all_rows(), performance schema does. In fact, both these engines push error in rare special cases. CSV and others are written by copy-paste programming and do not qualify as good examples.
[25 May 2010 16:07]
Konstantin Osipov
So, please let me refine this: The server assumes that no error comes from the engine in cursor calls: - delete_all_rows() (see the sql_truncate.cc code to verify) - rnd_first, rnd_next(), index_first(), index_next() (see records.cc code, that calls rr_print_error() explicitly after an error, and thus assumes there is no error on the stack to verify). There may be others. The case in point is delete_all_rows(), which currently demands a special case in the fix for Bug#42643 (please see the latest patch attached to the bug report).
[25 May 2010 17:12]
Sveta Smirnova
Konstantin, thank you for the update. Set back to "Verified"
[25 May 2010 20:19]
Konstantin Osipov
I don't see how errors can be ignored. The effect is that error may appear twice on the stack (i.e. the error list may contain duplicates).
[14 Jul 2010 1:06]
Marc ALFF
Analysis: Just found out today that there is a handler::get_error_message() method, which can be used to customize error messages from a storage engine. The fix should investigate how to use: - return HA_ERR_XXX from the performance schema code, preferably for existing HA_ERR error codes. - ha_perfschema::get_error_message()
[1 Dec 2010 12:07]
Bugs System
A patch for this bug has been committed. After review, it may be pushed to the relevant source trees for release in the next version. You can access the patch from: http://lists.mysql.com/commits/125646 3158 Marc Alff 2010-12-01 Bug#53696 Performance schema engine violates the PSEA API by calling my_error() This is a code cleanup. The implementation of a storage engine (subclasses of handler) is not supposed to call my_error() directly inside the engine implementation, but only return error codes, and report errors later at the demand of the sql layer only (if needed), using handler::print_error(). This fix removes misplaced calls to my_error(), and provide an implementation of print_error() instead. Given that the sql layer implementation of create table, ha_create_table(), does not use print_error() but returns ER_CANT_CREATE_TABLE directly, the return code for create table statements using the performance schema has changed to ER_CANT_CREATE_TABLE. Adjusted the test suite accordingly.
[1 Dec 2010 17:38]
Christopher Powers
Patch approved.
[1 Dec 2010 18:51]
Marc ALFF
Pushed into: - mysql-5.5-bugteam - mysql-trunk-bugfixing
[5 Dec 2010 12:43]
Bugs System
Pushed into mysql-trunk 5.6.1 (revid:alexander.nozdrin@oracle.com-20101205122447-6x94l4fmslpbttxj) (version source revid:alexander.nozdrin@oracle.com-20101205122447-6x94l4fmslpbttxj) (merge vers: 5.6.1) (pib:23)
[11 Dec 2010 17:07]
Paul DuBois
Code cleanup. No changelog entry needed.
[17 Dec 2010 12:52]
Bugs System
Pushed into mysql-5.5 5.5.9 (revid:georgi.kodinov@oracle.com-20101217124733-p1ivu6higouawv8l) (version source revid:marc.alff@oracle.com-20101201180757-db5woxysu9kenfau) (merge vers: 5.5.8) (pib:24)