List:Commits« Previous MessageNext Message »
From:Ole John Aske Date:February 19 2010 10:08am
Subject:bzr commit into mysql-5.1-telco-7.0-spj branch (ole.john.aske:3081)
View as plain text  
#At file:///home/oa136780/mysql/mysql-5.1-telco-7.0-spj/ based on revid:ole.john.aske@stripped

 3081 Ole John Aske	2010-02-19
      Mainly reverts http://lists.mysql.com/commits/100317 by
      reintroducing a handler method for reading from pushed child
      operations: (handler::index_read_pushed())
      
      The methode has a default implementation which does 'plain old'
      unpushed table access.
      
      The Cluster handler implementation will normally read the result
      from the pushed child operation. However there may be cases where
      the pushed join is not being executed where it has the same behaviour
      as the default implementation.

    modified:
      mysql-test/suite/ndb/r/ndb_join_pushdown.result
      mysql-test/suite/ndb/t/ndb_join_pushdown.test
      sql/ha_ndbcluster.cc
      sql/ha_ndbcluster.h
      sql/handler.h
      sql/sql_select.cc
=== modified file 'mysql-test/suite/ndb/r/ndb_join_pushdown.result'
--- a/mysql-test/suite/ndb/r/ndb_join_pushdown.result	2010-02-17 12:13:20 +0000
+++ b/mysql-test/suite/ndb/r/ndb_join_pushdown.result	2010-02-19 10:08:16 +0000
@@ -1118,6 +1118,23 @@ and t2.a = t1.b;
 a	b	a	b
 1	2	2	3
 3	1	1	2
+explain
+select *
+from t1, t1 as t2
+where t1.a in (1,3,5)
+and t2.a = t1.b
+order by t1.a desc;
+id	select_type	table	type	possible_keys	key	key_len	ref	rows	Extra
+1	SIMPLE	t1	range	PRIMARY	PRIMARY	4	NULL	3	Parent of 2 pushed join@1; Using where with pushed condition
+1	SIMPLE	t2	eq_ref	PRIMARY	PRIMARY	4	test.t1.b	1	Child of pushed join@1
+select *
+from t1, t1 as t2
+where t1.a in (1,3,5)
+and t2.a = t1.b
+order by t1.a desc;
+a	b	a	b
+3	1	1	2
+1	2	1	2
 drop table t1;
 set ndb_join_pushdown=true;
 create table t1 (a int, b int, primary key(a)) engine = ndb;

=== modified file 'mysql-test/suite/ndb/t/ndb_join_pushdown.test'
--- a/mysql-test/suite/ndb/t/ndb_join_pushdown.test	2010-02-17 12:13:20 +0000
+++ b/mysql-test/suite/ndb/t/ndb_join_pushdown.test	2010-02-19 10:08:16 +0000
@@ -666,6 +666,27 @@ from t1, t1 as t2
 where t1.a in (1,3,5)
   and t2.a = t1.b;
 
+
+## Adding and 'order by ... desc' trigger the usage
+## of QUICK_SELECT_DESC which somehow prepares a 
+## pushed join as indexscan but ends up executing it as 
+## primary key access. This (auto-) disables the pushed 
+## join execution (EXPLAIN still says 'pushdown') which
+## should test handling of this in ha_ndbcluster::index_read_pushed()
+
+explain
+select *
+from t1, t1 as t2
+where t1.a in (1,3,5)
+  and t2.a = t1.b
+order by t1.a desc;
+select *
+from t1, t1 as t2
+where t1.a in (1,3,5)
+  and t2.a = t1.b
+order by t1.a desc;
+
+
 drop table t1;
 
 

=== modified file 'sql/ha_ndbcluster.cc'
--- a/sql/ha_ndbcluster.cc	2010-02-17 12:13:20 +0000
+++ b/sql/ha_ndbcluster.cc	2010-02-19 10:08:16 +0000
@@ -3032,19 +3032,18 @@ private:
  * @return True if the operation may be pushed.
  */
 bool 
