| Bug #69378 | Wasted work in method row_ins_foreign_check_on_constraint() | ||
|---|---|---|---|
| Submitted: | 1 Jun 2013 23:33 | Modified: | 9 Jan 2015 16:03 |
| Reporter: | Po-Chun Chang (OCA) | Email Updates: | |
| Status: | Can't repeat | Impact on me: | |
| Category: | MySQL Server: InnoDB storage engine | Severity: | S5 (Performance) |
| Version: | 5.6, 5.7 | OS: | Any |
| Assigned to: | CPU Architecture: | Any | |
| Tags: | patch, performance | ||
[1 Jun 2013 23:33]
Po-Chun Chang
Suggested patch
Attachment: patch.diff (text/plain), 323 bytes.
[2 Jun 2013 10:20]
MySQL Verification Team
Thanks for the report. There are two such loops in the same function, both can be shortcut. Also variable name is spelt wrong, it should be fts_col_affected
[26 Jun 2013 2:56]
Po-Chun Chang
Suggested patch (*) I confirm the code being submitted is offered under the terms of the OCA, and that I am authorized to contribute it.
Contribution: patch.diff (text/plain), 323 bytes.
[27 Jun 2013 8:24]
Jimmy Yang
Hi, please notice there is a piece of work before this assignment that initialize the affected fields:
for (i = 0; i < foreign->n_fields; i++) {
upd_field_t* ufield = &update->fields[i];
ufield->field_no = dict_table_get_nth_col_pos(
table,
dict_index_get_nth_col_no(index, i));
ufield->orig_len = 0;
ufield->exp = NULL;
dfield_set_null(&ufield->new_val);
if (table->fts && dict_table_is_fts_column(
table->fts->indexes,
dict_index_get_nth_col_no(index, i))
!= ULINT_UNDEFINED) {
fts_col_affacted = TRUE;
}
}
Notice following code are repeated for all affected fields:
upd_field_t* ufield = &update->fields[i];
ufield->field_no = dict_table_get_nth_col_pos(
table,
dict_index_get_nth_col_no(index, i));
ufield->orig_len = 0;
ufield->exp = NULL;
dfield_set_null(&ufield->new_val);
So it can't be break prematurely.
[27 Jun 2013 19:57]
Po-Chun Chang
Hi Xinjun, There are two loops over "foreign->n_fields" in method "row_ins_foreign_check_on_constraint". You are talking about the first loop, at line 1174 (http://bazaar.launchpad.net/~mysql/mysql-server/5.7/view/head:/storage/innobase/row/row0in...), which indeed can't break prematurely. However, I am talking about the second loop, at line 1197 (http://bazaar.launchpad.net/~mysql/mysql-server/5.7/view/head:/storage/innobase/row/row0in...), 20 lines below the first loop. This second loop does not have any work before the assignment and it can break prematurely, like I described in the bug report and in the attached patch.
Description: The problem appears in version 5.6 and in the latest revision 5216 in version 5.7. I have attached a simple one-line patch (patch.diff) that fixes it. In method "row_ins_foreign_check_on_constraint" in row0ins.cc, the loop on line 1197 should break immediately after "fts_col_affacted" is set to "TRUE". All the iterations after "fts_col_affacted" is set to "TRUE" do not perform any useful work, at best they just set "fts_col_affacted" again to "TRUE". How to repeat: Once row_ins_foreign_check_on_constraint() is executed. Suggested fix: === modified file 'storage/innobase/row/row0ins.cc' --- storage/innobase/row/row0ins.cc 2013-01-29 06:45:46 +0000 +++ storage/innobase/row/row0ins.cc 2013-06-01 23:29:00 +0000 @@ -1200,6 +1200,7 @@ dict_index_get_nth_col_no(index, i)) != ULINT_UNDEFINED) { fts_col_affacted = TRUE; + break; } }