Fixed null value implementation and tests

hborn · Jun 4, 2014 · 87e73dd · 87e73dd
1 parent 5e63aad
commit 87e73dd
Show file tree

Hide file tree

Showing 13 changed files with 126 additions and 140 deletions.
diff --git a/include/cassandra.h b/include/cassandra.h
@@ -486,7 +486,6 @@ cass_statement_free(CassStatement* statement);
 
 /**
  * Binds null to a query or bound statement at the specified index.
- * This cannot be used for counters, custom, or collections
  *
  * @param[in] statement
  * @param[in] index
@@ -652,7 +651,7 @@ cass_statement_bind_custom(CassStatement* statement,
  *
  * @param[in] statement
  * @param[in] index
- * @param[in] collection Pass NULL for an empty collection
+ * @param[in] collection
  * @param[in] is_map This must be set to true if the collection represents a map.
  * @return CASS_OK if successful, otherwise an error occurred.
  */

diff --git a/src/batch_request.hpp b/src/batch_request.hpp
@@ -120,7 +120,10 @@ struct BatchRequest : public MessageBody {
       size += sizeof(uint16_t);
       for (const auto& value : *statement) {
         size += sizeof(int32_t);
-        size += value.size();
+        int32_t value_size = value.size();
+        if(value_size > 0) {
+          size += value_size;
+        }
       }
     }
     size    += sizeof(uint16_t);
@@ -146,7 +149,7 @@ struct BatchRequest : public MessageBody {
 
       buffer = encode_short(buffer, statement->values.size());
       for (const auto& value : *statement) {
-        buffer = encode_long_string(buffer, value.data(), value.size());
+        buffer = encode_bytes(buffer, value.data(), value.size());
       }
     }
     encode_short(buffer, consistency);

diff --git a/src/buffer.hpp b/src/buffer.hpp
@@ -17,28 +17,32 @@
 #ifndef __CASS_BUFFER_HPP_INCLUDED__
 #define __CASS_BUFFER_HPP_INCLUDED__
 
