List:Commits« Previous MessageNext Message »
From:Alexander Nozdrin Date:February 25 2009 10:03am
Subject:bzr commit into mysql-6.0 branch (alik:2732) Bug#39519
View as plain text  
#At file:///mnt/raid/alik/MySQL/bzr/bug39519/6.0-rt-bug39519/ based on revid:alik@stripped

 2732 Alexander Nozdrin	2009-02-25
      Bug#39519: mysql_stmt_close() should flush all data
      associated with the statement.
      modified:
        include/mysql.h
        include/mysql.h.pp
        libmysql/libmysql.c
        libmysqld/lib_sql.cc
        sql-common/client.c
        tests/mysql_client_test.c

per-file messages:
  include/mysql.h
    Add a new flag to MYSQL_METHODS::flush_use_result
    function pointer. This flag determines if all results
    should be flushed or only the first one:
    
      - if flush_all_results is TRUE, then cli_flush_use_result()
        will read/flush all pending results. I.e. it will read
        all packets while server status attribute indicates that
        there are more results. This is a new semantic, required
        to fix the bug.
            
      - if flush_all_results is FALSE, the old sematic
        is preserved -- i.e. cli_flush_use_result() reads data
        until first EOF-packet.
  include/mysql.h.pp
    Update API.
  libmysql/libmysql.c
    Flush all results in mysql_stmt_close().
  sql-common/client.c
    Implement new functionality in cli_flush_use_result():
    if flush_all_delete is TRUE, then it should read/flush
    all pending results.
    
    In other words, it should read all packets while server
    status attribute indicates that there are more results.
  tests/mysql_client_test.c
    Add a test case for Bug#39519 / WL#4435.
=== modified file 'include/mysql.h'
--- a/include/mysql.h	2008-12-05 01:05:05 +0000
+++ b/include/mysql.h	2009-02-25 10:03:18 +0000
@@ -654,7 +654,7 @@ typedef struct st_mysql_methods
   MYSQL_RES * (*use_result)(MYSQL *mysql);
   void (*fetch_lengths)(unsigned long *to, 
 			MYSQL_ROW column, unsigned int field_count);
-  void (*flush_use_result)(MYSQL *mysql);
+  void (*flush_use_result)(MYSQL *mysql, my_bool flush_all_results);
 #if !defined(MYSQL_SERVER) || defined(EMBEDDED_LIBRARY)
   MYSQL_FIELD * (*list_fields)(MYSQL *mysql);
   my_bool (*read_prepare_result)(MYSQL *mysql, MYSQL_STMT *stmt);

=== modified file 'include/mysql.h.pp'
--- a/include/mysql.h.pp	2008-12-13 11:02:16 +0000
+++ b/include/mysql.h.pp	2009-02-25 10:03:18 +0000
@@ -558,7 +558,7 @@ typedef struct st_mysql_methods
   MYSQL_RES * (*use_result)(MYSQL *mysql);
   void (*fetch_lengths)(unsigned long *to,
    MYSQL_ROW column, unsigned int field_count);
-  void (*flush_use_result)(MYSQL *mysql);
+  void (*flush_use_result)(MYSQL *mysql, my_bool flush_all_results);
   MYSQL_FIELD * (*list_fields)(MYSQL *mysql);
   my_bool (*read_prepare_result)(MYSQL *mysql, MYSQL_STMT *stmt);
   int (*stmt_execute)(MYSQL_STMT *stmt);

