List:Commits« Previous MessageNext Message »
From:Alexander Nozdrin Date:August 30 2007 4:06pm
Subject:bk commit into 5.1 tree (anozdrin:1.2587) BUG#25843
View as plain text  
Below is the list of changes that have just been committed into a local
5.1 repository of alik. When alik does a push these changes will
be propagated to the main repository and, within 24 hours after the
push, to the public repository.
For information on how to access the public repository
see http://dev.mysql.com/doc/mysql/en/installing-source-tree.html

ChangeSet@stripped, 2007-08-30 20:06:19+04:00, anozdrin@stripped +3 -0
  Cleanup-patch for BUG#25843: changing default database between
  PREPARE and EXECUTE of statement breaks binlog.

  sql/sp.cc@stripped, 2007-08-30 20:06:17+04:00, anozdrin@stripped +32 -25
    - Polishing sp_use_new_db():
      - renamed no_access_check to force_switch to be more adequate;
      - fixed comment;

  sql/sql_class.h@stripped, 2007-08-30 20:06:17+04:00, anozdrin@stripped +34 -5
    Polishing: fixed comment.

  sql/sql_db.cc@stripped, 2007-08-30 20:06:17+04:00, anozdrin@stripped +65 -22
    1. Use mysql_change_db_impl() to reset current database instead of
    THD::set_db() in mysql_rm_db(). THD::set_db() does not take care of
    THD::db_access and database attributes (@@collation_database).
    2. Polishing: add, fix comments.

