WL 1682: After review fixes + update bitvector class + bitvector test

cases + bug fixes


mysys/my_bitmap.c:
  Removed debug comments
sql/bitvector.h:
  Changed a number of things
  Added new methods, new handling of end bits, faster search
  for bits using word by word search..
sql/field.h:
  Added comment
sql/ha_federated.cc:
  Fixes
sql/ha_innodb.cc:
  Fixes
sql/ha_innodb.h:
  InnoDB still needs the extra calls not needed by anybody else
sql/ha_ndbcluster.cc:
  Removed some timestamp set bit stuff
  Always write all fields in write_row
sql/handler.cc:
  No inline methods in bit interface (caused strange bugs)
  initialise method
sql/handler.h:
  Changed to bitvector from bitmap
  Removed init variable
sql/opt_range.cc:
  Removed all extra HA_EXTRA_RETRIEVE_*
sql/sql_acl.cc:
  Removed all extra HA_EXTRA_RETRIEVE_*
sql/sql_base.cc:
  Set bits previously missed
sql/sql_insert.cc:
  Removed all extra HA_EXTRA_RETRIEVE_*
  Timestamp handling
sql/sql_load.cc:
  Updated comments plus set all bits in bitvector
sql/sql_select.cc:
  Removed ifdef since NDB and Federated needs the same calls
  Added comment or rather changed comment.
sql/sql_table.cc:
  Removed HA_EXTRA_RETRIEVE_* calls
  Updated comments
sql/sql_udf.cc:
  Removed HA_EXTRA_RETRIEVE_* calls
  Updated comments
sql/sql_update.cc:
  Removed HA_EXTRA_RETRIEVE_* calls
  Timstamp updating

18 files changed, 771 insertions, 200 deletions

diff --git a/sql/bitvector.cc b/sql/bitvector.cc
new file mode 100644
index 00000000000..d50ad7df33b
--- /dev/null
+++ b/sql/bitvector.cc
@@ -0,0 +1,320 @@
+/* -*- Mode: C++ -*-
+
+   Copyright (C) 2005 MySQL AB
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 2 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program; if not, write to the Free Software
+   Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA */
+
+#include <bitvector.h>
+
+void bitvector::create_last_word_mask()
+{
+
+  /* Get the number of used bits (1..8) in the last byte */
+  unsigned int const used= 1U + ((size()-1U) & 0x7U);
+
+  /*
+   * Create a mask with the upper 'unused' bits set and the lower 'used'
+   * bits clear. The bits within each byte is stored in big-endian order.
+   */
+  unsigned char const mask= (~((1 << used) - 1)) & 255;
+  last_word_ptr= (uint32*)(m_data+((bytes()-1U)>>2));
+
+  /*
+    The first bytes are to be set to zero since they represent real  bits
+    in the bitvector. The last bytes are set to 0xFF since they  represent
+    bytes not used by the bitvector. Finally the last byte contains  bits
+    as set by the mask above.
+  */
+
+  unsigned char *ptr= (unsigned char*)&last_word_mask;
+  switch (bytes()&3)
+  {
+  case 1:
+    last_word_mask= ~0U;
+    ptr[0]= mask;
+    return;
+  case 2:
+    last_word_mask= ~0U;
+    ptr[0]= 0;
+    ptr[1]= mask;
+    return;
+  case 3:
+    last_word_mask= 0U;
+    ptr[2]= mask;
+    ptr[3]= 0xFFU;
+    return;
+  case 0:
+    last_word_mask= 0U;
+    ptr[3]= mask;
+    return;
+  }
+}
+
+int bitvector::init(size_t size)
+{
+  DBUG_ASSERT(size < MYSQL_NO_BIT_FOUND);
+  m_size= size;
+  m_data= (uchar*)my_malloc(byte_size_word_aligned(size), MYF(0));
+  if (m_data)
+  {
+    create_last_word_mask();
+    clear_all();
+    return FALSE;
+  }
+  return TRUE;
+}
+
+uint bitvector::no_bits_set()
+{
+  uint no_bytes= bytes(), res=0, i;
+  uchar *ptr= m_data;
+  *last_word_ptr^=last_word_mask; //Reset last bits to zero
+  for (i=0; i< no_bytes; i++, ptr++)
+    res+=my_count_bits_ushort(*ptr);
+  *last_word_ptr^=last_word_mask; //Set last bits to one again
+  return res;
+}
+
+uint bitvector::get_first_bit_set()
+{
+  uchar *byte_ptr;
+  uint32 *data_ptr= (uint32*)data(), bit_found,i,j,k;
+  for (i=0; data_ptr <= last_word_ptr; data_ptr++, i++)
+  {
+    if (*data_ptr)
+    {
+      byte_ptr= (uchar*)data_ptr;
+      for (j=0; j < 4; j++, byte_ptr++)
+      {
+        if (*byte_ptr)
+        {
+          for (k=0; k < 8; k++)
+          {
+            if (*byte_ptr & (1 << k))
+            {
+              bit_found= (i << 5) + (j << 3) + k;
+              if (bit_found == m_size)
+                return MYSQL_NO_BIT_FOUND;
+              else
+                return bit_found;
+            }
+          }
+          DBUG_ASSERT(1);
+        }
+      }
+      DBUG_ASSERT(1);
+    }
+  }
+  return MYSQL_NO_BIT_FOUND;
+}
+
+uint bitvector::get_first_bit_unset()
+{
+  uchar *byte_ptr;
+  uint32 *data_ptr= (uint32*)data(), bit_found,i,j,k;
+  for (i=0; data_ptr <= last_word_ptr; data_ptr++, i++)
+  {
+    if (*data_ptr != 0xFFFFFFFF)
+    {
+      byte_ptr= (uchar*)data_ptr;
+      for (j=0; j < 4; j++, byte_ptr++)
+      {
+        if (*byte_ptr != 0xFF)
+        {
+          for (k=0; k < 8; k++)
+          {
+            if (!(*byte_ptr & (1 << k)))
+            {
+              bit_found= (i << 5) + (j << 3) + k;
+              if (bit_found == m_size)
+                return MYSQL_NO_BIT_FOUND;
+              else
+                return bit_found;
+            }
+          }
+          DBUG_ASSERT(1);
+        }
+      }
+      DBUG_ASSERT(1);
+    }
+  }
+  return MYSQL_NO_BIT_FOUND;
+}
+
+#ifdef TEST_BITVECTOR
+
+int main()
+{
+  int i;
+  for (i= 0; i < 4096; i++)
+    if (do_test(i))
+      return -1;
+  return 0;
+}
+
+bool do_test(uint bitsize)
+{
+  bitvector *bv;
+  bv = new bitvector;
+  bv->init(bitsize);
+  if (test_set_get_clear_bit(bv,bitsize))
+    return TRUE;
+  if (test_flip_bit(bv,bitsize))
+    return TRUE;
+  if (test_operators(bv,bitsize))
+    return TRUE;
+  if (test_get_all_bits(bvbitsize))
+    return TRUE;
+  if (test_compare_operators(bv,bitsize))
+    return TRUE;
+  if (test_count_bits_set(bv,bitsize))
+    return TRUE;
+  if (test_get_first_bit(bv,bitsize))
+    return TRUE;
+  if (test_get_next_bit(bv,bitsize))
+    return TRUE;
+  printf("OK");
+  return FALSE;
+}
+
+uint get_rand_bit(uint bitsize)
+{
+  return (rand() % bitsize);
+}
+
+bool test_set_get_clear_bit(bitvector *bv, uint bitsize)
+{
+  uint i, test_bit;
+  uint no_loops= bitsize > 128 ? 128 : bitsize;
+  for (i=0; i < no_loops; i++)
+  {
+    test_bit= get_rand_bit(bitsize);
+    bv->set_bit(test_bit)
+    if (!bv->get_bit(test_bit))
+      goto error1;
+    bv->clear_bit(test_bit);
+    if (bv->get_bit(test_bit))
+      goto error2;
+  }
+  return FALSE;
+error1:
+  printf("Error in set bit, bit %u, bitsize = %u", test_bit, bitsize);
+  return TRUE;
+error2:
+  printf("Error in clear bit, bit %u, bitsize = %u", test_bit, bitsize);
+  return TRUE;
+}
+
+bool test_flip_bit(bitvector *bv, uint bitsize)
+{
+  uint i test_bit;
+  uint no_loops= bitsize > 128 ? 128 : bitsize;
+  for (i=0; i < no_loops; i++)
+  {
+    test_bit= get_rand_bit(bitsize);
+    bv->flip_bit(test_bit)
+    if (!bv->get_bit(test_bit))
+      goto error1;
+    bv->flip_bit(test_bit);
+    if (bv->get_bit(test_bit))
+      goto error2;
+  }
+  return FALSE;
+error1:
+  printf("Error in flip bit 1, bit %u, bitsize = %u", test_bit, bitsize);
+  return TRUE;
+error2:
+  printf("Error in flip bit 2, bit %u, bitsize = %u", test_bit, bitsize);
+  return TRUE;
+}
+
+bool test_operators(bitvector *bv, uint bitsize)
+{
+  return FALSE;
+}
+
+bool test_get_all_bits(bitvector *bv, uint bitsize)
+{
+  uint i;
+  bv->set_all();
+  if (!bv->get_all_bits_set())
+    goto error1;
+  bv->clear_all();
+  if (!bv->get_all_bits_clear())
+    goto error2;
+  for (i=0; i<bitsize;i++)
+    bv->set_bit(i);
+  if (!bv->get_all_bits_set())
+    goto error3;
+  for (i=0; i<bitsize;i++)
+    bv->clear_bit(i);
+  if (!bv->get_all_bits_clear())
+    goto error4;
+  return FALSE;
+error1:
+  printf("Error in set_all, bitsize = %u", bitsize);
+  return TRUE;
+error2:
+  printf("Error in clear_all, bitsize = %u", bitsize);
+  return TRUE;
+error3:
+  printf("Error in bitwise set all, bitsize = %u", bitsize);
+  return TRUE;
+error4:
+  printf("Error in bitwise clear all, bitsize = %u", bitsize);
+  return TRUE;
+}
+
+bool test_compare_operators(bitvector *bv, uint bitsize)
+{
+  return FALSE;
+}
+
+bool test_count_bits_set(bitvector *bv, uint bitsize)
+{
+  uint i, bit_count=0;
+  uint no_loops= bitsize > 128 ? 128 : bitsize;
+  for (i=0; i < no_loops; i++)
+  {
+    test_bit=get_rand_bit(bitsize);
+    if (!bv->get_bit(test_bit))
+    {
+      bv->set_bit(test_bit);
+      bit_count++;
+    }
+  }
+  if (bit_count==0 && bitsize > 0)
+    error1;
+  if (bv->no_bits_set() != bit_count)
+    error2;
+  return FALSE;
+error1:
+  printf("No bits set  bitsize = %u", bitsize);
+  return TRUE;
+error2:
+  printf("Wrong count of bits set, bitsize = %u", bitsize);
+  return TRUE;
+}
+
+bool test_get_first_bit(bitvector *bv, uint bitsize)
+{
+  return FALSE;
+}
+
+bool test_get_next_bit(bitvector *bv, uint bitsize)
+{
+  return FALSE;
+}
+#endif
diff --git a/sql/bitvector.h b/sql/bitvector.h
index 9c3229df482..78def21df64 100644
--- a/sql/bitvector.h
+++ b/sql/bitvector.h
@@ -68,64 +68,70 @@ namespace
     inlining code.
 */
 
