Commit 6ae3bca9 authored by kostja@bodhi.(none)'s avatar kostja@bodhi.(none)
Browse files

Bug#27430 "Crash in subquery code when in PS and table DDL changed after 

 PREPARE", review fixes:
- make the patch follow the specification of WL#4166 and remove  
the new error that was originally introduced.
Now the client never gets an error from reprepare, unless it failed.
I.e. even if the statement at hand returns a completely different
result set, this is not considered a server error.
The C API library, that can not handle this situation, was modified to
return a client error.
Added additional test coverage.
parent 2c0ce2a8

include/errmsg.h

+2 −1
Original line number Diff line number Diff line
@@ -96,6 +96,7 @@ extern const char *client_errors[]; /* Error messages */ 
#define CR_NOT_IMPLEMENTED                      2054

#define CR_SERVER_LOST_EXTENDED			2055

#define CR_STMT_CLOSED				2056

#define CR_ERROR_LAST  /*Copy last error nr:*/  2056

#define CR_NEW_STMT_METADATA                    2057

#define CR_ERROR_LAST  /*Copy last error nr:*/  2057

/* Add error numbers before CR_ERROR_LAST and change it accordingly. */

include/mysql_com.h

+22 −2
Original line number Diff line number Diff line
@@ -184,19 +184,38 @@ enum enum_server_command 
#define SERVER_MORE_RESULTS_EXISTS 8    /* Multi query - next query exists */

#define SERVER_QUERY_NO_GOOD_INDEX_USED 16

#define SERVER_QUERY_NO_INDEX_USED      32

/*

/**

  The server was able to fulfill the clients request and opened a

  read-only non-scrollable cursor for a query. This flag comes

  in reply to COM_STMT_EXECUTE and COM_STMT_FETCH commands.

*/

#define SERVER_STATUS_CURSOR_EXISTS 64

/*

/**

  This flag is sent when a read-only cursor is exhausted, in reply to

  COM_STMT_FETCH command.

*/

#define SERVER_STATUS_LAST_ROW_SENT 128

#define SERVER_STATUS_DB_DROPPED        256 /* A database was dropped */

#define SERVER_STATUS_NO_BACKSLASH_ESCAPES 512

/**

  Sent to the client if after a prepared statement reprepare

  we discovered that the new statement returns a different 

  number of result set columns.

*/

#define SERVER_STATUS_METADATA_CHANGED 1024



/**

  Server status flags that must be cleared when starting

  execution of a new SQL statement.

  Flags from this set are only added to the

  current server status by the execution engine, but 

  never removed -- the execution engine expects them 

  to disappear automagically by the next command.

*/

#define SERVER_STATUS_CLEAR_SET (SERVER_QUERY_NO_GOOD_INDEX_USED| \

                                 SERVER_QUERY_NO_INDEX_USED|\

                                 SERVER_MORE_RESULTS_EXISTS|\

                                 SERVER_STATUS_METADATA_CHANGED)



#define MYSQL_ERRMSG_SIZE	512

#define NET_READ_TIMEOUT	30		/* Timeout on read */
@@ -205,6 +224,7 @@ enum enum_server_command 


#define ONLY_KILL_QUERY         1





struct st_vio;					/* Only C */

typedef struct st_vio Vio;

libmysql/errmsg.c

+3 −0
Original line number Diff line number Diff line
@@ -84,6 +84,7 @@ const char *client_errors[]= 
  "This feature is not implemented yet",

  "Lost connection to MySQL server at '%s', system error: %d",

  "Statement closed indirectly because of a preceeding %s() call",

  "The number of columns in the result set differs from the number of bound buffers. You must reset the statement, rebind the result set columns, and execute the statement again",

  ""

};
@@ -149,6 +150,7 @@ const char *client_errors[]= 
  "This feature is not implemented yet",

  "Lost connection to MySQL server at '%s', system error: %d",

  "Statement closed indirectly because of a preceeding %s() call",

  "The number of columns in the result set differs from the number of bound buffers. You must reset the statement, rebind the result set columns, and execute the statement again",

  ""

};
@@ -212,6 +214,7 @@ const char *client_errors[]= 
  "This feature is not implemented yet",

  "Lost connection to MySQL server at '%s', system error: %d",

  "Statement closed indirectly because of a preceeding %s() call",

  "The number of columns in the result set differs from the number of bound buffers. You must reset the statement, rebind the result set columns, and execute the statement again",

  ""

};

#endif

libmysql/libmysql.c

+88 −43
Original line number Diff line number Diff line
@@ -1706,6 +1706,7 @@ static my_bool setup_one_fetch_function(MYSQL_BIND *, MYSQL_FIELD *field); 
#define RESET_SERVER_SIDE 1