diff -Nrup a/sql/sp.cc b/sql/sp.cc
--- a/sql/sp.cc	2007-08-15 17:42:56 +04:00
+++ b/sql/sp.cc	2007-08-30 20:06:17 +04:00
@@ -574,7 +574,7 @@ db_load_routine(THD *thd, int type, sp_n
     we'll update it later in switch_query_ctx().
   */
 
-  if ((ret= sp_use_new_db(thd, name->m_db, &old_db, 1, &dbchanged)))
+  if ((ret= sp_use_new_db(thd, name->m_db, &old_db, TRUE, &dbchanged)))
     goto end;
 
   thd->spcont= NULL;
@@ -2027,34 +2027,41 @@ create_string(THD *thd, String *buf,
 
 
 
-/*
+/**
   Change the current database if needed.
 
-  SYNOPSIS
-    sp_use_new_db()
-      thd            thread handle
-      new_db         new database name (a string and its length)
-      old_db         [IN] str points to a buffer where to store the old
-                          database, length contains the size of the buffer
-                     [OUT] if old db was not NULL, its name is copied
-                     to the buffer pointed at by str and length is updated
-                     accordingly. Otherwise str[0] is set to '\0' and length
-                     is set to 0. The out parameter should be used only if
-                     the database name has been changed (see dbchangedp).
-     dbchangedp      [OUT] is set to TRUE if the current database is changed,
-                     FALSE otherwise. A database is not changed if the old
-                     name is the same as the new one, both names are empty,
-                     or an error has occurred.
-
-  RETURN VALUE
-    0                success
-    1                access denied or out of memory (the error message is
-                     set in THD)
+  @param[in]      thd             thread handle
+  @param[in]      new_db          new database name
+  @param[in, out] old_db          IN: str points to a buffer where to store
+                                  the old database, length contains the
+                                  size of the buffer
+                                  OUT: if old db was not NULL, its name is
+                                  copied to the buffer pointed at by str
+                                  and length is updated accordingly.
+                                  Otherwise str[0] is set to '\0' and
+                                  length is set to 0. The out parameter
+                                  should be used only if the database name
+                                  has been changed (see dbchangedp).
+  @param[in]      force_switch    Flag to mysql_change_db(). For more information,
+                                  see mysql_change_db() comment.
+  @param[out]     dbchangedp      is set to TRUE if the current database is
+                                  changed, FALSE otherwise. The current
+                                  database is not changed if the old name
+                                  is equal to the new one, both names are
+                                  empty, or an error has occurred.
+
+  @return Operation status.
+    @retval 0 on success
+    @retval 1 access denied or out of memory
+              (the error message is set in THD)
 */
 
 int
-sp_use_new_db(THD *thd, LEX_STRING new_db, LEX_STRING *old_db,
-	      bool no_access_check, bool *dbchangedp)
+sp_use_new_db(THD *thd,
+              LEX_STRING new_db,
+              LEX_STRING *old_db,
+              bool force_switch,
+              bool *dbchangedp)
 {
   int ret;
   DBUG_ENTER("sp_use_new_db");
@@ -2085,7 +2092,7 @@ sp_use_new_db(THD *thd, LEX_STRING new_d
     DBUG_RETURN(0);
   }
 
-  ret= mysql_change_db(thd, &new_db, no_access_check);
+  ret= mysql_change_db(thd, &new_db, force_switch);
 
   *dbchangedp= ret == 0;
   DBUG_RETURN(ret);
diff -Nrup a/sql/sql_class.h b/sql/sql_class.h
--- a/sql/sql_class.h	2007-08-17 12:18:54 +04:00
+++ b/sql/sql_class.h	2007-08-30 20:06:17 +04:00
@@ -1802,11 +1802,27 @@ public:
     }
   }
 
-  /*
-    Initialize the current database from a NULL-terminated string with length
-    If we run out of memory, we free the current database and return TRUE.
-    This way the user will notice the error as there will be no current
-    database selected (in addition to the error message set by malloc).
+  /**
+    Set the current database; use deep copy of C-string.
+
+    @param new_db     a pointer to the new database name.
+    @param new_db_len length of the new database name.
+
+    Initialize the current database from a NULL-terminated string with
+    length. If we run out of memory, we free the current database and
+    return TRUE.  This way the user will notice the error as there will be
+    no current database selected (in addition to the error message set by
+    malloc).
+
+    @note This operation just sets {thd->db, thd->db_length}. Switching the
+    current database usually involves other actions, like switching other
+    database attributes including security context. In the future, this
+    operation will be made private and more convenient interface will be
+    provided.
+
+    @return Operation status
+      @retval FALSE Success
+      @retval TRUE  Out-of-memory error
   */
   bool set_db(const char *new_db, size_t new_db_len)
   {
@@ -1821,6 +1837,19 @@ public:
     db_length= db ? new_db_len : 0;
     return new_db && !db;
   }
+
+  /**
+    Set the current database; use shallow copy of C-string.
+
+    @param new_db     a pointer to the new database name.
+    @param new_db_len length of the new database name.
+
+    @note This operation just sets {thd->db, thd->db_length}. Switching the
+    current database usually involves other actions, like switching other
+    database attributes including security context. In the future, this
+    operation will be made private and more convenient interface will be
+    provided.
+  */
   void reset_db(char *new_db, size_t new_db_len)
   {
     db= new_db;
diff -Nrup a/sql/sql_db.cc b/sql/sql_db.cc
--- a/sql/sql_db.cc	2007-08-15 17:42:58 +04:00
+++ b/sql/sql_db.cc	2007-08-30 20:06:17 +04:00
@@ -39,6 +39,10 @@ static long mysql_rm_known_files(THD *th
          
 static long mysql_rm_arc_files(THD *thd, MY_DIR *dirp, const char *org_path);
 static my_bool rm_dir_w_symlink(const char *org_path, my_bool send_error);
+static void mysql_change_db_impl(THD *thd,
+                                 LEX_STRING *new_db_name,
+                                 ulong new_db_access,
+                                 CHARSET_INFO *new_db_charset);
 
 
 /* Database lock hash */
@@ -988,7 +992,7 @@ exit:
     it to 0.
   */
   if (thd->db && !strcmp(thd->db, db))
-    thd->set_db(NULL, 0);
+    mysql_change_db_impl(thd, NULL, 0, thd->variables.collation_server);
   VOID(pthread_mutex_unlock(&LOCK_mysql_create_db));
   start_waiting_global_read_lock(thd);
 exit2:
@@ -1347,21 +1351,48 @@ static void mysql_change_db_impl(THD *th
 
 
 /**
-  @brief Change the current database.
+  @brief Change the current database and its attributes.
 
   @param thd          thread handle
   @param new_db_name  database name
-  @param force_switch if this flag is set (TRUE), mysql_change_db() will
-                      switch to NULL db if the specified database is not
-                      available anymore. Corresponding warning will be
-                      thrown in this case. This flag is used to change
-                      database in stored-routine-execution code.
-
-  @details Check that the database name corresponds to a valid and existent
-  database, check access rights (unless called with no_access_check), and
-  set the current database. This function is called to change the current
-  database upon user request (COM_CHANGE_DB command) or temporarily, to
-  execute a stored routine.
+  @param force_switch if force_switch is FALSE, then the operation will fail if
+
+                        - new_db_name is NULL or empty;
+
+                        - OR new database name is invalid
+                          (check_db_name() failed);
+
+                        - OR user has no privilege on the new database;
+
+                        - OR new database does not exist;
+
+                      if force_switch is TRUE, then
+
+                        - if new_db_name is NULL or empty, the current
+                          database will be NULL, @@collation_database will
+                          be set to @@collation_server, the operation will
+                          succeed.
+
+                        - if new database name is invalid
+                          (check_db_name() failed), the current database
+                          will be NULL, @@collation_database will be set to
+                          @@collation_server, but the operation will fail;
+
+                        - user privileges will not be checked
+                          (THD::db_access however is updated);
+
+                          TODO: is this really the intention?
+                                (see sp-security.test).
+
+                        - if new database does not exist,the current database
+                          will be NULL, @@collation_database will be set to
+                          @@collation_server, a warning will be thrown, the
+                          operation will succeed.
+
+  @details The function checks that the database name corresponds to a
+  valid and existent database, checks access rights and changes the current
+  database with database attributes (@@collation_database session variable,
+  THD::db_access).
 
   This function is not the only way to switch the database that is
   currently employed. When the replication slave thread switches the
@@ -1398,8 +1429,13 @@ bool mysql_change_db(THD *thd, const LEX
     if (force_switch)
     {
       /*
-        This can only happen when we restore the old db in THD after
-        execution of a routine is complete. Change db to NULL.
+        This can happen only if we're switching the current database back
+        after loading stored program. The thing is that loading of stored
+        program can happen when there is no current database.
+
+        TODO: actually, new_db_name and new_db_name->str seem to be always
+        non-NULL. In case of stored program, new_db_name->str == "" and
+        new_db_name->length == 0.
       */
 
       mysql_change_db_impl(thd, NULL, 0, thd->variables.collation_server);
@@ -1417,7 +1453,7 @@ bool mysql_change_db(THD *thd, const LEX
   if (my_strcasecmp(system_charset_info, new_db_name->str,
                     INFORMATION_SCHEMA_NAME.str) == 0)
   {
-    /* Switch database to INFORMATION_SCHEMA. */
+    /* Switch the current database to INFORMATION_SCHEMA. */
 
     mysql_change_db_impl(thd, &INFORMATION_SCHEMA_NAME, SELECT_ACL,
                          system_charset_info);
@@ -1444,8 +1480,8 @@ bool mysql_change_db(THD *thd, const LEX
     even if we are called from sp_head::execute().
 
     It's next to impossible however to get this error when we are called
-    from sp_head::execute(). But let's switch database to NULL in this case
-    to be sure.
+    from sp_head::execute(). But let's switch the current database to NULL
+    in this case to be sure.
   */
 
   if (check_db_name(&new_db_file_name))
@@ -1454,10 +1490,8 @@ bool mysql_change_db(THD *thd, const LEX
     my_free(new_db_file_name.str, MYF(0));
 
     if (force_switch)
-    {
-      /* Change db to NULL. */
       mysql_change_db_impl(thd, NULL, 0, thd->variables.collation_server);
-    }
+
     DBUG_RETURN(TRUE);
   }
 
@@ -1492,6 +1526,8 @@ bool mysql_change_db(THD *thd, const LEX
   {
     if (force_switch)
     {
+      /* Throw a warning and free new_db_file_name. */
+
       push_warning_printf(thd, MYSQL_ERROR::WARN_LEVEL_NOTE,
                           ER_BAD_DB_ERROR, ER(ER_BAD_DB_ERROR),
                           new_db_file_name.str);
@@ -1502,12 +1538,19 @@ bool mysql_change_db(THD *thd, const LEX
 
       mysql_change_db_impl(thd, NULL, 0, thd->variables.collation_server);
 
+      /* The operation succeed. */
+
       DBUG_RETURN(FALSE);
     }
     else
     {
+      /* Report an error and free new_db_file_name. */
+
       my_error(ER_BAD_DB_ERROR, MYF(0), new_db_file_name.str);
       my_free(new_db_file_name.str, MYF(0));
+
+      /* The operation failed. */
+
       DBUG_RETURN(TRUE);
     }
   }
@@ -1820,7 +1863,7 @@ bool mysql_rename_db(THD *thd, LEX_STRIN
 
   /* Step9: Let's do "use newdb" if we renamed the current database */
   if (change_to_newdb)
-    error|= mysql_change_db(thd, new_db, 0);
+    error|= mysql_change_db(thd, new_db, FALSE);
 
 exit:
   pthread_mutex_lock(&LOCK_lock_db);
Thread
bk commit into 5.1 tree (anozdrin:1.2587) BUG#25843Alexander Nozdrin30 Aug