Bug #69186 Wasted work in method test_rewind
Submitted: 9 May 2013 15:47 Modified: 21 Jun 2013 6:45
Reporter: Po-Chun Chang (OCA) Email Updates:
Status: Not a Bug Impact on me:
None 
Category:MySQL Server: DML Severity:S5 (Performance)
Version:5.6, 5.7 OS:Any
Assigned to: CPU Architecture:Any
Tags: Contribution, patch, performance

[9 May 2013 15:47] Po-Chun Chang
Description:
The problem appears in version 5.6 and in the latest revision 5216 in
version 5.7.  I attached a simple two-line patch (patch.diff) that
fixes it.  This problem is similar to the already verified MySQL
#69118.

In method "test_rewind" in mysql_client_test.c, the loop on
"mysql_stmt_fetch(stmt)" (line 12576) should not be executed when
"opt_silent" is "true".  When "opt_silent" is "true", all the
iterations do not perform any useful work.  The patch simply flips the
"while" and "if" statements.

There are three other similar problems (loops that should not be
executed when "opt_silent" is "true") in the same file
(mysql_client_test.c) that have similar fixes (flip the "while" and
"if" statements).  I attached three more patches (patch2.diff,
patch3.diff, patch4.diff) for them.  patch3.diff and patch4.diff also
simplify the code in addition to fixing the problems.

How to repeat:
Once test_rewind() is executed.

Suggested fix:
=== modified file 'tests/mysql_client_test.c'
--- tests/mysql_client_test.c	2013-02-26 08:24:00 +0000
+++ tests/mysql_client_test.c	2013-05-07 22:57:17 +0000
@@ -12573,8 +12573,8 @@
   DIE_UNLESS(rc == 0);
 
   /* retreive all result sets till we are at the end */
-  while(!mysql_stmt_fetch(stmt))
-    if (!opt_silent)
+  if (!opt_silent)
+    while(!mysql_stmt_fetch(stmt))
       printf("fetched result:%ld\n", Data);
 
   DIE_UNLESS(rc != MYSQL_NO_DATA);
[9 May 2013 15:48] Po-Chun Chang
Suggested patch

Attachment: patch.diff (text/plain), 461 bytes.

[9 May 2013 15:48] Po-Chun Chang
Suggested patch

Attachment: patch2.diff (text/plain), 515 bytes.

[9 May 2013 15:48] Po-Chun Chang
Suggested patch

Attachment: patch3.diff (text/plain), 648 bytes.

[9 May 2013 15:48] Po-Chun Chang
Suggested patch

Attachment: patch4.diff (text/plain), 537 bytes.

[9 May 2013 17:43] MySQL Verification Team
Please submit the OCA. Thanks.
[9 May 2013 23:33] Po-Chun Chang
OCA remitted, thanks.
[14 Jun 2013 10:19] Anitha Gopi
Thanks for the contribution. Can you please upload the patch in the contributions section
[15 Jun 2013 18:24] 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 (application/octet-stream, text), 461 bytes.

[15 Jun 2013 18:24] 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: patch2.diff (application/octet-stream, text), 515 bytes.

[15 Jun 2013 18:24] 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: patch3.diff (application/octet-stream, text), 648 bytes.

[15 Jun 2013 18:25] 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: patch4.diff (application/octet-stream, text), 537 bytes.

[21 Jun 2013 6:44] Nirbhay Choubey
IMHO the reported bug and hence the patch is invalid. See,
'silent' option is normally to reduce the verbosity of a test
but in itself should not cut-down some important (execution)
steps of the test case. For example, if we look at original
test_rewind :

..
  /* retreive all result sets till we are at the end */
  while(!mysql_stmt_fetch(stmt))
    if (!opt_silent)
      printf("fetched result:%ld\n", Data);

  DIE_UNLESS(rc != MYSQL_NO_DATA);
..

The test tries to fetch all the rows and later asserts
that the final return should result in MYSQL_NO_DATA.

With the supplied patch, the test would fail with :
check failed: 'rc == MYSQL_NO_DATA'

Same applies to other contributed patches for test_bug9478,
test_bug11172.
[25 Jun 2013 11:02] Ståle Deraas
Hi Po-Chun Chang,

Your contributions were rejected due to the explanation given above by Nirbhay. But thank you anyway!

Staale