=== modified file 'libmysql/libmysql.c'
--- a/libmysql/libmysql.c	2009-02-14 09:18:41 +0000
+++ b/libmysql/libmysql.c	2009-02-25 10:03:18 +0000
@@ -4608,7 +4608,7 @@ static my_bool reset_stmt_handle(MYSQL_S
         if (stmt->field_count && mysql->status != MYSQL_STATUS_READY)
         {
           /* There is a result set and it belongs to this statement */
-          (*mysql->methods->flush_use_result)(mysql);
+          (*mysql->methods->flush_use_result)(mysql, FALSE);
           if (mysql->unbuffered_fetch_owner)
             *mysql->unbuffered_fetch_owner= TRUE;
           mysql->status= MYSQL_STATUS_READY;
@@ -4692,7 +4692,7 @@ my_bool STDCALL mysql_stmt_close(MYSQL_S
           Flush result set of the connection. If it does not belong
           to this statement, set a warning.
         */
-        (*mysql->methods->flush_use_result)(mysql);
+        (*mysql->methods->flush_use_result)(mysql, TRUE);
         if (mysql->unbuffered_fetch_owner)
           *mysql->unbuffered_fetch_owner= TRUE;
         mysql->status= MYSQL_STATUS_READY;

=== modified file 'libmysqld/lib_sql.cc'
--- a/libmysqld/lib_sql.cc	2009-01-13 15:26:20 +0000
+++ b/libmysqld/lib_sql.cc	2009-02-25 10:03:18 +0000
@@ -139,7 +139,7 @@ emb_advanced_command(MYSQL *mysql, enum 
   return result;
 }
 
-static void emb_flush_use_result(MYSQL *mysql)
+static void emb_flush_use_result(MYSQL *mysql, my_bool)
 {
   THD *thd= (THD*) mysql->thd;
   if (thd->cur_data)

=== modified file 'sql-common/client.c'
--- a/sql-common/client.c	2008-12-24 10:48:24 +0000
+++ b/sql-common/client.c	2009-02-25 10:03:18 +0000
@@ -655,10 +655,14 @@ err:
 }
 #endif
 
-/*****************************************************************************
+/**
   Read a packet from server. Give error message if socket was down
   or packet is an error message
-*****************************************************************************/
+
+  @retval  packet_error    An error occurred during reading.
+                           Error message is set.
+  @retval  
+*/
 
 ulong
 cli_safe_read(MYSQL *mysql)
@@ -824,31 +828,132 @@ void free_old_query(MYSQL *mysql)
   DBUG_VOID_RETURN;
 }
 
+
+/**
+  Finish reading of a partial result set from the server.
+  Get the EOF packet, and update mysql->status
+  and mysql->warning_count.
+
+  @return  TRUE if a communication or protocol error, an error
+           is set in this case, FALSE otherwise.
+*/
+
+my_bool flush_one_result(MYSQL *mysql)
+{
+  ulong packet_length;
+
+  DBUG_ASSERT(mysql->status != MYSQL_STATUS_READY);
+
+  do
+  {
+    packet_length= cli_safe_read(mysql);
+    /*
+      There is an error reading from the connection,
+      or (sic!) there were no error and no
+      data in the stream, i.e. no more data from the server.
+      Since we know our position in the stream (somewhere in
+      the middle of a result set), this latter case is an error too
+      -- each result set must end with a EOF packet.
+      cli_safe_read() has set an error for us, just return.
+    */
+    if (packet_length == packet_error)
+      return TRUE;
+  }
+  while (packet_length > 8 || mysql->net.read_pos[0] != 254);
+
+  /* Analyze EOF packet of the result set. */
+
+  if (protocol_41(mysql))
+  {
+    char *pos= (char*) mysql->net.read_pos + 1;
+    mysql->warning_count=uint2korr(pos);
+    pos+=2;
+    mysql->server_status=uint2korr(pos);
+    pos+=2;
+  }
+  return FALSE;
+}
+
+
+/**
+  Read a packet from network. If it's an OK packet, flush it.
+
+  @return  TRUE if error, FALSE otherwise. In case of 
+           success, is_ok_packet is set to TRUE or FALSE,
+           based on what we got from network.
+*/
+
+my_bool opt_flush_ok_packet(MYSQL *mysql, my_bool *is_ok_packet)
+{
+  ulong packet_length= cli_safe_read(mysql);
+
+  if (packet_length == packet_error)
+    return TRUE;
+
+  /* cli_safe_read always reads a non-empty packet. */
+  DBUG_ASSERT(packet_length);
+
+  *is_ok_packet= mysql->net.read_pos[0] == 0;
+  if (*is_ok_packet)
+  {
+    uchar *pos= mysql->net.read_pos + 1;
+
+    net_field_length_ll(&pos); /* affected rows */
+    net_field_length_ll(&pos); /* insert id */
+
+    mysql->server_status=uint2korr(pos);
+    pos+=2;
+
+    if (protocol_41(mysql))
+    {
+      mysql->warning_count=uint2korr(pos);
+      pos+=2;
+    }
+  }
+  return FALSE;
+}
+
+
 /*
   Flush result set sent from server
 */
 
-static void cli_flush_use_result(MYSQL *mysql)
+static void cli_flush_use_result(MYSQL *mysql, my_bool flush_all_results)
 {
   /* Clear the current execution status */
   DBUG_ENTER("cli_flush_use_result");
   DBUG_PRINT("warning",("Not all packets read, clearing them"));
-  for (;;)
+
+  if (flush_one_result(mysql))
+    DBUG_VOID_RETURN;                           /* An error occurred */
+
+  if (! flush_all_results)
+    DBUG_VOID_RETURN;
+
+  while (mysql->server_status & SERVER_MORE_RESULTS_EXISTS)
   {
-    ulong pkt_len;
-    if ((pkt_len=cli_safe_read(mysql)) == packet_error)
-      break;
-    if (pkt_len <= 8 && mysql->net.read_pos[0] == 254)
+    my_bool is_ok_packet;
+    if (opt_flush_ok_packet(mysql, &is_ok_packet))
+      DBUG_VOID_RETURN;                         /* An error occurred. */
+    if (is_ok_packet)
     {
-      if (protocol_41(mysql))
-      {
-        char *pos= (char*) mysql->net.read_pos + 1;
-        mysql->warning_count=uint2korr(pos); pos+=2;
-        mysql->server_status=uint2korr(pos); pos+=2;
-      }
-      break;                            /* End of data */
+      /*
+        Indeed what we got from network was an OK packet, and we
+        know that OK is the last one in a multi-result-set, so
+        just return.
+      */
+      DBUG_VOID_RETURN;
     }
+    /*
+      It's a result set, not an OK packet. A result set contains
+      of two result set subsequences: field metadata, terminated
+      with EOF packet, and result set data, again terminated with
+      EOF packet. Read and flush them.
+    */
+    if (flush_one_result(mysql) || flush_one_result(mysql))
+      DBUG_VOID_RETURN;                         /* An error occurred. */
   }
+
   DBUG_VOID_RETURN;
 }
 
@@ -950,7 +1055,7 @@ mysql_free_result(MYSQL_RES *result)
         mysql->unbuffered_fetch_owner= 0;
       if (mysql->status == MYSQL_STATUS_USE_RESULT)
       {
-        (*mysql->methods->flush_use_result)(mysql);
+        (*mysql->methods->flush_use_result)(mysql, FALSE);
         mysql->status=MYSQL_STATUS_READY;
         if (mysql->unbuffered_fetch_owner)
           *mysql->unbuffered_fetch_owner= TRUE;

=== modified file 'tests/mysql_client_test.c'
--- a/tests/mysql_client_test.c	2009-02-16 21:18:45 +0000
+++ b/tests/mysql_client_test.c	2009-02-25 10:03:18 +0000
@@ -2039,6 +2039,66 @@ static void test_wl4435()
   }
 }
 
+static void test_wl4435_2()
+{
+  MYSQL_STMT *stmt;
+  int  i;
+  int  rc;
+  char query[MAX_TEST_QUERY_LENGTH];
+
+  myheader("test_wl4435_2");
+  mct_start_logging("test_wl4435_2");
+
+  /*
+    Do a few iterations so that we catch any problem with incorrect
+    handling/flushing prepared statement results.
+  */
+
+  for (i= 0; i < 10; ++i)
+  {
+    /*
+      Prepare a procedure. That can be moved out of the loop, but it was
+      left in the loop for the sake of having as many statements as
+      possible.
+    */
+
+    rc= mysql_query(mysql, "DROP PROCEDURE IF EXISTS p1");
+    myquery(rc);
+
+    rc= mysql_query(mysql,
+      "CREATE PROCEDURE p1()"
+      "BEGIN "
+      "  SELECT 1; "
+      "  SELECT 2, 3 UNION SELECT 4, 5; "
+      "  SELECT 6, 7, 8; "
+      "END");
+    myquery(rc);
+
+    /* Invoke a procedure, that returns several result sets. */
+
+    strmov(query, "CALL p1()");
+    stmt= mysql_simple_prepare(mysql, query);
+    check_stmt(stmt);
+
+    /* Execute! */
+
+    rc= mysql_stmt_execute(stmt);
+    check_execute(stmt, rc);
+
+    /* Flush all the results. */
+
+    mysql_stmt_close(stmt);
+
+    /* Clean up. */
+    rc= mysql_commit(mysql);
+    myquery(rc);
+
+    rc= mysql_query(mysql, "DROP PROCEDURE p1");
+    myquery(rc);
+  }
+}
+
+
 /* Test simple prepare field results */
 
 static void test_prepare_field_result()
@@ -18957,6 +19017,7 @@ static struct my_tests_st my_tests[]= {
   { "test_bug36004", test_bug36004 },
   { "test_wl4284_1", test_wl4284_1 },
   { "test_wl4435",   test_wl4435 },
+  { "test_wl4435_2", test_wl4435_2 },
   { "test_bug38486", test_bug38486 },
   { "test_bug33831", test_bug33831 },
   { "test_bug40365", test_bug40365 },

Thread
bzr commit into mysql-6.0 branch (alik:2732) Bug#39519Alexander Nozdrin25 Feb