+#include <stdint.h>
+
 namespace cass {
 
 class Buffer {
   public:
-    static const size_t FIXED_BUFFER_SIZE = 32;
+    static const int32_t FIXED_BUFFER_SIZE = 32;
 
     Buffer()
-      : size_(0) { }
+      : size_(-1) { }
 
     Buffer(const char* data, size_t size)
       : size_(size) {
-      if(size > FIXED_BUFFER_SIZE) {
-        data_.alloced = new char[size];
-        memcpy(data_.alloced, data, size);
-      } else {
-        memcpy(data_.fixed, data, size);
+      if(size > 0) {
+        if(size > FIXED_BUFFER_SIZE) {
+          data_.alloced = new char[size];
+          memcpy(data_.alloced, data, size);
+        } else {
+          memcpy(data_.fixed, data, size);
+        }
       }
     }
 
-    Buffer(size_t size)
+    Buffer(int32_t size)
       : size_(size) {
-      if(size > FIXED_BUFFER_SIZE) {
+      if(size > 0 && size > FIXED_BUFFER_SIZE) {
         data_.alloced = new char[size];
       }
     }
@@ -49,25 +53,25 @@ class Buffer {
       }
     }
 
-    Buffer(const Buffer& buffer) {
-      copy(buffer);
+    Buffer(const Buffer& Buffer) {
+      copy(Buffer);
     }
 
-    Buffer& operator=(const Buffer& buffer) {
-      copy(buffer);
+    Buffer& operator=(const Buffer& Buffer) {
+      copy(Buffer);
       return *this;
     }
 
-    Buffer(Buffer&& buffer) {
-      move(std::move(buffer));
+    Buffer(Buffer&& Buffer) {
+      move(std::move(Buffer));
     }
 
-    Buffer& operator=(Buffer&& buffer) {
-      move(std::move(buffer));
+    Buffer& operator=(Buffer&& Buffer) {
+      move(std::move(Buffer));
       return *this;
     }
 
-    void copy(const char* source, size_t size) {
+    void copy(const char* source, int32_t size) {
       memcpy(data(), source, size);
     }
 
@@ -79,36 +83,40 @@ class Buffer {
       return size_ > FIXED_BUFFER_SIZE ? data_.alloced : data_.fixed;
     }
 
-    size_t size() const {
+    int32_t size() const {
       return size_;
     }
 
   private:
-    void move(Buffer&& buffer) {
-      size_ = buffer.size_;
-      if(size_ > FIXED_BUFFER_SIZE) {
-        data_.alloced = buffer.data_.alloced;
-        buffer.data_.alloced = nullptr;
-      } else {
-        memcpy(data_.fixed, buffer.data_.fixed, size_);
+    void move(Buffer&& Buffer) {
+      size_ = Buffer.size_;
+      if(size_ > 0) {
+        if(size_ > FIXED_BUFFER_SIZE) {
+          data_.alloced = Buffer.data_.alloced;
+          Buffer.data_.alloced = nullptr;
+        } else {
+          memcpy(data_.fixed, Buffer.data_.fixed, size_);
+        }
       }
     }
 
-    void copy(const Buffer& buffer) {
-      size_ = buffer.size_;
-      if(size_ > FIXED_BUFFER_SIZE) {
-        data_.alloced = new char[size_];
-        memcpy(data_.alloced, buffer.data_.alloced, size_);
-      } else {
-        memcpy(data_.fixed, buffer.data_.fixed, size_);
+    void copy(const Buffer& Buffer) {
+      size_ = Buffer.size_;
+      if(size_ > 0) {
+        if(size_ > FIXED_BUFFER_SIZE) {
+          data_.alloced = new char[size_];
+          memcpy(data_.alloced, Buffer.data_.alloced, size_);
+        } else {
+          memcpy(data_.fixed, Buffer.data_.fixed, size_);
+        }
       }
     }
 
     union {
         char fixed[FIXED_BUFFER_SIZE];
         char* alloced;
     } data_;
-    size_t size_;
+    int32_t size_;
 };
 
 } // namespace cass

diff --git a/src/buffer_list.hpp b/src/buffer_list.hpp
@@ -31,31 +31,31 @@ class BufferList
       buffers_.reserve(count);
     }
 
-    Buffer* append(size_t size) {
+    Buffer* append(int32_t size) {
       buffers_.push_back(Buffer(size));
       size_ += size;
       return &buffers_.back();
     }
 
-    void append(const char* data, size_t size) {
+    void append(const char* data, int32_t size) {
       buffers_.push_back(Buffer(data, size));
       size_ += size;
     }
 
     void combine(char* output) const {
-      size_t offset = 0;
+      int32_t offset = 0;
       for(const auto&  buffer : buffers_) {
         memcpy(output + offset, buffer.data(), buffer.size());
         offset += buffer.size();
       }
     }
 
     size_t count() const { return buffers_.size(); }
-    size_t size() const { return size_; }
+    int32_t size() const { return size_; }
 
   private:
     std::vector<Buffer> buffers_;
-    size_t size_;
+    int32_t size_;
 };
 
 }

diff --git a/src/buffer_piece.hpp b/src/buffer_piece.hpp
@@ -17,24 +17,26 @@
 #ifndef __CASS_BUFFER_PIECE_HPP_INCLUDED__
 #define __CASS_BUFFER_PIECE_HPP_INCLUDED__
 
+#include <stdint.h>
+
 namespace cass {
 
 class BufferPiece {
   public:
     BufferPiece()
       : data_(nullptr)
-      , size_(0) { }
+      , size_(-1) { }
 
     BufferPiece(char* data, size_t size)
       : data_(data)
       , size_(size) { }
 
     char* data() const { return data_; }
-    size_t size() const { return size_; }
+    int32_t size() const { return size_; }
 
   private:
     char* data_;
-    size_t size_;
+    int32_t size_;
 };
 
 } // namespace cass

diff --git a/src/execute_request.hpp b/src/execute_request.hpp
@@ -140,7 +140,10 @@ struct ExecuteRequest : public Statement {
         size += sizeof(int16_t);
         for (const auto& value : values) {
           size += sizeof(int32_t);
-          size += value.size();
+          int32_t value_size = value.size();
+          if(value_size > 0) {
+            size += value_size;
+          }
         }
         flags |= CASS_QUERY_FLAG_VALUES;
       }
@@ -168,7 +171,7 @@ struct ExecuteRequest : public Statement {
       if (!values.empty()) {
         buffer = encode_short(buffer, values.size());
         for (const auto& value : values) {
-          buffer = encode_long_string(buffer, value.data(), value.size());
+          buffer = encode_bytes(buffer, value.data(), value.size());
         }
       }
 

diff --git a/src/query_request.hpp b/src/query_request.hpp
@@ -148,7 +148,10 @@ struct QueryRequest : public Statement {
       size += sizeof(int16_t);
       for (const auto& value : values) {
         size += sizeof(int32_t);
-        size += value.size();
+        int32_t value_size = value.size();
+        if(value_size > 0) {
+          size += value_size;
+        }
       }
       flags |= CASS_QUERY_FLAG_VALUES;
     }
@@ -176,7 +179,7 @@ struct QueryRequest : public Statement {
     if (!values.empty()) {
       buffer = encode_short(buffer, values.size());
       for (const auto& value : values) {
-        buffer = encode_long_string(buffer, value.data(), value.size());
+        buffer = encode_bytes(buffer, value.data(), value.size());
       }
     }
 

diff --git a/src/row.cpp b/src/row.cpp
@@ -44,20 +44,23 @@ char* decode_row(char* rows, const ResultResponse* result, Row& output) {
     const ResultResponse::ColumnMetaData& metadata = result->column_metadata[i];
     CassValueType type = static_cast<CassValueType>(metadata.type);
 
-    if(type == CASS_VALUE_TYPE_MAP ||
-       type == CASS_VALUE_TYPE_LIST ||
-       type == CASS_VALUE_TYPE_SET) {
-      uint16_t count = 0;
-      Value value(type, decode_short(buffer, count), size - sizeof(uint16_t));
-      value.count = count;
-      value.primary_type = static_cast<CassValueType>(metadata.collection_primary_type);
-      value.secondary_type = static_cast<CassValueType>(metadata.collection_secondary_type);
-      output.push_back(value);
-    } else {
-      output.push_back(Value(type, buffer, size));
+    if(size >= 0) {
+      if(type == CASS_VALUE_TYPE_MAP ||
+         type == CASS_VALUE_TYPE_LIST ||
+         type == CASS_VALUE_TYPE_SET) {
+        uint16_t count = 0;
+        Value value(type, decode_short(buffer, count), size - sizeof(uint16_t));
+        value.count = count;
+        value.primary_type = static_cast<CassValueType>(metadata.collection_primary_type);
+        value.secondary_type = static_cast<CassValueType>(metadata.collection_secondary_type);
+        output.push_back(value);
+      } else {
+        output.push_back(Value(type, buffer, size));
+      }
+      buffer += size;
+    } else { // null value
+      output.push_back(Value());
     }
-
-    buffer += size;
   }
   return buffer;
 }

diff --git a/src/serialization.hpp b/src/serialization.hpp
@@ -179,6 +179,16 @@ encode_long_string(
   return buffer + size;
 }
 
+
+inline char* encode_bytes(char* output, const char* input, int32_t size) {
+  char* buffer = encode_int(output, size);
+  if(size > 0) {
+    memcpy(buffer, input, size);
+    return buffer + size;
+  }
+  return buffer;
+}
+
 inline char*
 encode_string_map(
     char*                                     output,

diff --git a/src/statement.hpp b/src/statement.hpp
@@ -32,7 +32,7 @@
 namespace cass {
 
 struct Statement : public MessageBody {
-  typedef std::vector<Buffer>         ValueCollection;
+  typedef std::vector<Buffer>        ValueCollection;
   typedef ValueCollection::iterator       ValueIterator;
   typedef ValueCollection::const_iterator ConstValueIterator;
 
@@ -91,13 +91,13 @@ struct Statement : public MessageBody {
     return values.size();
   }
 
-#define BIND_FIXED_TYPE(DeclType, EncodeType, Name)                  \
+#define BIND_FIXED_TYPE(DeclType, EncodeType, Name)                   \
   inline CassError bind_##Name(size_t index, const DeclType& value) { \
-    CASS_VALUE_CHECK_INDEX(index);                       \
-    Buffer buffer(sizeof(DeclType));                      \
-    encode_##EncodeType(buffer.data(), value);            \
-    values[index] = buffer;         \
-    return CASS_OK;                          \
+    CASS_VALUE_CHECK_INDEX(index);                                    \
+    Buffer buffer(sizeof(DeclType));                                  \
+    encode_##EncodeType(buffer.data(), value);                        \
+    values[index] = buffer;                                           \
+    return CASS_OK;                                                   \
   }
 
   BIND_FIXED_TYPE(int32_t, int, int32)
@@ -147,16 +147,10 @@ struct Statement : public MessageBody {
 
   inline CassError bind(size_t index, const Collection* collection, bool is_map) {
     CASS_VALUE_CHECK_INDEX(index);
-    if(collection != nullptr) {
-      if(is_map && collection->item_count() % 2 != 0) {
-        return CASS_ERROR_LIB_INVALID_ITEM_COUNT;
-      }
-      values[index] = collection->build(is_map);
-    } else {
-      Buffer buffer(sizeof(uint16_t));
-      encode_short(buffer.data(), 0);
-      values[index] = buffer;
+    if(is_map && collection->item_count() % 2 != 0) {
+      return CASS_ERROR_LIB_INVALID_ITEM_COUNT;
     }
+    values[index] = collection->build(is_map);
     return CASS_OK;
   }
 

diff --git a/src/value.cpp b/src/value.cpp
@@ -115,11 +115,7 @@ CassValueType cass_value_type(const CassValue* value) {
 }
 
 cass_bool_t cass_value_is_null(const CassValue* value) {
-  if(cass_value_is_collection(value)) {
-    return static_cast<cass_bool_t>(value->count == 0);
-  } else {
-    return static_cast<cass_bool_t>(value->buffer.size() == 0);
-  }
+  return static_cast<cass_bool_t>(value->buffer.size() < 0);
 }
 
 cass_bool_t cass_value_is_collection(const CassValue* value) {