+/* Number returned when no bit found */
+#define MYSQL_NO_BIT_FOUND 1 << 20
 class bitvector
 {
 private:
-  /* Helper classes */
-  struct flip_bit_op
-  { 
-    void operator()(byte* p, byte m) { *p^= m; }
-  };
-
-  struct set_bit_op
-  { 
-    void operator()(byte* p, byte m) { *p|= m; }
-  };
-
-  struct clear_bit_op
-  { 
-    void operator()(byte* p, byte m) { *p&= ~m; }
-  };
-
-  struct test_bit_op
-  { 
-    bool operator()(byte* p, byte m) { return *p & m; }
-  };
-
   /* Compute the number of bytes required to store 'bits' bits in an array. */
   static inline size_t byte_size(size_t bits)
   { 
-    int const byte_bits = sizeof(byte) * CHAR_BIT;
+    uint const byte_bits = sizeof(byte) * CHAR_BIT;
     return (bits + (byte_bits-1)) / byte_bits; 
   }
 
-  /* Tidy the last byte (by clearing the unused bits) of the bitvector to make
-   * comparison easy.  This code is assuming that we're working with 8-bit
-   * bytes.
-   */
-  void tidy_last_byte()
+  static inline size_t byte_size_word_aligned(size_t bits)
+  {
+    return ((bits + 31) >> 5) << 2;
+  }
+
+  void create_last_word_mask()
   {
-    byte* const last_byte= m_data + bytes() - 1;
 
     /* Get the number of used bits (1..8) in the last byte */
     unsigned int const used= 1U + ((size()-1U) & 0x7U);
 
-    /* Create a mask with the upper 'unused' bits clear and the lower 'used'
-     * bits set. The bits within each byte is stored in big-endian order.
+    /*
+     * Create a mask with the upper 'unused' bits set and the lower 'used'
+     * bits clear. The bits within each byte is stored in big-endian order.
      */
-    unsigned int const mask= ((1 << used) - 1);
-
-    /* Mask the last byte */
-    *last_byte&= mask;
+    unsigned char const mask= (~((1 << used) - 1)) & 255;
+    last_word_ptr= (uint32*)(m_data+((bytes()-1U)>>2));
+
+    /*
+      The first bytes are to be set to zero since they represent real  bits
+      in the bitvector. The last bytes are set to 0xFF since they  represent
+      bytes not used by the bitvector. Finally the last byte contains  bits
+      as set by the mask above.
+    */
+
+    unsigned char *ptr= (unsigned char*)&last_word_mask;
+    switch (bytes()&3)
+    {
+    case 1:
+      last_word_mask= ~0U;
+      ptr[0]= mask;
+      return;
+    case 2:
+      last_word_mask= ~0U;
+      ptr[0]= 0;
+      ptr[1]= mask;
+      return;
+    case 3:
+      last_word_mask= 0U;
+      ptr[2]= mask;
+      ptr[3]= 0xFFU;
+      return;
+    case 0:
+      last_word_mask= 0U;
+      ptr[3]= mask;
+      return;
+    }
   }
 
-  template <class ReturnType, class Func>
-  inline ReturnType apply_to_byte(size_t const pos, Func op) const
+  inline void tidy_last_word()
   {
-    /* Here I'm assuming that we're working with 8-bit bytes. */
-    ptrdiff_t const byte_pos= pos >> 3;
-    byte const mask= (1 << (pos & 0x7U));
-    return op(&m_data[byte_pos], mask);
+    *last_word_ptr|= last_word_mask;
   }
 
 public:
@@ -135,8 +141,11 @@ public:
   }
 
   explicit bitvector(size_t size, bool value= false) 
