bk commit into 5.1 tree (andrey:1.2370) BUG#16992

[ahristov]

Below is the list of changes that have just been committed into a local
5.1 repository of andrey. When andrey 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
  1.2370 06/04/19 18:44:20 andrey@lmy004. +1 -0
  post-review change of fix for bug #16992 (information_schema.events troubles)

  sql/sql_show.cc
    1.325 06/04/19 18:44:12 andrey@lmy004. +149 -116
    refactor SHOW EVENTS/SELECT I_S.EVENTS code
    This is a post-review step for fix of bug #16992. The code was already
    spaghetti and some refactoring was really desired.

# This is a BitKeeper patch.  What follows are the unified diffs for the
# set of deltas contained in the patch.  The rest of the patch, the part
# that BitKeeper cares about, is below these diffs.
# User:	andrey
# Host:	lmy004.
# Root:	/work/mysql-5.1-bug16992-new

--- 1.324/sql/sql_show.cc	2006-04-19 16:31:42 +03:00
+++ 1.325/sql/sql_show.cc	2006-04-19 18:44:12 +03:00
@@ -3982,7 +3982,7 @@ extern LEX_STRING interval_type_to_name[
 */
 
 static int
-fill_events_copy_to_schema_table(THD *thd, TABLE *sch_table, TABLE *event_table)
+copy_event_to_schema_table(THD *thd, TABLE *sch_table, TABLE *event_table)
 {
   const char *wild= thd->lex->wild ? thd->lex->wild->ptr() : NullS;
   CHARSET_INFO *scs= system_charset_info;
@@ -4091,6 +4091,130 @@ fill_events_copy_to_schema_table(THD *th
 
 
 /*
+  Performs an index scan of event_table (mysql.event) and fills schema_table.
+
+  Synopsis
+    schema_events_do_index_scan()
+      thd          Thread
+      schema_table The I_S.EVENTS in memory table
+      event_table  The event table to use for loading.
+      definer      The user. Used in the prefix scanning.
+      idx_num      Which index to use.
+
+  Returns
+    0  OK
+    1  Error
+*/
+
+static
+int schema_events_do_index_scan(THD *thd, TABLE *schema_table,
+                                TABLE *event_table, char *definer, int idx_num)
+{
+  int ret=0;
+  CHARSET_INFO *scs= system_charset_info;
+  KEY *key_info;
+  uint key_len;
+  byte *key_buf= NULL;
+  LINT_INIT(key_buf);
+
+  DBUG_ENTER("schema_events_do_index_scan");
+
+  event_table->file->ha_index_init(idx_num, 1);
+  key_info= event_table->key_info + idx_num;
+  key_len= key_info->key_part[0].store_length;
+  if (idx_num == 1)
+  {
+    DBUG_PRINT("info",("Using prefix scanning on secondary KEY"));
+    DBUG_ASSERT(thd->lex->select_lex.db);
+    DBUG_PRINT("info",("Using  `%s` in the prefix", thd->lex->select_lex.db));
+    event_table->field[EVEX_FIELD_DB]->
+        store(thd->lex->select_lex.db, strlen(thd->lex->select_lex.db), scs);
+  }
+  else
+  {
+    DBUG_ASSERT(idx_num == 0);
+    DBUG_PRINT("info", ("Using prefix scanning on PK"));
+
+    event_table->field[EVEX_FIELD_DEFINER]->store(definer,strlen(definer),scs);
+
+    if (thd->lex->orig_sql_command == SQLCOM_SHOW_EVENTS)
+    {
+      DBUG_PRINT("info",("Scanning also with %s", thd->lex->select_lex.db));
+      event_table->field[EVEX_FIELD_DB]->
+            store(thd->lex->select_lex.db, strlen(thd->lex->select_lex.db),
scs);
+      key_len+= key_info->key_part[1].store_length;
+    }
+  }
+
+  if (!(key_buf= (byte *)alloc_root(thd->mem_root, key_len)))
+  {
+    ret= 1;
+    my_error(ER_OUTOFMEMORY, MYF(0));
+  }
+  else
+  {
+    key_copy(key_buf, event_table->record[0], key_info, key_len);
+    if (!(ret= event_table->file->index_read(event_table->record[0], key_buf,
+                                             key_len, HA_READ_PREFIX)))
+    {
+      DBUG_PRINT("info",("Found rows. Let's retrieve them. ret=%d", ret));
+      while (!ret)
+        if (!(ret= copy_event_to_schema_table(thd, schema_table, event_table)))
+          ret= event_table->file->index_next_same(event_table->record[0],
+                                                  key_buf, key_len);
+    }
+    DBUG_PRINT("info",("Scan finished. ret=%d", ret));
+  }
+  event_table->file->ha_index_end(); 
+  /*  ret is guaranteed to be != 0 */
+  if (ret == HA_ERR_END_OF_FILE || ret == HA_ERR_KEY_NOT_FOUND)
+    DBUG_RETURN(0);
+  DBUG_RETURN(1);
+}
+
+
+/*
+  Performs a table scan of event_table (mysql.event) and fills schema_table.
+
+  Synopsis
+    schema_events_do_index_scan()
+      thd          Thread
+      schema_table The I_S.EVENTS in memory table
+      event_table  The event table to use for loading.
+
+  Returns
+    0  OK
+    1  Error
+*/
+
+static
+int schema_events_do_table_scan(THD *thd, TABLE *schema_table,
+                                TABLE *event_table)
+{
+  int ret;
+  READ_RECORD read_record_info;
+
+  DBUG_ENTER("schema_events_do_table_scan");
+  init_read_record(&read_record_info, thd, event_table, NULL, 1, 0);
+
+  /*
+    rr_sequential, in read_record(), returns 137==HA_ERR_END_OF_FILE,
+    but rr_handle_error returns -1 for that reason. Thus, read_record()
+    returns -1 eventually.
+  */
+  if (!(ret= read_record_info.read_record(&read_record_info)))
+    while (!ret)
+      if (!(ret= copy_event_to_schema_table(thd, schema_table, event_table)))
+        ret= read_record_info.read_record(&read_record_info);
+  DBUG_PRINT("info",("Scan finished. ret=%d", ret));
+  end_read_record(&read_record_info);
+
+  /*  ret is guaranteed to be != 0 */
+  DBUG_RETURN(ret==-1? 0:1);
+}
+
+
+/*
   Fills I_S.EVENTS with data loaded from mysql.event. Also used by
   SHOW EVENTS
 
@@ -4107,18 +4231,12 @@ fill_events_copy_to_schema_table(THD *th
 
 int fill_schema_events(THD *thd, TABLE_LIST *tables, COND *cond)
 {
-  TABLE *table= tables->table;
-  CHARSET_INFO *scs= system_charset_info;
+  TABLE *schema_table= tables->table;
   TABLE *event_table= NULL;
-  READ_RECORD read_record_info;
   Open_tables_state backup;
   int ret=0;
   bool verbose= false;
   char definer[HOSTNAME_LENGTH+USERNAME_LENGTH+2];
-  bool use_index_scan= true;
-  uint key_len= 0;
-  byte *key_buf= NULL;
-  LINT_INIT(key_buf);
 
   DBUG_ENTER("fill_schema_events");
 
@@ -4130,11 +4248,11 @@ int fill_schema_events(THD *thd, TABLE_L
 
   thd->reset_n_backup_open_tables_state(&backup);
 
-  if ((ret= evex_open_event_table(thd, TL_READ, &event_table)))
+  if (evex_open_event_table(thd, TL_READ, &event_table))
   {
     sql_print_error("Table mysql.event is damaged.");
-    ret= 1;
-    goto err;
+    thd->restore_backup_open_tables_state(&backup);
+    DBUG_RETURN(1);
   }
 
   {
@@ -4145,121 +4263,36 @@ int fill_schema_events(THD *thd, TABLE_L
     verbose= !check_table_access(thd, SELECT_ACL, &tmp_table, 1);
   }
 
+  /*
+    1. SELECT I_S => use table scan. I_S.EVENTS does not guarantee order
+                     thus we won't order it. OTOH, SHOW EVENTS will be
+                     ordered.
+    2. SHOW EVENTS => second index(1) (db)
+       Reasoning: Events are per schema, therefore a scan over an index
+                  will save use from doing a table scan and comparing
+                  every single row's `db` with the schema which we show.
+                  In case of verbose mode we have to see all events
+                  of every user in the schema. Hence using the second
+                  key (numbered 1). If not in verbose mode then definer
+                  is known and db+definer is used but against PK - key 0.
+  */
   if (verbose)
   {
-    /*
-      1. SELECT I_S => use table scan. I_S.EVENTS does not guarantee order
-                       thus we won't order it. OTOH SHOW EVENTS will be
-                       ordered.
-      2. SHOW EVENTS => second index(1) (db)
-         Reasoning: Events are per schema, therefore a scan over an index
-                    will save use from doing a table scan and comparing
-                    every single row's `db` with the schema which we show.
-                    Name is used. Because here we have verbose we have to
-                    see all events of every user in the schema. If not
-                    a verbose mode then db+definer is used but for PK.
-    */
     if (thd->lex->orig_sql_command == SQLCOM_SHOW_EVENTS)
-    {
-      DBUG_PRINT("info",("Using prefix scanning on secondary KEY"));
-      event_table->file->ha_index_init(1, 1);
-      DBUG_ASSERT(thd->lex->select_lex.db);
-      DBUG_PRINT("info",("Using  `%s` in the prefix", thd->lex->select_lex.db));
-      event_table->field[EVEX_FIELD_DB]->
-          store(thd->lex->select_lex.db, strlen(thd->lex->select_lex.db),
scs);
-      key_len= event_table->key_info[1].key_part[0].store_length;
-
-      if (!(key_buf= (byte *)alloc_root(thd->mem_root, key_len)))
-      {
-        ret= 1;
-        goto err;
-      }
-    
-      key_copy(key_buf, event_table->record[0], event_table->key_info + 1,
-               key_len);
-      ret= event_table->file->index_read(event_table->record[0], key_buf,
-                                         key_len, HA_READ_PREFIX);
-    }
+      ret= schema_events_do_index_scan(thd, schema_table, event_table,
+                                       definer, 1);
     else
-    {
-      DBUG_PRINT("info",("Not using index scan but table scan."));
-      use_index_scan= false;
-      init_read_record(&read_record_info, thd, event_table, NULL, 1, 0);
-      if (-1== (ret= read_record_info.read_record(&read_record_info)))
-        ret= HA_ERR_END_OF_FILE;
-    }
+      ret= schema_events_do_table_scan(thd, schema_table, event_table);
   }
   else
-  {
-    DBUG_PRINT("info", ("Using prefix scanning on PK"));
+    ret= schema_events_do_index_scan(thd, schema_table, event_table,
+                                     definer, 0);
 
-    event_table->file->ha_index_init(0, 1);
-    event_table->field[EVEX_FIELD_DEFINER]->store(definer, strlen(definer), scs);
-    key_len= event_table->key_info->key_part[0].store_length;
-
-    if (thd->lex->orig_sql_command == SQLCOM_SHOW_EVENTS)
-    {
-      DBUG_PRINT("info",("Using also `%s` in the prefix scanning",
-                  thd->lex->select_lex.db));
-      event_table->field[EVEX_FIELD_DB]->
-            store(thd->lex->select_lex.db, strlen(thd->lex->select_lex.db),
scs);
-      key_len+= event_table->key_info->key_part[1].store_length;
-    }
-
-    if (!(key_buf= (byte *)alloc_root(thd->mem_root, key_len)))
-    {
-      ret= 1;
-      goto err;
-    }
-
-    key_copy(key_buf, event_table->record[0], event_table->key_info, key_len);
-    ret= event_table->file->index_read(event_table->record[0], key_buf, key_len,
-                                       HA_READ_PREFIX);
-  }
-
-  if (ret)
-  {
-    ret= (ret == HA_ERR_END_OF_FILE || ret == HA_ERR_KEY_NOT_FOUND) ? 0 : 1;
-    goto err;
-  }
-  DBUG_PRINT("info",("Found some rows, let's retrieve them. ret=%d", ret));
-
-  while (!ret)
-  {
-    if ((ret= fill_events_copy_to_schema_table(thd, table, event_table)))
-      goto err;
-
-    if (use_index_scan)
-      ret= event_table->file->
-                       index_next_same(event_table->record[0], key_buf, key_len);
-    else
-    {
-      /*
-        rr_sequential, in read_record(), returns 137==HA_ERR_END_OF_FILE,
-        but rr_handle_error returns -1 for that reason. Thus, read_record()
-        returns -1 eventually.
-      */
-      if (-1 ==(ret= read_record_info.read_record(&read_record_info)))
-        ret= HA_ERR_END_OF_FILE;
-    }
-  }
-  DBUG_PRINT("info",("Scan finished. ret=%d", ret));
-  /*  ret is guaranteed to be != 0 */
-  ret= (ret != HA_ERR_END_OF_FILE);
-err:
-  if (event_table)
-  {
-    DBUG_PRINT("info",("Closing index"));
-    if (use_index_scan)
-      event_table->file->ha_index_end();
-    else
-      end_read_record(&read_record_info);
-    close_thread_tables(thd);
-  }
+  close_thread_tables(thd);
 
   thd->restore_backup_open_tables_state(&backup);
 
-  DBUG_PRINT("info",("Return code=%d", ret));
+  DBUG_PRINT("info", ("Return code=%d", ret));
   DBUG_RETURN(ret);
 }