#define RESET_LONG_DATA 2

#define RESET_STORE_RESULT 4

#define RESET_CLEAR_ERROR 8



static my_bool reset_stmt_handle(MYSQL_STMT *stmt, uint flags);
@@ -2090,7 +2091,7 @@ mysql_stmt_prepare(MYSQL_STMT *stmt, const char *query, ulong length) 
  To be removed when all commands will fully support prepared mode.

*/



static unsigned int alloc_stmt_fields(MYSQL_STMT *stmt)

static void alloc_stmt_fields(MYSQL_STMT *stmt)

{

  MYSQL_FIELD *fields, *field, *end;

  MEM_ROOT *alloc= &stmt->mem_root;
@@ -2108,7 +2109,10 @@ static unsigned int alloc_stmt_fields(MYSQL_STMT *stmt) 
      !(stmt->bind= (MYSQL_BIND *) alloc_root(alloc,

					      sizeof(MYSQL_BIND) *

					      stmt->field_count)))

    return 0;

  {

    set_stmt_error(stmt, CR_OUT_OF_MEMORY, unknown_sqlstate, NULL);

    return;

  }



  for (fields= mysql->fields, end= fields+stmt->field_count,

	 field= stmt->fields;
@@ -2127,13 +2131,15 @@ static unsigned int alloc_stmt_fields(MYSQL_STMT *stmt) 
    field->def      = fields->def ? strdup_root(alloc,fields->def): 0;

    field->max_length= 0;

  }

  return stmt->field_count;

}





/*

/**

  Update result set columns metadata if it was sent again in

  reply to COM_STMT_EXECUTE.



  @note If the new field count is different from the original one,

        an error is set and no update is performed.

*/