-    : m_size(size), m_data(my_malloc(byte_size(size), MYF(0)))
+    : m_size(size),
+      m_data((uchar*)my_malloc(byte_size_word_aligned(size), MYF(0)))
   {
+    DBUG_ASSERT(size < MYSQL_NO_BIT_FOUND);
+    create_last_word_mask();
     if (value)
       set_all();
     else
@@ -147,32 +156,43 @@ public:
    * number of *bits* in the bitvector. 
    */
   explicit bitvector(byte const* data, size_t size)
-    : m_size(size), m_data(my_malloc(byte_size(size), MYF(0)))
+    : m_size(size),
+      m_data((uchar*)my_malloc(byte_size_word_aligned(size), MYF(0)))
   {
-    /* std::copy(data, data + byte_size(size), m_data); */
+    DBUG_ASSERT(size < MYSQL_NO_BIT_FOUND);
+    create_last_word_mask();
     memcpy(m_data, data, byte_size(size));
-    tidy_last_byte();
+    tidy_last_word();
   }
 
   bitvector(bitvector const& other) 
-    : m_size(other.size()), m_data(my_malloc(other.bytes(), MYF(0)))
+    : m_size(other.size()),
+      m_data((uchar*)my_malloc(other.bytes(), MYF(0)))
   {
-    /* std::copy(other.m_data, other.m_data + other.bytes(), m_data); */
+    DBUG_ASSERT(m_size < MYSQL_NO_BIT_FOUND);
+    create_last_word_mask();
     memcpy(m_data, other.data(), other.bytes());
-    tidy_last_byte();           /* Just a precaution */
+    tidy_last_word();
   }
 
-  /* Assignment operator */
-  bitvector& operator=(bitvector other)
+  ~bitvector() 
   {
-    swap(other);
-    return *this;
+    if (m_data)
+      my_free((char*)m_data, MYF(0));
   }
 
-  ~bitvector() 
+  int init(size_t size)
   {
+    DBUG_ASSERT(size < MYSQL_NO_BIT_FOUND);
+    m_size= size;
+    m_data= (uchar*)my_malloc(byte_size_word_aligned(size), MYF(0));
     if (m_data)
-      my_free(m_data, MYF(0));
+    {
+      create_last_word_mask();
+      clear_all();
+      return FALSE;
+    }
+    return TRUE;
   }
 
   /* Swap the guts of this instance with another instance. */
@@ -182,8 +202,35 @@ public:
     my_swap(m_data, other.m_data);
   }
 
+  bitvector &operator=(const bitvector &rhs)
+  {
+    DBUG_ASSERT(rhs.size() == size());
+    memcpy(m_data, rhs.data(), byte_size_word_aligned(m_size));
+    return *this;
+  }
+
+  bool get_all_bits_set()
+  {
+    uint32 *data_ptr= (uint32*)&m_data[0];
+    for (; data_ptr <= last_word_ptr; data_ptr++)
+      if (*data_ptr != 0xFFFFFFFF)
+        return FALSE;
+    return TRUE;
+  }
+
+  bool get_all_bits_clear()
+  {
+    uint32 *data_ptr= (uint32*)&m_data[0];
+    if (*last_word_ptr ^ last_word_mask)
+      return FALSE;
+    for (; data_ptr < last_word_ptr; data_ptr++)
+      if (*data_ptr)
+        return FALSE;
+    return TRUE;
+  }
+
   /* A pointer to the bytes representing the bits */
-  byte const *data() const { return m_data; }
+  uchar const *data() const { return m_data; }
 
   /* The size of the data in *bytes* */
   size_t bytes() const { return byte_size(m_size); }
@@ -194,47 +241,44 @@ public:
   /* Set all bits in the vector */
   void set_all()
   { 
-    /* std::fill_n(m_data, bytes(), 255); */
-    memset(m_data, 255, bytes()); 
-    tidy_last_byte();
+    memset(m_data, 255, byte_size_word_aligned(m_size)); 
   }
 
   /* Set a bit to a value */
   void set_bit(size_t pos)
   {
-    apply_to_byte<void>(pos, set_bit_op());
+    *(uchar*)(m_data + (pos >> 3)) |= (uchar)(1 << (pos & 0x7U));
   }
 
   /* Reset (clear) all bits in the vector */
   void clear_all()
   { 
-    /* std::fill_n(m_data, bytes(), 0); */
     memset(m_data, 0, bytes()); 
-    tidy_last_byte();
+    tidy_last_word();
   }
 
   /* Reset one bit in the vector */
   void clear_bit(size_t pos)
   {
-    apply_to_byte<void>(pos, clear_bit_op());
+    *(u_char*)(m_data + (pos >> 3)) &= (u_char)(~(1 << (pos & 0x7U)));
   }
 
   void flip_bit(size_t pos)
   {
-    apply_to_byte<void>(pos, flip_bit_op());
+    *(uchar*)(m_data + (pos >> 3)) ^= (uchar)(1 << (pos & 0x7U));
   }
 
   bool get_bit(size_t pos) const
   {
-    return apply_to_byte<bool>(pos, test_bit_op());
+    (bool)(*(uchar*)(m_data + (pos >> 3))) & (uchar)(1 << (pos & 0x7U));
   };
 
   bool operator==(bitvector const& rhs) const
   {
     if (size() != rhs.size())
       return false;
-    /* This works since I have ensured that the last byte of the array contain
-     * sensible data.
+    /* This works since I have ensured that the last byte of the array
+     * contain sensible data.
      */
     if (memcmp(data(), rhs.data(), bytes()) != 0)
       return false;
@@ -246,9 +290,130 @@ public:
     return !(*this == rhs);
   }
 
