You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by jb...@apache.org on 2017/03/20 14:48:48 UTC

geode-native git commit: GEODE-2578: Remove 64 KiB limit on query strings.

Repository: geode-native
Updated Branches:
  refs/heads/develop d2ae527f2 -> 2cd7b10b8


GEODE-2578: Remove 64 KiB limit on query strings.

- Remove artificial cap of 65535 for query string
  length by using 32 bits for the length of query
  strings in DataOutput::writeFullUTF().
- Rename parameter to TcrMessage::writeStringPart()
  whose name is misleading due to copy-and-paste.
- Add three unit tests for query-related messages:
  * testConstructorEXECUTECQ_MSG_TYPE
  * testConstructorWithGinormousQueryEXECUTECQ_MSG_TYPE
  * testConstructorEXECUTECQ_WITH_IR_MSG_TYPE
- Fix the cursor advance tests to not depend on
  specific values for buffer length and not to
  test remaining buffer length.

This closes #48


Project: http://git-wip-us.apache.org/repos/asf/geode-native/repo
Commit: http://git-wip-us.apache.org/repos/asf/geode-native/commit/2cd7b10b
Tree: http://git-wip-us.apache.org/repos/asf/geode-native/tree/2cd7b10b
Diff: http://git-wip-us.apache.org/repos/asf/geode-native/diff/2cd7b10b

Branch: refs/heads/develop
Commit: 2cd7b10b8c581c22082b85426478d693e584c7d0
Parents: d2ae527
Author: Sarge <md...@pivotal.io>
Authored: Tue Mar 7 11:26:53 2017 -0800
Committer: Jacob Barrett <jb...@pivotal.io>
Committed: Mon Mar 20 07:48:19 2017 -0700

----------------------------------------------------------------------
 src/cppcache/include/geode/DataOutput.hpp |  5 +-
 src/cppcache/src/TcrMessage.cpp           |  4 +-
 src/cppcache/src/TcrMessage.hpp           |  2 +-
 src/cppcache/test/DataOutputTest.cpp      | 20 ++------
 src/cppcache/test/TcrMessage_unittest.cpp | 64 ++++++++++++++++++++++++++
 5 files changed, 73 insertions(+), 22 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/geode-native/blob/2cd7b10b/src/cppcache/include/geode/DataOutput.hpp