static void update_stmt_fields(MYSQL_STMT *stmt)
@@ -2143,7 +2149,22 @@ static void update_stmt_fields(MYSQL_STMT *stmt) 
  MYSQL_FIELD *stmt_field= stmt->fields;

  MYSQL_BIND *my_bind= stmt->bind_result_done ? stmt->bind : 0;



  DBUG_ASSERT(stmt->field_count == stmt->mysql->field_count);

  if (stmt->field_count != stmt->mysql->field_count)

  {

    /*

      The tables used in the statement were altered,

      and the query now returns a different number of columns.

      There is no way to continue without reallocating the bind

      array:

      - if the number of columns increased, mysql_stmt_fetch()

      will write beyond allocated memory

      - if the number of columns decreased, some user-bound

      buffers will be left unassigned without user knowing

      that.

    */

    set_stmt_error(stmt, CR_NEW_STMT_METADATA, unknown_sqlstate, NULL);

    return;

  }



  for (; field < field_end; ++field, ++stmt_field)

  {
@@ -2792,6 +2813,50 @@ my_bool STDCALL mysql_stmt_attr_get(MYSQL_STMT *stmt, 
}





/**

  Update statement result set metadata from with the new field

  information sent during statement execute.



  @pre mysql->field_count is not zero



  @retval TRUE   if error: out of memory or the new

                 result set has a different number of columns

  @retval FALSE  success

*/



static void reinit_result_set_metadata(MYSQL_STMT *stmt)

{

  /* Server has sent result set metadata */

  if (stmt->field_count == 0)

  {

    /*

      This is 'SHOW'/'EXPLAIN'-like query. Current implementation of

      prepared statements can't send result set metadata for these queries

      on prepare stage. Read it now.

    */

    alloc_stmt_fields(stmt);

  }

  else

  {

    /*

      Update result set metadata if it for some reason changed between

      prepare and execute, i.e.:

      - in case of 'SELECT ?' we don't know column type unless data was

      supplied to mysql_stmt_execute, so updated column type is sent

      now.

      - if data dictionary changed between prepare and execute, for

      example a table used in the query was altered.

      Note, that now (4.1.3) we always send metadata in reply to

      COM_STMT_EXECUTE (even if it is not necessary), so either this or

      previous branch always works.

      TODO: send metadata only when it's really necessary and add a warning

      'Metadata changed' when it's sent twice.

      */

    update_stmt_fields(stmt);

  }

}





/*

  Send placeholders data to server (if there are placeholders)

  and execute prepared statement.
@@ -2847,7 +2912,7 @@ int STDCALL mysql_stmt_execute(MYSQL_STMT *stmt) 
    DBUG_RETURN(1);

  }



  if (reset_stmt_handle(stmt, RESET_STORE_RESULT))

  if (reset_stmt_handle(stmt, RESET_STORE_RESULT | RESET_CLEAR_ERROR))

    DBUG_RETURN(1);

  /*

    No need to check for stmt->state: if the statement wasn't
@@ -2855,40 +2920,10 @@ int STDCALL mysql_stmt_execute(MYSQL_STMT *stmt) 
  */

  if (mysql->methods->stmt_execute(stmt))

    DBUG_RETURN(1);

  if (mysql->field_count)

  {

    /* Server has sent result set metadata */

    if (stmt->field_count == 0)

    {

      /*

        This is 'SHOW'/'EXPLAIN'-like query. Current implementation of

        prepared statements can't send result set metadata for these queries

        on prepare stage. Read it now.

      */

      alloc_stmt_fields(stmt);

    }

    else

    {

      /*

        Update result set metadata if it for some reason changed between

        prepare and execute, i.e.:

        - in case of 'SELECT ?' we don't know column type unless data was

          supplied to mysql_stmt_execute, so updated column type is sent

          now.

        - if data dictionary changed between prepare and execute, for

          example a table used in the query was altered.

        Note, that now (4.1.3) we always send metadata in reply to

        COM_STMT_EXECUTE (even if it is not necessary), so either this or

        previous branch always works.

        TODO: send metadata only when it's really necessary and add a warning

        'Metadata changed' when it's sent twice.

      */

      update_stmt_fields(stmt);

    }

  }

  stmt->state= MYSQL_STMT_EXECUTE_DONE;

  if (stmt->field_count)

  if (mysql->field_count)

  {

    reinit_result_set_metadata(stmt);

    if (stmt->server_status & SERVER_STATUS_CURSOR_EXISTS)

    {

      mysql->status= MYSQL_STATUS_READY;
@@ -2903,7 +2938,7 @@ int STDCALL mysql_stmt_execute(MYSQL_STMT *stmt) 
        network or b) is more efficient if all (few) result set rows are

        precached on client and server's resources are freed.

      */

      DBUG_RETURN(mysql_stmt_store_result(stmt));

      mysql_stmt_store_result(stmt);

    }

    else

    {
@@ -2912,7 +2947,7 @@ int STDCALL mysql_stmt_execute(MYSQL_STMT *stmt) 
      stmt->read_row_func= stmt_read_row_unbuffered;

    }

  }

  DBUG_RETURN(0);

  DBUG_RETURN(test(stmt->last_errno));

}
@@ -4766,6 +4801,12 @@ int STDCALL mysql_stmt_store_result(MYSQL_STMT *stmt) 
    DBUG_RETURN(1);

  }



  if (stmt->last_errno)

  {

    /* An attempt to use an invalid statement handle. */

    DBUG_RETURN(1);

  }



  if (mysql->status == MYSQL_STATUS_READY &&

      stmt->server_status & SERVER_STATUS_CURSOR_EXISTS)

  {
@@ -4973,9 +5014,10 @@ static my_bool reset_stmt_handle(MYSQL_STMT *stmt, uint flags) 
          stmt->state= MYSQL_STMT_INIT_DONE;

          return 1;

        }

        stmt_clear_error(stmt);

      }

    }

    if (flags & RESET_CLEAR_ERROR)

      stmt_clear_error(stmt);

    stmt->state= MYSQL_STMT_PREPARE_DONE;

  }

  return 0;
@@ -4986,7 +5028,8 @@ my_bool STDCALL mysql_stmt_free_result(MYSQL_STMT *stmt) 
  DBUG_ENTER("mysql_stmt_free_result");



  /* Free the client side and close the server side cursor if there is one */

  DBUG_RETURN(reset_stmt_handle(stmt, RESET_LONG_DATA | RESET_STORE_RESULT));

  DBUG_RETURN(reset_stmt_handle(stmt, RESET_LONG_DATA | RESET_STORE_RESULT |

                                RESET_CLEAR_ERROR));

}



/********************************************************************
@@ -5067,7 +5110,9 @@ my_bool STDCALL mysql_stmt_reset(MYSQL_STMT *stmt) 
    DBUG_RETURN(1);

  }

  /* Reset the client and server sides of the prepared statement */

  DBUG_RETURN(reset_stmt_handle(stmt, RESET_SERVER_SIDE | RESET_LONG_DATA));

  DBUG_RETURN(reset_stmt_handle(stmt,

                                RESET_SERVER_SIDE | RESET_LONG_DATA |

                                RESET_CLEAR_ERROR));

}



/*

sql/share/errmsg.txt

+0 −3
Original line number Diff line number Diff line
@@ -6130,6 +6130,3 @@ ER_LOG_PURGE_NO_FILE 


ER_NEED_REPREPARE

  eng "Prepared statement needs to be re-prepared"
 


ER_PS_REBIND

  eng "Prepared statement result set has changed, a rebind needed"