+  bitvector &operator&=(bitvector const& rhs)
+  {
+    DBUG_ASSERT(size() == rhs.size());
+    uint32 *data_ptr=(uint32*)data(), *rhs_data_ptr=(uint32*)rhs.data();
+    uint32 *last_ptr= last_word_ptr;
+    for (; data_ptr <= last_ptr; data_ptr++, rhs_data_ptr++)
+      *data_ptr&=*rhs_data_ptr;
+    return *this;
+  }
+
+  bitvector &operator|=(bitvector const& rhs)
+  {
+    DBUG_ASSERT(size() == rhs.size());
+    uint32 *data_ptr=(uint32*)data(), *rhs_data_ptr=(uint32*)rhs.data();
+    uint32 *last_ptr= last_word_ptr;
+    for (; data_ptr <= last_ptr; data_ptr++, rhs_data_ptr++)
+      *data_ptr|=*rhs_data_ptr;
+    return *this;
+  }
+
+  bitvector &operator^=(bitvector const& rhs)
+  {
+    DBUG_ASSERT(size() == rhs.size());
+    uint32 *data_ptr=(uint32*)data(), *rhs_data_ptr=(uint32*)rhs.data();
+    uint32 *last_ptr= last_word_ptr;
+    for (; data_ptr <= last_ptr; data_ptr++, rhs_data_ptr++)
+      *data_ptr^=*rhs_data_ptr;
+    tidy_last_word();
+    return *this;
+  }
+
+  bitvector &operator~()
+  {
+    uint32 *data_ptr= (uint32*)data();
+    uint32 *last_ptr= last_word_ptr;
+    for (; data_ptr <= last_ptr; data_ptr++)
+      *data_ptr^=0xFFFFFFFF;
+    tidy_last_word();
+    return *this;
+  }
+
+  uint no_bits_set()
+  {
+    uint no_bytes= bytes(), res=0, i;
+    uchar *ptr= m_data;
+    *last_word_ptr^=last_word_mask; //Reset last bits to zero
+    for (i=0; i< no_bytes; i++, ptr++)
+      res+=my_count_bits_ushort(*ptr);
+    *last_word_ptr^=last_word_mask; //Set last bits to one again
+    return res;
+  }
+
+  uint get_first_bit_set()
+  {
+    uchar *byte_ptr;
+    uint32 *data_ptr= (uint32*)data(), bit_found,i,j,k;
+    for (i=0; data_ptr <= last_word_ptr; data_ptr++, i++)
+    {
+      if (*data_ptr)
+      {
+        byte_ptr= (uchar*)data_ptr;
+        for (j=0; j < 4; j++, byte_ptr++)
+        {
+          if (*byte_ptr)
+          {
+            for (k=0; k < 8; k++)
+            {
+              if (*byte_ptr & (1 << k))
+              {
+                bit_found= (i << 5) + (j << 3) + k;
+                if (bit_found == m_size)
+                  return MYSQL_NO_BIT_FOUND;
+                else
+                  return bit_found;
+              }
+            }
+            DBUG_ASSERT(1);
+          }
+        }
+        DBUG_ASSERT(1);
+      }
+    }
+    return MYSQL_NO_BIT_FOUND;
+  }
+
+  uint get_first_bit_unset()
+  {
+    uchar *byte_ptr;
+    uint32 *data_ptr= (uint32*)data(), bit_found,i,j,k;
+    for (i=0; data_ptr <= last_word_ptr; data_ptr++, i++)
+    {
+      if (*data_ptr != 0xFFFFFFFF)
+      {
+        byte_ptr= (uchar*)data_ptr;
+        for (j=0; j < 4; j++, byte_ptr++)
+        {
+          if (*byte_ptr != 0xFF)
+          {
+            for (k=0; k < 8; k++)
+            {
+              if (!(*byte_ptr & (1 << k)))
+              {
+                bit_found= (i << 5) + (j << 3) + k;
+                if (bit_found == m_size)
+                  return MYSQL_NO_BIT_FOUND;
+                else
+                  return bit_found;
+              }
+            }
+            DBUG_ASSERT(1);
+          }
+        }
+        DBUG_ASSERT(1);
+      }
+    }
+    return MYSQL_NO_BIT_FOUND;
+  }
+
+
 private:
   size_t m_size;
-  byte *m_data;
+  uint32 last_word_mask;
+  uchar *m_data;
+  uint32 *last_word_ptr;
 };
 
 #endif /* BITVECTOR_H */
diff --git a/sql/field.h b/sql/field.h
index f7ed6592a6d..38a8f4d0bd0 100644
--- a/sql/field.h
+++ b/sql/field.h
@@ -86,7 +86,10 @@ public:
   utype		unireg_check;
   uint32	field_length;		// Length of field
   uint16	flags;
