quickstep-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From jianq...@apache.org
Subject [3/7] incubator-quickstep git commit: QUICKSTEP-61 Fixes varlen insert bug.
Date Mon, 07 Nov 2016 21:44:08 GMT
QUICKSTEP-61 Fixes varlen insert bug.

This change fixes an incorrect estimate for the maximum size of a
tuple. Previously, it was not accounting for the space used by the
fixed length portion of the tuple.

The unit tests were modified because, now, we insert fewer tuples. It's
actually quite hard to test the correct number of tuples which will be
inserted, so this has been left as a TODO.


Project: http://git-wip-us.apache.org/repos/asf/incubator-quickstep/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-quickstep/commit/70a3d47c
Tree: http://git-wip-us.apache.org/repos/asf/incubator-quickstep/tree/70a3d47c
Diff: http://git-wip-us.apache.org/repos/asf/incubator-quickstep/diff/70a3d47c

Branch: refs/heads/min-max-stats
Commit: 70a3d47cdcf46bbb8f39b1cc1a0fbd2650a6afa0
Parents: fc898b1
Author: cramja <marc.spehlmann@gmail.com>
Authored: Tue Nov 1 13:42:04 2016 -0500
Committer: cramja <marc.spehlmann@gmail.com>
Committed: Tue Nov 1 13:42:04 2016 -0500

----------------------------------------------------------------------
 storage/SplitRowStoreTupleStorageSubBlock.cpp                | 3 ++-
 storage/tests/SplitRowStoreTupleStorageSubBlock_unittest.cpp | 8 ++------
 2 files changed, 4 insertions(+), 7 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-quickstep/blob/70a3d47c/storage/SplitRowStoreTupleStorageSubBlock.cpp
----------------------------------------------------------------------
diff --git a/storage/SplitRowStoreTupleStorageSubBlock.cpp b/storage/SplitRowStoreTupleStorageSubBlock.cpp
index 068e975..1e6f7ff 100644
--- a/storage/SplitRowStoreTupleStorageSubBlock.cpp
+++ b/storage/SplitRowStoreTupleStorageSubBlock.cpp
@@ -334,6 +334,7 @@ tuple_id SplitRowStoreTupleStorageSubBlock::bulkInsertPartialTuplesImpl(
       varlen_reserve -= relation_.getAttributeById(
         copy_groups.varlen_attrs_[vattr_idx].dst_attr_id_)->getType().maximumByteLength();
     }
+    DCHECK_GE(relation_.getMaximumVariableByteLength(), varlen_reserve);
   }
 
   InvokeOnAnyValueAccessor(
@@ -392,7 +393,7 @@ tuple_id SplitRowStoreTupleStorageSubBlock::bulkInsertPartialTuplesImpl(
                                                           (varlen_heap_offset_orig - varlen_heap_offset));
           DCHECK_LE(0, remaining_storage_after_inserts);
           std::size_t additional_tuples_insert =
-            remaining_storage_after_inserts / this->relation_.getMaximumByteLength();
+            remaining_storage_after_inserts / (tuple_slot_bytes_ + this->relation_.getMaximumByteLength());
           // We want to avoid a situation where we have several short insert iterations
           // near the end of an insertion cycle.
           if (additional_tuples_insert > this->getInsertLowerBoundThreshold()) {

http://git-wip-us.apache.org/repos/asf/incubator-quickstep/blob/70a3d47c/storage/tests/SplitRowStoreTupleStorageSubBlock_unittest.cpp
----------------------------------------------------------------------
diff --git a/storage/tests/SplitRowStoreTupleStorageSubBlock_unittest.cpp b/storage/tests/SplitRowStoreTupleStorageSubBlock_unittest.cpp
index b953854..9270d93 100644
--- a/storage/tests/SplitRowStoreTupleStorageSubBlock_unittest.cpp
+++ b/storage/tests/SplitRowStoreTupleStorageSubBlock_unittest.cpp
@@ -656,9 +656,7 @@ TEST_P(SplitRowStoreTupleStorageSubBlockTest, BulkInsertTest) {
   // Actually do the bulk-insert.
   accessor.beginIteration();
   tuple_id num_inserted = tuple_store_->bulkInsertTuples(&accessor);
-  if (testVariableLength()) {
-    EXPECT_LE(current_tuple_idx - num_inserted, getInsertLowerBoundThreshold());
-  } else {
+  if (!testVariableLength()) {
     EXPECT_EQ(current_tuple_idx, num_inserted);
     ASSERT_TRUE(accessor.iterationFinished());
     // Shouldn't be able to insert any more tuples.
@@ -885,9 +883,7 @@ TEST_P(SplitRowStoreTupleStorageSubBlockTest, BulkInsertWithRemappedAttributesTe
   // Actually do the bulk-insert.
   accessor.beginIteration();
   tuple_id num_inserted = tuple_store_->bulkInsertTuplesWithRemappedAttributes(attribute_map,
&accessor);
-  if (testVariableLength()) {
-    EXPECT_LE(current_tuple_idx - num_inserted, getInsertLowerBoundThreshold());
-  } else {
+  if (!testVariableLength()) {
     EXPECT_EQ(current_tuple_idx, num_inserted);
     ASSERT_TRUE(accessor.iterationFinished());
     // Shouldn't be able to insert any more tuples.


Mime
View raw message