----------------------------------------------------------------------
diff --git a/src/cppcache/include/geode/DataOutput.hpp b/src/cppcache/include/geode/DataOutput.hpp
index 9fe1472..dee5a1d 100644
--- a/src/cppcache/include/geode/DataOutput.hpp
+++ b/src/cppcache/include/geode/DataOutput.hpp
@@ -378,9 +378,8 @@ class CPPCACHE_EXPORT DataOutput {
      */
   inline void writeFullUTF(const char* value, uint32_t length = 0) {
     if (value != NULL) {
-      int32_t len = getEncodedLength(value, length);
-      uint16_t encodedLen = static_cast<uint16_t>(len > 0xFFFF ? 0xFFFF : len);
-      writeInt(static_cast<int32_t>(encodedLen));
+      int32_t encodedLen = getEncodedLength(value, length);
+      writeInt(encodedLen);
       ensureCapacity(encodedLen);
       write(static_cast<int8_t>(0));  // isObject = 0 BYTE_CODE
       uint8_t* end = m_buf + encodedLen;

http://git-wip-us.apache.org/repos/asf/geode-native/blob/2cd7b10b/src/cppcache/src/TcrMessage.cpp
----------------------------------------------------------------------
diff --git a/src/cppcache/src/TcrMessage.cpp b/src/cppcache/src/TcrMessage.cpp
index b6194a1..ea99461 100644
--- a/src/cppcache/src/TcrMessage.cpp
+++ b/src/cppcache/src/TcrMessage.cpp
@@ -756,8 +756,8 @@ void TcrMessage::writeRegionPart(const std::string& regionName) {
   m_request->writeBytesOnly((int8_t*)regionName.c_str(), len);
 }
 
-void TcrMessage::writeStringPart(const std::string& regionName) {
-  m_request->writeFullUTF(regionName.c_str());
+void TcrMessage::writeStringPart(const std::string& str) {
+  m_request->writeFullUTF(str.c_str());
 }
 
 void TcrMessage::writeEventIdPart(int reserveSize,

http://git-wip-us.apache.org/repos/asf/geode-native/blob/2cd7b10b/src/cppcache/src/TcrMessage.hpp
----------------------------------------------------------------------
diff --git a/src/cppcache/src/TcrMessage.hpp b/src/cppcache/src/TcrMessage.hpp
index 1e3cc8b..96fd5b1 100644
--- a/src/cppcache/src/TcrMessage.hpp
+++ b/src/cppcache/src/TcrMessage.hpp
@@ -555,7 +555,7 @@ class CPPCACHE_EXPORT TcrMessage {
                        const VectorOfCacheableKey* getAllKeyList = NULL);
   void writeHeader(uint32_t msgType, uint32_t numOfParts);
   void writeRegionPart(const std::string& regionName);
-  void writeStringPart(const std::string& regionName);
+  void writeStringPart(const std::string& str);
   void writeEventIdPart(int reserveSize = 0,
                         bool fullValueAfterDeltaFail = false);
   void writeMessageLength();

http://git-wip-us.apache.org/repos/asf/geode-native/blob/2cd7b10b/src/cppcache/test/DataOutputTest.cpp
----------------------------------------------------------------------
diff --git a/src/cppcache/test/DataOutputTest.cpp b/src/cppcache/test/DataOutputTest.cpp
index 6ef3e79..f397b22 100644
--- a/src/cppcache/test/DataOutputTest.cpp
+++ b/src/cppcache/test/DataOutputTest.cpp
@@ -305,15 +305,9 @@ TEST_F(DataOutputTest, TestCursorAdvance) {
       "001B596F7520686164206D65206174206D65617420746F726E61646F2E",
       dataOutput.getByteArray());
 
-  EXPECT_EQ((2 + 27), dataOutput.getBufferLength());
-
-  // buffers are pre-allocated 8k and have 2 bytes to hold the data length
-  EXPECT_EQ(((8 * 1024) - (2 + 27)), dataOutput.getRemainingBufferLength());
-
+  const uint32_t originalLength = dataOutput.getBufferLength();
   dataOutput.advanceCursor(2);
-  EXPECT_EQ((2 + 27 + 2), dataOutput.getBufferLength());
-
-  EXPECT_EQ(((8 * 1024) - (2 + 27 + 2)), dataOutput.getRemainingBufferLength());
+  EXPECT_EQ((originalLength + 2), dataOutput.getBufferLength()) << "Correct length after advance";
 }
 
 TEST_F(DataOutputTest, TestCursorNegativeAdvance) {
@@ -323,13 +317,7 @@ TEST_F(DataOutputTest, TestCursorNegativeAdvance) {
       "001B596F7520686164206D65206174206D65617420746F726E61646F2E",
       dataOutput.getByteArray());
 
-  EXPECT_EQ((2 + 27), dataOutput.getBufferLength());
-
-  // buffers are pre-allocated 8k and have 2 bytes to hold the data length
-  EXPECT_EQ(((8 * 1024) - (2 + 27)), dataOutput.getRemainingBufferLength());
-
+  const uint32_t originalLength = dataOutput.getBufferLength();
   dataOutput.advanceCursor(-2);
-  EXPECT_EQ((2 + 27 - 2), dataOutput.getBufferLength());
-
-  EXPECT_EQ(((8 * 1024) - (2 + 27 - 2)), dataOutput.getRemainingBufferLength());
+  EXPECT_EQ((originalLength - 2), dataOutput.getBufferLength()) << "Correct length after negative advance";
 }

http://git-wip-us.apache.org/repos/asf/geode-native/blob/2cd7b10b/src/cppcache/test/TcrMessage_unittest.cpp
----------------------------------------------------------------------
diff --git a/src/cppcache/test/TcrMessage_unittest.cpp b/src/cppcache/test/TcrMessage_unittest.cpp
index d73a603..b71bf72 100644
--- a/src/cppcache/test/TcrMessage_unittest.cpp
+++ b/src/cppcache/test/TcrMessage_unittest.cpp
@@ -19,6 +19,7 @@
 
 #include "gtest/gtest.h"
 
+#include <geode/CqState.hpp>
 #include <TcrMessage.hpp>
 #include "ByteArrayFixture.hpp"
 
@@ -561,3 +562,66 @@ TEST_F(TcrMessageTest, testConstructorEXECUTE_FUNCTION) {
       "75746546756E6374696F6E0000000301570000",
       testMessage);
 }
+
+TEST_F(TcrMessageTest, testConstructorEXECUTECQ_MSG_TYPE) {
+  CacheablePtr myCacheablePtr(CacheableString::createDeserializable());
+
+  TcrMessageExecuteCq testMessage("ExecuteCQ", "select * from /somewhere",
+                                  CqState::RUNNING, false,
+                                  static_cast<ThinClientBaseDM *>(NULL));
+
+  EXPECT_EQ(TcrMessage::EXECUTECQ_MSG_TYPE, testMessage.getMessageType());
+
+  EXPECT_MESSAGE_EQ(
+      "0000002A0000004000000005FFFFFFFF0000000009004578656375746543510000001800"
+      "73656C656374202A2066726F6D202F736F6D657768657265000000040000000001000000"
+      "010000000000010001",
+      testMessage);
+}
+
+TEST_F(TcrMessageTest, testConstructorWithGinormousQueryEXECUTECQ_MSG_TYPE) {
+  CacheablePtr myCacheablePtr(CacheableString::createDeserializable());
+
+  std::ostringstream oss;
+  oss << "select * from /somewhere s where s.data.id in SET(";
+  // Ensure over 64KiB of query string.
+  const int n = (((64 * 1024) + 11) / 12);
+  for (int i = 0; i < n; ++i) {
+    if (0 < i) {
+      oss << ',';
+    }
+    oss << '\'';
+    oss.fill('0');
+    oss.width(9);
+    oss << i;
+    oss << '\'';
+  }
+  oss << ") and s.type in SET('AAA','BBB','CCC','DDD') limit 60000";
+  TcrMessageExecuteCq testMessage("ExecuteCQ", oss.str(), CqState::RUNNING,
+                                  false, static_cast<ThinClientBaseDM *>(NULL));
+
+  EXPECT_EQ(TcrMessage::EXECUTECQ_MSG_TYPE, testMessage.getMessageType());
+
+  EXPECT_MESSAGE_EQ(
+      "0000002A0001009900000005FFFFFFFF0000000009004578656375746543510001007100"
+      "\\h{131298}000000040000000001000000010000000000010001",
+      testMessage);
+}
+
+TEST_F(TcrMessageTest, testConstructorEXECUTECQ_WITH_IR_MSG_TYPE) {
+  CacheablePtr myCacheablePtr(CacheableString::createDeserializable());
+
+  TcrMessageExecuteCqWithIr testMessage(
+      "ExecuteCQWithIr", "select * from /somewhere", CqState::RUNNING, false,
+      static_cast<ThinClientBaseDM *>(NULL));
+
+  EXPECT_EQ(TcrMessage::EXECUTECQ_WITH_IR_MSG_TYPE,
+            testMessage.getMessageType());
+
+  EXPECT_MESSAGE_EQ(
+      "0000002B0000004600000005FFFFFFFF000000000F004578656375746543515769746849"
+      "72000000180073656C656374202A2066726F6D202F736F6D657768657265000000040000"
+      "000001000000010000000000010001",
+      testMessage);
+}
+