-  uint16        fieldnr;                // Field number
+  /* fieldnr is the id of the field (first field = 1) as is also
+     used in key_part.
+  */
+  uint16        fieldnr;
   uchar		null_bit;		// Bit used to test null bit
 
   Field(char *ptr_arg,uint32 length_arg,uchar *null_ptr_arg,uchar null_bit_arg,
diff --git a/sql/ha_federated.cc b/sql/ha_federated.cc
index 1854801f6a2..52fdeee4ed4 100644
--- a/sql/ha_federated.cc
+++ b/sql/ha_federated.cc
@@ -1113,7 +1113,7 @@ int ha_federated::write_row(byte *buf)
     loop through the field pointer array, add any fields to both the values
     list and the fields list that is part of the write set
   */
-  for (x=0, num_fields= 0, field= table->field; *field; field++, x++)
+  for (num_fields= 0, field= table->field; *field; field++)
   {
     /* if there is a query id and if it's equal to the current query id */
     if (ha_get_bit_in_write_set((*field)->fieldnr))
@@ -1122,15 +1122,16 @@ int ha_federated::write_row(byte *buf)
 
       if ((*field)->is_null())
       {
-        DBUG_PRINT("info", ("column %d field is_null", x));
+        DBUG_PRINT("info", ("column %d field is_null", num_fields-1));
         insert_field_value_string.append("NULL");
       }
       else
       {
-        DBUG_PRINT("info", ("column %d field is not null", x));
+        DBUG_PRINT("info", ("column %d field is not null", num_fields-1));
         (*field)->val_str(&insert_field_value_string);
         /* quote these fields if they require it */
-        (*field)->quote_data(&insert_field_value_string); }
+        (*field)->quote_data(&insert_field_value_string); 
+      }
       /* append the field name */
       insert_string.append((*field)->field_name);
 
diff --git a/sql/ha_innodb.cc b/sql/ha_innodb.cc
index e15e201bb32..9da90c9cbb7 100644
--- a/sql/ha_innodb.cc
+++ b/sql/ha_innodb.cc
@@ -2684,8 +2684,8 @@ build_template(
 		     (!(fetch_all_in_key && index_contains_field) &&
 		      !(fetch_primary_key_cols &&
 			dict_table_col_in_clustered_key(index->table, i)) &&
-                      !(ha_get_bit_in_read_set(i+1) ||
-                        ha_get_bit_in_write_set(i+1))))) {
+                      (!(table->file->ha_get_bit_in_read_set(i+1) ||
+                        table->file->ha_get_bit_in_write_set(i+1)))))) {
 
 			/* This field is not needed in the query, skip it */
 
diff --git a/sql/ha_innodb.h b/sql/ha_innodb.h
index 35f95ead757..f596ec35fe5 100644
--- a/sql/ha_innodb.h
+++ b/sql/ha_innodb.h
@@ -165,6 +165,16 @@ class ha_innobase: public handler
 	int transactional_table_lock(THD *thd, int lock_type);
 	int start_stmt(THD *thd);
 
+        int ha_retrieve_all_cols()
+        {
+          ha_set_all_bits_in_read_set();
+          return extra(HA_EXTRA_RETRIEVE_ALL_COLS);
+        }
+        int ha_retrieve_all_pk()
+        {
+          ha_set_primary_key_in_read_set();
+          return extra(HA_EXTRA_RETRIEVE_PRIMARY_KEY);
+        }
   	void position(byte *record);
   	ha_rows records_in_range(uint inx, key_range *min_key, key_range
 								*max_key);
diff --git a/sql/ha_ndbcluster.cc b/sql/ha_ndbcluster.cc
index 705fbf957b7..722623c4f2f 100644
--- a/sql/ha_ndbcluster.cc
+++ b/sql/ha_ndbcluster.cc
@@ -1921,10 +1921,7 @@ int ha_ndbcluster::write_row(byte *record)
 
   statistic_increment(thd->status_var.ha_write_count, &LOCK_status);
   if (table->timestamp_field_type & TIMESTAMP_AUTO_SET_ON_INSERT)
-  {
     table->timestamp_field->set_time();
-    ha_set_bit_in_write_set(table->timestamp_field->fieldnr);
-  }
   has_auto_increment= (table->next_number_field && record == table->record[0]);
 
   if (!(op= trans->getNdbOperation((const NDBTAB *) m_table)))
@@ -1977,7 +1974,6 @@ int ha_ndbcluster::write_row(byte *record)
   {
     Field *field= table->field[i];
     if (!(field->flags & PRI_KEY_FLAG) &&
-        (ha_get_bit_in_write_set(i+1)) &&
         set_ndb_value(op, field, i, &set_blob_value))
     {
       m_skip_auto_increment= TRUE;
diff --git a/sql/handler.cc b/sql/handler.cc
index 5460dd6d48b..8bd89888071 100644
--- a/sql/handler.cc
+++ b/sql/handler.cc
@@ -271,10 +271,10 @@ handler *get_new_handler(TABLE *table, enum db_type db_type)
   }
   if (file)
   {
-    if (table && file->ha_allocate_read_write_set(table->s->fields))
+    if (file->ha_initialise())
     {
       delete file;
-      file= 0;
+      file=0;
     }
   }
   return file;
@@ -1343,38 +1343,154 @@ int handler::ha_open(const char *name, int mode, int test_if_locked)
   DBUG_RETURN(error);
 }
 
+int handler::ha_initialise()
+{
+  if (table && table->s->fields &&
+      ha_allocate_read_write_set(table->s->fields))
+  {
+    return TRUE;
+  }
+  return FALSE;
+}
+
 int handler::ha_allocate_read_write_set(ulong no_fields)
 {
   DBUG_ENTER("ha_allocate_read_write_set");
-  uint map_size= ((no_fields + 8)/8)*8;
-  if (rw_set_allocated)
-    ha_deallocate_read_write_set();
-  DBUG_PRINT("info", ("no_fields = %d, map_size = %d", no_fields, map_size));
-  if (bitmap_init(&read_set, NULL, map_size, FALSE))
+  DBUG_PRINT("info", ("no_fields = %d", no_fields));
+  read_set= new bitvector;
+  write_set= new bitvector;
+  if (!read_set || !write_set)
   {
+    ha_deallocate_read_write_set();
     DBUG_RETURN(TRUE);
   }
-  if (bitmap_init(&write_set, NULL, map_size, FALSE))
+  if (read_set->init(no_fields+1) || write_set->init(no_fields+1))
   {
-    bitmap_free(&read_set);
+    ha_deallocate_read_write_set();
     DBUG_RETURN(TRUE);
   }
   ha_clear_all_set();
-  rw_set_allocated= TRUE;
   DBUG_RETURN(FALSE);
 }
 
 void handler::ha_deallocate_read_write_set()
 {
-  if (!rw_set_allocated)
-    return;
-  bitmap_free(&read_set);
-  bitmap_free(&write_set);
+  delete read_set;
+  delete write_set;
+  read_set=write_set=0;
+}
+
+void handler::ha_clear_all_set()
+{
+  DBUG_ENTER("ha_clear_all_set");
+  read_set->clear_all();
+  write_set->clear_all();
+  read_set->set_bit((uint)0);
+  write_set->set_bit((uint)0);
+  DBUG_VOID_RETURN;
+}
+
+void handler::ha_set_all_bits_in_read_set()
+{
+  DBUG_ENTER("handler::ha_set_all_bits_in_read_set");
+  read_set->set_all();
+  DBUG_VOID_RETURN;
+}
+
+void handler::ha_set_all_bits_in_write_set()
+{
+  DBUG_ENTER("handler::ha_set_all_bits_in_write_set");
+  write_set->set_all();
+  DBUG_VOID_RETURN;
+}
+
+void handler::ha_set_bit_in_read_set(uint fieldnr)
+{
+  DBUG_ENTER("handler::ha_set_bit_in_read_set");
+  DBUG_PRINT("info", ("fieldnr = %d", fieldnr));
+  read_set->set_bit((size_t)fieldnr);
+  DBUG_VOID_RETURN;
+}
+
+void handler::ha_clear_bit_in_read_set(uint fieldnr)
+{
+  DBUG_ENTER("handler::ha_clear_bit_in_read_set");
+  DBUG_PRINT("info", ("fieldnr = %d", fieldnr));
+  read_set->clear_bit((size_t)fieldnr);
+  DBUG_VOID_RETURN;
+}
+
+void handler::ha_set_bit_in_write_set(uint fieldnr)
+{
+  DBUG_ENTER("handler::ha_set_bit_in_write_set");
+  DBUG_PRINT("info", ("fieldnr = %d", fieldnr));
+  write_set->set_bit((size_t)fieldnr);
+  DBUG_VOID_RETURN;
+}
+
+void handler::ha_clear_bit_in_write_set(uint fieldnr)
+{
+  DBUG_ENTER("handler::ha_clear_bit_in_write_set");
+  DBUG_PRINT("info", ("fieldnr = %d", fieldnr));
+  write_set->clear_bit((size_t)fieldnr);
+  DBUG_VOID_RETURN;
+}
+
+void handler::ha_set_bit_in_rw_set(uint fieldnr, bool write_op)
+{
+  if (!write_op)
+    read_set->set_bit((size_t)fieldnr);
+  else
+    write_set->set_bit((size_t)fieldnr);
+}
+
+bool handler::ha_get_bit_in_read_set(uint fieldnr)
+{
+  read_set->get_bit((size_t)fieldnr);
+}
+
+bool handler::ha_get_bit_in_write_set(uint fieldnr)
+{
+  write_set->get_bit((size_t)fieldnr);
+}
+
+bool handler::ha_get_all_bit_in_read_set()
+{
+  return read_set->get_all_bits_set();
+}
+
+bool handler::ha_get_all_bit_in_read_clear()
+{
+  return read_set->get_all_bits_clear();
+}
+
+bool handler::ha_get_all_bit_in_write_set()
+{
+  return write_set->get_all_bits_set();
+}
+
+bool handler::ha_get_all_bit_in_write_clear()
+{
+  return write_set->get_all_bits_clear();
+}
+
+int handler::ha_retrieve_all_cols()
+{
+  read_set->set_all();
+  return 0;
+}
+
+int handler::ha_retrieve_all_pk()
+{
+  ha_set_primary_key_in_read_set();
+  return 0;
 }
 
 void handler::ha_set_primary_key_in_read_set()
 {
   ulong prim_key= table->s->primary_key;
+  DBUG_ENTER("handler::ha_set_primary_key_in_read_set");
+  DBUG_PRINT("info", ("Primary key = %d", prim_key));
   if (prim_key != MAX_KEY)
   {
     KEY_PART_INFO *key_part= table->key_info[prim_key].key_part;
@@ -1383,6 +1499,7 @@ void handler::ha_set_primary_key_in_read_set()
     for (;key_part != key_part_end; ++key_part)
       ha_set_bit_in_read_set(key_part->fieldnr);
   }
+  DBUG_VOID_RETURN;
 }
 /*
   Read first row (only) from a table
diff --git a/sql/handler.h b/sql/handler.h
index d64bc7e706e..31bd5978e1c 100644
--- a/sql/handler.h
+++ b/sql/handler.h
@@ -23,6 +23,7 @@
 
 #include <ft_global.h>
 #include <keycache.h>
+#include <bitvector.h>
 
 #ifndef NO_HASH
 #define NO_HASH				/* Not yet implemented */
@@ -483,9 +484,8 @@ public:
   bool  auto_increment_column_changed;
   bool implicit_emptied;                /* Can be !=0 only if HEAP */
   const COND *pushed_cond;
-  MY_BITMAP read_set;
-  MY_BITMAP write_set;
-  bool rw_set_allocated;
+  bitvector *read_set;
+  bitvector *write_set;
 
   handler(TABLE *table_arg) :table(table_arg),
     ref(0), data_file_length(0), max_data_file_length(0), index_file_length(0),
@@ -495,13 +495,14 @@ public:
     key_used_on_scan(MAX_KEY), active_index(MAX_KEY),
     ref_length(sizeof(my_off_t)), block_size(0),
     raid_type(0), ft_handler(0), inited(NONE), implicit_emptied(0),
-    pushed_cond(NULL), rw_set_allocated(0)
+    pushed_cond(NULL), read_set(0), write_set(0)
     {}
   virtual ~handler(void)
   {
-    ha_deallocate_read_write_set(); 
+    ha_deallocate_read_write_set();
     /* TODO: DBUG_ASSERT(inited == NONE); */
   }
+  virtual int ha_initialise();
   int ha_open(const char *name, int mode, int test_if_locked);
   void update_auto_increment();
   virtual void print_error(int error, myf errflag);
@@ -578,64 +579,32 @@ public:
     The routines are all part of the general handler and are not possible
     to override by a handler. A handler can however set/reset bits by
     calling these routines.
+
+    The methods ha_retrieve_all_cols and ha_retrieve_all_pk are made
+    virtual to handle InnoDB specifics. If InnoDB doesn't need the
+    extra parameters HA_EXTRA_RETRIEVE_ALL_COLS and
+    HA_EXTRA_RETRIEVE_PRIMARY_KEY anymore then these methods need not be
+    virtual anymore.
   */
-  void ha_set_all_bits_in_read_set()
-  {
-    bitmap_set_all(&read_set);
-  }
-  void ha_set_all_bits_in_write_set()
-  {
-    bitmap_set_all(&write_set);
-  }
-  void ha_set_bit_in_read_set(uint fieldnr)
-  {
-    bitmap_set_bit(&read_set, fieldnr);
-  }
-  void ha_clear_bit_in_read_set(uint fieldnr)
-  {
-    bitmap_clear_bit(&read_set, fieldnr);
-  }
-  void ha_set_bit_in_write_set(uint fieldnr)
-  {
-    bitmap_set_bit(&write_set, fieldnr);
-  }
-  void ha_clear_bit_in_write_set(uint fieldnr)
-  {
-    bitmap_clear_bit(&write_set, fieldnr);
-  }
-  void ha_set_bit_in_rw_set(uint fieldnr, bool write_set)
-  {
-    if (!write_set)
-      ha_set_bit_in_read_set(fieldnr);
-    else
-      ha_set_bit_in_write_set(fieldnr);
-  }
-  my_bool ha_get_bit_in_read_set(uint fieldnr)
-  {
-    return bitmap_is_set(&read_set, fieldnr);
-  }
-  my_bool ha_get_all_bit_in_read_set()
-  {
-    return bitmap_is_set_all(&read_set);
-  }
-  my_bool ha_get_all_bit_in_write_set()
-  {
-    return bitmap_is_set_all(&write_set);
-  }
-  my_bool ha_get_bit_in_write_set(uint fieldnr)
-  {
-    return bitmap_is_set(&write_set, fieldnr);
-  }
-  void ha_clear_all_set()
-  {
-    bitmap_clear_all(&read_set);
-    bitmap_clear_all(&write_set);
-    bitmap_set_bit(&read_set,0);
-    bitmap_set_bit(&write_set,0);
-  }
+  virtual int ha_retrieve_all_cols();
+  virtual int ha_retrieve_all_pk();
+  void ha_set_all_bits_in_read_set();
+  void ha_set_all_bits_in_write_set();
+  void ha_set_bit_in_read_set(uint fieldnr);
+  void ha_clear_bit_in_read_set(uint fieldnr);
+  void ha_set_bit_in_write_set(uint fieldnr);
+  void ha_clear_bit_in_write_set(uint fieldnr);
+  void ha_set_bit_in_rw_set(uint fieldnr, bool write_set);
+  bool ha_get_bit_in_read_set(uint fieldnr);
+  bool ha_get_bit_in_write_set(uint fieldnr);
+  bool ha_get_all_bit_in_read_set();
+  bool ha_get_all_bit_in_read_clear();
+  bool ha_get_all_bit_in_write_set();
+  bool ha_get_all_bit_in_write_clear();
   void ha_set_primary_key_in_read_set();
   int ha_allocate_read_write_set(ulong no_fields);
   void ha_deallocate_read_write_set();
+  void ha_clear_all_set();
 
   uint get_index(void) const { return active_index; }
   virtual int open(const char *name, int mode, uint test_if_locked)=0;
diff --git a/sql/opt_range.cc b/sql/opt_range.cc
index 0e89cd0026a..b83e4f369af 100644
--- a/sql/opt_range.cc
+++ b/sql/opt_range.cc
@@ -916,12 +916,11 @@ int QUICK_RANGE_SELECT::init_ror_merged_scan(bool reuse_handler)
   {
     DBUG_PRINT("info", ("Reusing handler %p", file));
     if (file->extra(HA_EXTRA_KEYREAD) ||
-        file->extra(HA_EXTRA_RETRIEVE_PRIMARY_KEY) ||
+        file->ha_retrieve_all_pk() ||
         init() || reset())
     {
       DBUG_RETURN(1);
     }
-    file->ha_set_primary_key_in_read_set();
     DBUG_RETURN(0);
   }
 
@@ -945,14 +944,13 @@ int QUICK_RANGE_SELECT::init_ror_merged_scan(bool reuse_handler)
     goto failure;
 
   if (file->extra(HA_EXTRA_KEYREAD) ||
-      file->extra(HA_EXTRA_RETRIEVE_PRIMARY_KEY) ||
+      file->ha_retrieve_all_pk() ||
       init() || reset())
   {
     file->external_lock(thd, F_UNLCK);
     file->close();
     goto failure;
   }
-  file->ha_set_primary_key_in_read_set();
   free_file= TRUE;
   last_rowid= file->ref;
   DBUG_RETURN(0);
@@ -5627,9 +5625,8 @@ int QUICK_INDEX_MERGE_SELECT::read_keys_and_merge()
     (This also creates a deficiency - it is possible that we will retrieve
      parts of key that are not used by current query at all.)
   */
-  if (head->file->extra(HA_EXTRA_RETRIEVE_PRIMARY_KEY))
+  if (head->file->ha_retrieve_all_pk())
     DBUG_RETURN(1);
-  head->file->ha_set_primary_key_in_read_set();
 
   cur_quick_it.rewind();
   cur_quick= cur_quick_it++;
diff --git a/sql/sql_acl.cc b/sql/sql_acl.cc
index 9552e286b34..63d3992c4c8 100644
--- a/sql/sql_acl.cc
+++ b/sql/sql_acl.cc
@@ -1526,8 +1526,7 @@ static bool update_user_table(THD *thd, const char *host, const char *user,
   key_copy((byte *) user_key, table->record[0], table->key_info,
            table->key_info->key_length);
 
-  table->file->ha_set_all_bits_in_read_set();
-  table->file->extra(HA_EXTRA_RETRIEVE_ALL_COLS);
+  table->file->ha_retrieve_all_cols();
   if (table->file->index_read_idx(table->record[0], 0,
 				  (byte *) user_key, table->key_info->key_length,
 				  HA_READ_KEY_EXACT))
@@ -1620,8 +1619,7 @@ static int replace_user_table(THD *thd, TABLE *table, const LEX_USER &combo,
   key_copy(user_key, table->record[0], table->key_info,
            table->key_info->key_length);
 
-  table->file->ha_set_all_bits_in_read_set();
-  table->file->extra(HA_EXTRA_RETRIEVE_ALL_COLS);
+  table->file->ha_retrieve_all_cols();
   if (table->file->index_read_idx(table->record[0], 0,
                                   user_key, table->key_info->key_length,
                                   HA_READ_KEY_EXACT))
@@ -1754,8 +1752,7 @@ static int replace_user_table(THD *thd, TABLE *table, const LEX_USER &combo,
       We should NEVER delete from the user table, as a uses can still
       use mysqld even if he doesn't have any privileges in the user table!
     */
-    table->file->ha_set_all_bits_in_read_set();
-    table->file->extra(HA_EXTRA_RETRIEVE_ALL_COLS);
+    table->file->ha_retrieve_all_cols();
     if (cmp_record(table,record[1]) &&
 	(error=table->file->update_row(table->record[1],table->record[0])))
     {						// This should never happen
@@ -1837,8 +1834,7 @@ static int replace_db_table(TABLE *table, const char *db,
   key_copy(user_key, table->record[0], table->key_info,
            table->key_info->key_length);
 
-  table->file->ha_set_all_bits_in_read_set();
-  table->file->extra(HA_EXTRA_RETRIEVE_ALL_COLS);
+  table->file->ha_retrieve_all_cols();
   if (table->file->index_read_idx(table->record[0],0,
                                   user_key, table->key_info->key_length,
                                   HA_READ_KEY_EXACT))
@@ -1874,8 +1870,7 @@ static int replace_db_table(TABLE *table, const char *db,
     /* update old existing row */
     if (rights)
     {
-      table->file->ha_set_all_bits_in_read_set();
-      table->file->extra(HA_EXTRA_RETRIEVE_ALL_COLS);
+      table->file->ha_retrieve_all_cols();
       if ((error=table->file->update_row(table->record[1],table->record[0])))
 	goto table_error;			/* purecov: deadcode */
     }
@@ -2208,8 +2203,7 @@ static int replace_column_table(GRANT_TABLE *g_t,
     key_copy(user_key, table->record[0], table->key_info,
              table->key_info->key_length);
 
-    table->file->ha_set_all_bits_in_read_set();
-    table->file->extra(HA_EXTRA_RETRIEVE_ALL_COLS);
+    table->file->ha_retrieve_all_cols();
     if (table->file->index_read(table->record[0], user_key,
 				table->key_info->key_length,
                                 HA_READ_KEY_EXACT))
@@ -2286,8 +2280,7 @@ static int replace_column_table(GRANT_TABLE *g_t,
     key_copy(user_key, table->record[0], table->key_info,
              key_prefix_length);
 
-    table->file->ha_set_all_bits_in_read_set();
-    table->file->extra(HA_EXTRA_RETRIEVE_ALL_COLS);
+    table->file->ha_retrieve_all_cols();
     if (table->file->index_read(table->record[0], user_key,
 				key_prefix_length,
                                 HA_READ_KEY_EXACT))
@@ -2385,8 +2378,7 @@ static int replace_table_table(THD *thd, GRANT_TABLE *grant_table,
   key_copy(user_key, table->record[0], table->key_info,
            table->key_info->key_length);
 
-  table->file->ha_set_all_bits_in_read_set();
-  table->file->extra(HA_EXTRA_RETRIEVE_ALL_COLS);
+  table->file->ha_retrieve_all_cols();
   if (table->file->index_read_idx(table->record[0], 0,
                                   user_key, table->key_info->key_length,
 				  HA_READ_KEY_EXACT))
diff --git a/sql/sql_base.cc b/sql/sql_base.cc
index d885ab50371..63fcf2b70b3 100644
--- a/sql/sql_base.cc
+++ b/sql/sql_base.cc
@@ -3705,12 +3705,14 @@ int setup_conds(THD *thd, TABLE_LIST *tables, TABLE_LIST *leaves, COND **conds)
                 goto err;
               /* Mark field used for table cache */
               t2_field->query_id= thd->query_id;
+              t2->file->ha_set_bit_in_read_set(t2_field->fieldnr);
               t2->used_keys.intersect(t2_field->part_of_key);
             }
             if ((t1_field= iterator->field()))
             {
               /* Mark field used for table cache */
               t1_field->query_id= thd->query_id;
+              t1->file->ha_set_bit_in_read_set(t1_field->fieldnr);
               t1->used_keys.intersect(t1_field->part_of_key);
             }
             Item_func_eq *tmp= new Item_func_eq(iterator->item(thd),
diff --git a/sql/sql_insert.cc b/sql/sql_insert.cc
index 9ecd02aed76..512fdb90ef9 100644
--- a/sql/sql_insert.cc
+++ b/sql/sql_insert.cc
@@ -231,7 +231,10 @@ static int check_update_fields(THD *thd, TABLE_LIST *insert_table_list,
       clear_timestamp_auto_bits(table->timestamp_field_type,
                                 TIMESTAMP_AUTO_SET_ON_UPDATE);
     else
+    {
       table->timestamp_field->query_id= timestamp_query_id;
+      table->file->ha_set_bit_in_write_set(table->timestamp_field->fieldnr);
+    }
   }
 
   return 0;
@@ -798,10 +801,7 @@ bool mysql_prepare_insert(THD *thd, TABLE_LIST *table_list, TABLE *table,
     DBUG_RETURN(TRUE);
   }
   if (duplic == DUP_UPDATE || duplic == DUP_REPLACE)
-  {
-    table->file->ha_set_primary_key_in_read_set();
-    table->file->extra(HA_EXTRA_RETRIEVE_PRIMARY_KEY);
-  }
+    table->file->ha_retrieve_all_pk();
   thd->lex->select_lex.first_execution= 0;
   DBUG_RETURN(FALSE);
 }
diff --git a/sql/sql_load.cc b/sql/sql_load.cc
index 71222b4d1c4..880fbb62c13 100644
--- a/sql/sql_load.cc
+++ b/sql/sql_load.cc
@@ -163,7 +163,7 @@ bool mysql_load(THD *thd,sql_exchange *ex,TABLE_LIST *table_list,
 
     The main thing to fix to remove this restriction is to ensure that the
     table is marked to be 'used for insert' in which case we should never
-    mark this table as as 'const table' (ie, one that has only one row).
+    mark this table as 'const table' (ie, one that has only one row).
   */
   if (unique_table(table_list, table_list->next_global))
   {
@@ -180,7 +180,7 @@ bool mysql_load(THD *thd,sql_exchange *ex,TABLE_LIST *table_list,
     for (field=table->field; *field ; field++)
       fields_vars.push_back(new Item_field(*field));
     /*
-      Since all fields is be set we set all bits in the write set
+      Since all fields are set we set all bits in the write set
     */
     table->file->ha_set_all_bits_in_write_set();
     table->timestamp_field_type= TIMESTAMP_NO_AUTO_SET;
@@ -197,8 +197,11 @@ bool mysql_load(THD *thd,sql_exchange *ex,TABLE_LIST *table_list,
     /* TODO: use this conds for 'WITH CHECK OPTIONS' */
     /*
       Indicate that both variables in field list and fields in update_list
-      is to be included in write set of table
+      is to be included in write set of table. We do however set all bits
+      in write set anyways since it is not allowed to specify NULLs in
+      LOAD DATA
     */
+    table->file->ha_set_all_bits_in_write_set();
     if (setup_fields(thd, 0, table_list, fields_vars, 2, 0, 0) ||
         setup_fields(thd, 0, table_list, set_fields, 2, 0, 0) ||
         check_that_all_fields_are_given_values(thd, table))
diff --git a/sql/sql_select.cc b/sql/sql_select.cc
index 44354168518..0d5efb99ce6 100644
--- a/sql/sql_select.cc
+++ b/sql/sql_select.cc
@@ -922,24 +922,19 @@ JOIN::optimize()
 
   }
   /*
-    Need to tell Innobase that to play it safe, it should fetch all
-    columns of the tables: this is because MySQL may build row
-    pointers for the rows, and for all columns of the primary key the
-    field->query_id has not necessarily been set to thd->query_id by
-    MySQL.
+    Need to tell handlers that to play it safe, it should fetch all
+    columns of the primary key of the tables: this is because MySQL may
+    build row pointers for the rows, and for all columns of the primary key
+    the read set has not necessarily been set by the server code.
   */
-
-#ifdef HAVE_INNOBASE_DB
   if (need_tmp || select_distinct || group_list || order)
   {
     for (uint i_h = const_tables; i_h < tables; i_h++)
     {
       TABLE* table_h = join_tab[i_h].table;
-      table_h->file->set_primary_key_in_read_set();
-      table_h->file->extra(HA_EXTRA_RETRIEVE_PRIMARY_KEY);
+      table_h->file->ha_retrieve_all_pk();
     }
   }
-#endif
 
   DBUG_EXECUTE("info",TEST_join(this););
   /*
@@ -8090,6 +8085,7 @@ create_tmp_table(THD *thd,TMP_TABLE_PARAM *param,List<Item> &fields,
   table->s->tmp_table= TMP_TABLE;
   table->s->db_low_byte_first=1;                // True for HEAP and MyISAM
   table->s->table_charset= param->table_charset;
+  table->s->primary_key= MAX_KEY; //Indicate no primary key
   table->s->keys_for_keyread.init();
   table->s->keys_in_use.init();
   /* For easier error reporting */
diff --git a/sql/sql_table.cc b/sql/sql_table.cc
index a6b1560b86f..cb98bba5e4d 100644
--- a/sql/sql_table.cc
+++ b/sql/sql_table.cc
@@ -3826,8 +3826,7 @@ copy_data_between_tables(TABLE *from,TABLE *to,
     current query id
   */
   to->file->ha_set_all_bits_in_write_set();
-  from->file->ha_set_all_bits_in_read_set();
-  from->file->extra(HA_EXTRA_RETRIEVE_ALL_COLS); //To be removed RONM
+  from->file->ha_retrieve_all_cols();
   init_read_record(&info, thd, from, (SQL_SELECT *) 0, 1,1);
   if (ignore ||
       handle_duplicates == DUP_REPLACE)
@@ -3990,11 +3989,11 @@ bool mysql_checksum_table(THD *thd, TABLE_LIST *tables, HA_CHECK_OPT *check_opt)
 	/* calculating table's checksum */
 	ha_checksum crc= 0;
 
-	/* InnoDB must be told explicitly to retrieve all columns, because
-	this function does not set field->query_id in the columns to the
-	current query id */
-        t->file->ha_set_all_bits_in_read_set();
-	t->file->extra(HA_EXTRA_RETRIEVE_ALL_COLS);
+        /*
+          Set all bits in read set and inform InnoDB that we are reading all
+          fields
+        */
+        t->file->ha_retrieve_all_cols();
 
 	if (t->file->ha_rnd_init(1))
 	  protocol->store_null();
diff --git a/sql/sql_udf.cc b/sql/sql_udf.cc
index a552fa9c0e7..dc6e8380cfe 100644
--- a/sql/sql_udf.cc
+++ b/sql/sql_udf.cc
@@ -526,8 +526,7 @@ int mysql_drop_function(THD *thd,const LEX_STRING *udf_name)
   if (!(table = open_ltable(thd,&tables,TL_WRITE)))
     goto err;
   table->field[0]->store(udf_name->str, udf_name->length, system_charset_info);
-  table->file->extra(HA_EXTRA_RETRIEVE_ALL_COLS);
-  table->file->ha_set_all_bits_in_read_set();
+  table->file->ha_retrieve_all_cols();
   if (!table->file->index_read_idx(table->record[0], 0,
 				   (byte*) table->field[0]->ptr,
 				   table->key_info[0].key_length,
diff --git a/sql/sql_update.cc b/sql/sql_update.cc
index e73ccaa7473..8cb2cbef684 100644
--- a/sql/sql_update.cc
+++ b/sql/sql_update.cc
@@ -212,7 +212,10 @@ int mysql_update(THD *thd,
     if (table->timestamp_field->query_id == thd->query_id)
       table->timestamp_field_type= TIMESTAMP_NO_AUTO_SET;
     else
+    {
       table->timestamp_field->query_id=timestamp_query_id;
+      table->file->ha_set_bit_in_write_set(table->timestamp_field->fieldnr);
+    }
   }
 
 #ifndef NO_EMBEDDED_ACCESS_CHECKS
@@ -272,8 +275,7 @@ int mysql_update(THD *thd,
       We can't update table directly;  We must first search after all
       matching rows before updating the table!
     */
-    table->file->ha_set_all_bits_in_read_set();
-    table->file->extra(HA_EXTRA_RETRIEVE_ALL_COLS);
+    table->file->ha_retrieve_all_cols();
     if (used_index < MAX_KEY && old_used_keys.is_set(used_index))
     {
       table->key_read=1;