-ha_ndbcluster::check_if_pushable_parent(
-                    const NdbQueryOperationTypeWrapper& type) const
+ha_ndbcluster::check_if_pushable(const NdbQueryOperationTypeWrapper& type) const
 {
   bool pushable= FALSE;
 
-  if(m_pushed_join != NULL)
+  if (m_pushed_join != NULL)
   {
     const NdbQueryOperationTypeWrapper& query_def_type= 
       m_pushed_join->get_query_def().getQueryOperation(0U)->getType();
 
     if (query_def_type == type)
     {
-      if(m_disable_pushed_join)
+      if (m_disable_pushed_join)
       {
         DBUG_PRINT("info", ("Push disabled (HA_EXTRA_KEYREAD)"));
       }
@@ -3066,14 +3065,14 @@ ha_ndbcluster::check_if_pushable_parent(
 }
 
 
+/**
+ * Check if this table access operation is part if a pushed join operation
+ * which is actively executing.
+ */
 bool 
-ha_ndbcluster::check_if_pushed_child(
-                    const NdbQueryOperationTypeWrapper& type) const
+ha_ndbcluster::check_is_pushed() const
 {
-  // Is child of active pushed join
-  return (m_pushed_join_member &&
-          m_pushed_join_member != this &&
-          m_pushed_join_member->m_active_query);
+  return (m_pushed_join_member && m_pushed_join_member->m_active_query);
 }
 
 
@@ -3095,12 +3094,7 @@ int ha_ndbcluster::pk_read(const uchar *
   NdbOperation::LockMode lm=
     (NdbOperation::LockMode)get_ndb_lock_type(m_lock.type, table->read_set);
 
-  if (check_if_pushed_child(NdbQueryOperationDef::PrimaryKeyAccess))
-  {
-    // Is child of active pushed join
-    DBUG_RETURN(read_pushed_next(buf));
-  }
-  else if (check_if_pushable_parent(NdbQueryOperationDef::PrimaryKeyAccess))
+  if (check_if_pushable(NdbQueryOperationDef::PrimaryKeyAccess))
   {
     // Is parent of pushed join
     NdbQuery *query;
@@ -3499,12 +3493,7 @@ int ha_ndbcluster::unique_index_read(con
   NdbOperation::LockMode lm=
     (NdbOperation::LockMode)get_ndb_lock_type(m_lock.type, table->read_set);
 
-  if (check_if_pushed_child(NdbQueryOperationDef::UniqueIndexAccess))
-  {
-    // Is child of active pushed join
-    DBUG_RETURN(read_pushed_next(buf));
-  }
-  else if (check_if_pushable_parent(NdbQueryOperationDef::UniqueIndexAccess))
+  if (check_if_pushable(NdbQueryOperationDef::UniqueIndexAccess))
   {
 #ifndef DBUG_OFF
     /* 
@@ -3676,7 +3665,7 @@ int ha_ndbcluster::fetch_next(NdbQuery* 
   /**
    * Only prepare result & status from first operation in pushed join
    * which is the one related to 'this' handlers table.
-   * Consecutive rows are prepared through ::read_pushed_next() which unpack
+   * Consecutive rows are prepared through ::index_read_pushed() which unpack
    * and set correct status for each row.
    */
   if (result == NdbQuery::NextResult_gotRow)
@@ -3685,7 +3674,7 @@ int ha_ndbcluster::fetch_next(NdbQuery* 
     unpack_record(table->record[0], m_next_row);
 
     // Invalidate rows from linked operations.
-    // ::read_pushed_next() will later unpack row and set propper status
+    // ::index_read_pushed() will later unpack row and set propper status
     for (uint i= 1; i<m_pushed_join->get_join_count(); i++)
     {
       m_pushed_join->get_table(i)->status= STATUS_GARBAGE;
@@ -3706,18 +3695,30 @@ int ha_ndbcluster::fetch_next(NdbQuery* 
 
 
 int 
-ha_ndbcluster::read_pushed_next(uchar *buf)
+ha_ndbcluster::index_read_pushed(uchar *buf, const uchar *key,
+                                 key_part_map keypart_map)
 {
-  if (m_next_row)
+  DBUG_ENTER("index_read_pushed");
+
+  // Handler might have decided to not execute the pushed joins which has been prepared
+  // In this case we do an unpushed index_read based on 'Plain old' NdbOperations
+  if (unlikely(!check_is_pushed()))
+  {
+    DBUG_RETURN(index_read_map(buf, key, keypart_map, HA_READ_KEY_EXACT));
+  }
+
+  // Result from pushed operation will be referred by 'm_next_row' if non-NULL
+  else if (m_next_row)
   {
-    table->status= 0;
     unpack_record(buf, m_next_row);
+    table->status= 0;
   }
   else
   {
     table->status= STATUS_NOT_FOUND;
+    DBUG_PRINT("info", ("No record found"));
   }
-  return 0;
+  DBUG_RETURN(0);
 }
 
 
@@ -4056,10 +4057,7 @@ int ha_ndbcluster::ordered_index_scan(co
     pbound = &bound;
   }
 
-  // Index scans can't be child of pushed joins
-  DBUG_ASSERT (!check_if_pushed_child(NdbQueryOperationDef::OrderedIndexScan));
-
-  if (check_if_pushable_parent(NdbQueryOperationDef::OrderedIndexScan))
+  if (check_if_pushable(NdbQueryOperationDef::OrderedIndexScan))
   {
     DBUG_PRINT("info", 
                ("executing chain of a index scan + %d primary key joins."
@@ -4300,10 +4298,7 @@ int ha_ndbcluster::full_table_scan(const
   if (table_share->primary_key == MAX_KEY)
     get_hidden_fields_scan(&options, gets);
 
-  // Index scans can't be child of pushed joins
-  DBUG_ASSERT (!check_if_pushed_child(NdbQueryOperationDef::TableScan));
-
-  if (check_if_pushable_parent(NdbQueryOperationDef::TableScan))
+  if (check_if_pushable(NdbQueryOperationDef::TableScan))
   {
     DBUG_PRINT("info", 
                ("executing chain of a table scan + %d primary key joins."
@@ -12444,11 +12439,8 @@ ha_ndbcluster::read_multi_range_first(KE
         break;
       }
 
-      // Index scans can't be child of pushed joins
-      DBUG_ASSERT(!check_if_pushed_child(NdbQueryOperationDef::OrderedIndexScan));
-
       /* Create the scan operation for the first scan range. */
-      if (check_if_pushable_parent(NdbQueryOperationDef::OrderedIndexScan))
+      if (check_if_pushable(NdbQueryOperationDef::OrderedIndexScan))
       {
         if (!m_active_query)
         {

=== modified file 'sql/ha_ndbcluster.h'
--- a/sql/ha_ndbcluster.h	2010-02-14 21:14:44 +0000
+++ b/sql/ha_ndbcluster.h	2010-02-19 10:08:16 +0000
@@ -555,6 +555,9 @@ static void set_tabname(const char *path
 
   bool prefer_index() const;
 
+  int index_read_pushed(uchar *buf, const uchar *key,
+                        key_part_map keypart_map);
+
   uint8 table_cache_type();
 
   /*
@@ -642,8 +645,8 @@ private:
   NDB_INDEX_TYPE get_index_type_from_key(uint index_no, KEY *key_info, 
                                          bool primary) const;
   bool has_null_in_unique_index(uint idx_no) const;
-  bool check_if_pushable_parent(const NdbQueryOperationTypeWrapper& type) const;
-  bool check_if_pushed_child(const NdbQueryOperationTypeWrapper& type) const;
+  bool check_if_pushable(const NdbQueryOperationTypeWrapper& type) const;
+  bool check_is_pushed() const;
   bool check_index_fields_not_null(KEY *key_info);
 
   uint set_up_partition_info(partition_info *part_info,
@@ -795,11 +798,6 @@ private:
   NdbScanOperation *m_active_cursor;
   NdbQuery* m_active_query;
 
-  /**
-   * Read next row from a child operation of a pushed query.
-   */
-  int read_pushed_next(uchar *buf);
-
   const NdbDictionary::Table *m_table;
   /*
     Normal NdbRecord for accessing rows, with all fields including hidden

=== modified file 'sql/handler.h'
--- a/sql/handler.h	2010-02-14 21:14:44 +0000
+++ b/sql/handler.h	2010-02-19 10:08:16 +0000
@@ -1859,6 +1859,11 @@ public:
     return FALSE;
   }
 
+  virtual int index_read_pushed(uchar * buf, const uchar * key,
+                             key_part_map keypart_map)
+  { return index_read_map(buf, key, keypart_map, HA_READ_KEY_EXACT); }
+
+
  /*
     Part of old fast alter table, to be depricated
   */

=== modified file 'sql/sql_select.cc'
--- a/sql/sql_select.cc	2010-02-14 21:14:44 +0000
+++ b/sql/sql_select.cc	2010-02-19 10:08:16 +0000
@@ -153,6 +153,7 @@ static int join_read_const_table(JOIN_TA
 static int join_read_system(JOIN_TAB *tab);
 static int join_read_const(JOIN_TAB *tab);
 static int join_read_key(JOIN_TAB *tab);
+static int join_read_linked_key(JOIN_TAB *tab);
 static void join_read_key_unlock_row(st_join_table *tab);
 static int join_read_always_key(JOIN_TAB *tab);
 static int join_read_last_key(JOIN_TAB *tab);
@@ -1616,11 +1617,7 @@ make_pushed_join(JOIN *join)
   {
     JOIN_TAB *tab=join->join_tab+i;
 
-    if (tab->table->file->member_of_pushed_join())
-    {
-      active_pushed_joins--;
-    }
-    else
+    if (!tab->table->file->member_of_pushed_join())
     {
       // Try to start a pushed join from current join_tab
       AQP::Join_plan plan(tab, join->tables-i);
@@ -1628,6 +1625,17 @@ make_pushed_join(JOIN *join)
       if (pushed_joins > 0)
         active_pushed_joins += (pushed_joins-1);
     }
+    else
+    {
+      // Replace 'read_key' access with its linked counterpart 
+      // ... Which is effectively a NOOP as the row is read as part of the linked operation
+      DBUG_ASSERT(tab->read_first_record==join_read_key ||
+                  tab->read_first_record==join_read_const);
+      DBUG_ASSERT(tab->read_record.read_record == join_no_more_records);
+      tab->read_first_record= join_read_linked_key;
+      tab->read_record.unlock_row= rr_unlock_row;
+      active_pushed_joins--;
+    }
 
     // Disable 'Using join buffer' if there are active pushed join sequence
     // across the scope of the join buffer.
@@ -11916,6 +11924,59 @@ join_read_key_unlock_row(st_join_table *
 }
 
 
+/**
+  Read a table *assumed* to be included in execution of a pushed join.
+  This is the counterpart of join_read_key() for child tables in a 
+  pushed join.
+
+    When the table access is performed as part of the pushed join,
+    all 'linked' child colums are prefetched together with the parent row.
+    The handler will then only format the row as required by MySQL and set
+    'table->status' accordingly.
+
+    However, there may be situations where the prepared pushed join was not
+    executed as assumed. It is the responsibility of the handler to handle
+    these situation by letting ::index_read_pushed() then effectively do a 
+    plain old' index_read_map(..., HA_READ_KEY_EXACT);
+  
+  @param tab			Table to read
+
+  @retval
+    0	Row was found
+  @retval
+    -1   Row was not found
+  @retval
+    1   Got an error (other than row not found) during read
+*/
+static int
+join_read_linked_key(JOIN_TAB *tab)
+{
+  TABLE *table= tab->table;
+  DBUG_ENTER("join_read_linked_key");
+
+  if (!table->file->inited)
+    table->file->ha_index_init(tab->ref.key, tab->sorted);
+
+  // Fetch result from linked_key operation if not already present
+  if (table->status & STATUS_GARBAGE)
+  {
+    if (cp_buffer_from_ref(tab->join->thd, table, &tab->ref))
+      DBUG_RETURN(-1);
+
+    // 'read' itself is a NOOP: 
+    //  handler::index_read_pushed() only unpack the prefetched row and set 'status'
+    int error=table->file->index_read_pushed(table->record[0],
+                                        tab->ref.key_buff,
+                                        make_prev_keypart_map(tab->ref.key_parts));
+    if (error && error != HA_ERR_KEY_NOT_FOUND && error != HA_ERR_END_OF_FILE)
+      DBUG_RETURN(report_error(table, error));
+  }
+  table->null_row=0;
+  int rc = table->status ? -1 : 0;
+  DBUG_RETURN(rc);
+}
+
+
 /*
   ref access method implementation: "read_first" function
 


Attachment: [text/bzr-bundle] bzr/ole.john.aske@sun.com-20100219100816-otcw2l6bter0tetn.bundle
Thread
bzr commit into mysql-5.1-telco-7.0-spj branch (ole.john.aske:3081) Ole John Aske19 Feb