Fixing range queries for sparse data, some queries are off by one

duckdb · Dec 5, 2019 · 6593975 · 6593975
1 parent c269ead
commit 6593975
Show file tree

Hide file tree

Showing 15 changed files with 226 additions and 81 deletions.
diff --git a/src/execution/index/art/art.cpp b/src/execution/index/art/art.cpp
@@ -493,6 +493,8 @@ bool ART::IteratorNext(Iterator &it) {
 
 //===--------------------------------------------------------------------===//
 // Greater Than
+// Returns: True (If found leaf >= key)
+//          False (Otherwise)
 //===--------------------------------------------------------------------===//
 bool ART::Bound(unique_ptr<Node> &n, Key &key, Iterator &it, bool inclusive) {
 	it.depth = 0;
@@ -508,24 +510,42 @@ bool ART::Bound(unique_ptr<Node> &n, Key &key, Iterator &it, bool inclusive) {
 		it.depth++;
 
 		if (node->type == NodeType::NLeaf) {
-			// found a leaf node: check if it is bigger than the current key
+			// found a leaf node: check if it is bigger or equal than the current key
 			auto leaf = static_cast<Leaf *>(node);
 			it.node = leaf;
-			if (key > *leaf->value) {
-				// the key is bigger than the min_key
-				// in this case there are no keys in the set that are bigger than key
-				// thus we terminate
-				return false;
-			}
 			// if the search is not inclusive the leaf node could still be equal to the current value
 			// check if leaf is equal to the current key
-			if (!inclusive && *leaf->value == key) {
-				// leaf is equal: move to next node
-				if (!IteratorNext(it)) {
+			if (*leaf->value == key) {
+				// if its not inclusive check if there is a next leaf
+				if (!inclusive && !IteratorNext(it)) {
 					return false;
 				}
+				else{
+				    return true;
+				}
 			}
-			return true;
+
+			if (*leaf->value > key){
+			    return true;
+			}
+            // Leaf is lower than key
+            // Check if next leaf is still lower than key
+            while (IteratorNext(it)){
+                if (*it.node->value == key){
+                    // if its not inclusive check if there is a next leaf
+                    if (!inclusive && !IteratorNext(it)) {
+                        return false;
+                    }
+                    else{
+                        return true;
+                    }
+                }
+                else if (*it.node->value > key){
+                    // if its not inclusive check if there is a next leaf
+                        return true;
+                }
+            }
+			return false;
 		}
 		uint32_t mismatchPos = Node::PrefixMismatch(*this, node, key, depth);
 		if (mismatchPos != node->prefix_length) {
@@ -543,9 +563,10 @@ bool ART::Bound(unique_ptr<Node> &n, Key &key, Iterator &it, bool inclusive) {
 		depth += node->prefix_length;
 
 		top.pos = node->GetChildGreaterEqual(key[depth]);
+
 		if (top.pos == INVALID_INDEX) {
-			// no node that is >= to the current node: abort
-			return false;
+		    // Find min leaf
+            top.pos = node->GetMin();
 		}
 		node = node->GetChild(top.pos)->get();
 		depth++;

diff --git a/src/execution/index/art/art_key.cpp b/src/execution/index/art/art_key.cpp
@@ -21,7 +21,7 @@ static uint8_t FlipSign(uint8_t key_byte) {
 	return key_byte ^ 128;
 }
 
-uint32_t EncodeFloat(float x)
+uint32_t Key::EncodeFloat(float x)
 {
     unsigned long buff;
     int expbits = 8;
@@ -67,7 +67,7 @@ uint32_t EncodeFloat(float x)
 }
 
 
-uint64_t EncodeDouble(double x) {
+uint64_t  Key::EncodeDouble(double x) {
     unsigned long hilong, lowlong;
     int expbits = 11;
     uint64_t buff;
@@ -158,14 +158,12 @@ template <> unique_ptr<data_t[]> Key::CreateData(float value, bool is_little_end
     uint32_t converted_value = EncodeFloat(value);
     auto data = unique_ptr<data_t[]>(new data_t[sizeof(converted_value)]);
     reinterpret_cast<uint32_t *>(data.get())[0] = is_little_endian ? BSWAP32(converted_value) : converted_value;
-    data[0] = FlipSign(data[0]);
     return data;
 }
 template <> unique_ptr<data_t[]> Key::CreateData(double value, bool is_little_endian) {
     uint64_t converted_value = EncodeDouble(value);
     auto data = unique_ptr<data_t[]>(new data_t[sizeof(converted_value)]);
     reinterpret_cast<uint64_t *>(data.get())[0] = is_little_endian ? BSWAP64(converted_value) : converted_value;
-//    data[0] = FlipSign(data[0]);
     return data;
 }
 
@@ -189,6 +187,17 @@ bool Key::operator>(const Key &k) const {
 	return len > k.len;
 }
 
+bool Key::operator<(const Key &k) const {
+    for (index_t i = 0; i < std::min(len, k.len); i++) {
+        if (data[i] < k.data[i]) {
+            return true;
+        } else if (data[i] > k.data[i]) {
+            return false;
+        }
+    }
+    return len < k.len;
+}
+
 bool Key::operator>=(const Key &k) const {
 	for (index_t i = 0; i < std::min(len, k.len); i++) {
 		if (data[i] > k.data[i]) {

diff --git a/src/execution/index/art/node.cpp b/src/execution/index/art/node.cpp
@@ -17,6 +17,10 @@ unique_ptr<Node> *Node::GetChild(index_t pos) {
 	assert(0);
 	return nullptr;
 }
+index_t Node::GetMin(){
+    assert(0);
+    return 0;
+}
 
 uint32_t Node::PrefixMismatch(ART &art, Node *node, Key &key, uint64_t depth) {
 	uint64_t pos;

diff --git a/src/execution/index/art/node16.cpp b/src/execution/index/art/node16.cpp
@@ -40,6 +40,10 @@ unique_ptr<Node> *Node16::GetChild(index_t pos) {
 	return &child[pos];
 }
 
+index_t Node16::GetMin() {
+    return 0;
+}
+
 void Node16::insert(ART &art, unique_ptr<Node> &node, uint8_t keyByte, unique_ptr<Node> &child) {
 	Node16 *n = static_cast<Node16 *>(node.get());
 

diff --git a/src/execution/index/art/node256.cpp b/src/execution/index/art/node256.cpp
@@ -23,6 +23,14 @@ index_t Node256::GetChildGreaterEqual(uint8_t k) {
 	return INVALID_INDEX;
 }
 
+index_t Node256::GetMin(){
+    for (index_t i = 0; i < 256; i ++){
+        if (child[i]) {
+             return i;
+        }
+    }
+}
+
 index_t Node256::GetNextPos(index_t pos) {
 	for (pos == INVALID_INDEX ? pos = 0 : pos++; pos < 256; pos++) {
 		if (child[pos]) {

diff --git a/src/execution/index/art/node4.cpp b/src/execution/index/art/node4.cpp
@@ -26,6 +26,10 @@ index_t Node4::GetChildGreaterEqual(uint8_t k) {
 	return Node::GetChildGreaterEqual(k);
 }
 
+index_t Node4::GetMin() {
+    return 0;
+}
+
 index_t Node4::GetNextPos(index_t pos) {
 	if (pos == INVALID_INDEX) {
 		return 0;

diff --git a/src/execution/index/art/node48.cpp b/src/execution/index/art/node48.cpp
@@ -41,6 +41,14 @@ unique_ptr<Node> *Node48::GetChild(index_t pos) {
 	return &child[childIndex[pos]];
 }
 
+index_t Node48::GetMin(){
+    for (index_t i = 0; i < 256; i ++){
+        if (childIndex[i] != Node::EMPTY_MARKER){
+            return i;
+        }
+    }
+}
+
 void Node48::insert(ART &art, unique_ptr<Node> &node, uint8_t keyByte, unique_ptr<Node> &child) {
 	Node48 *n = static_cast<Node48 *>(node.get());
 

diff --git a/src/include/duckdb/execution/index/art/art_key.hpp b/src/include/duckdb/execution/index/art/art_key.hpp
@@ -30,11 +30,14 @@ class Key {
 	data_t &operator[](std::size_t i);
 	const data_t &operator[](std::size_t i) const;
 	bool operator>(const Key &k) const;
-	bool operator>=(const Key &k) const;
+    bool operator<(const Key &k) const;
+    bool operator>=(const Key &k) const;
 	bool operator==(const Key &k) const;
 
 	string ToString(bool is_little_endian, TypeId type);
 
+    static uint32_t EncodeFloat(float x);
+    static uint64_t EncodeDouble(double x);
 private:
 	template <class T> static unique_ptr<data_t[]> CreateData(T value, bool is_little_endian) {
 		throw NotImplementedException("Cannot create data from this type");

diff --git a/src/include/duckdb/execution/index/art/node.hpp b/src/include/duckdb/execution/index/art/node.hpp
@@ -44,6 +44,8 @@ class Node {
 	virtual index_t GetChildGreaterEqual(uint8_t k) {
 		return INVALID_INDEX;
 	}
+    //! Get the position of the biggest element in node
+    virtual index_t GetMin();
 	//! Get the next position in the node, or INVALID_INDEX if there is no next position. if pos == INVALID_INDEX, then
 	//! the first valid position in the node will be returned.
 	virtual index_t GetNextPos(index_t pos) {

diff --git a/src/include/duckdb/execution/index/art/node16.hpp b/src/include/duckdb/execution/index/art/node16.hpp
@@ -29,6 +29,8 @@ class Node16 : public Node {
 	//! Get Node16 Child
 	unique_ptr<Node> *GetChild(index_t pos) override;
 
+	index_t GetMin() override;
+
 	//! Insert node into Node16
 	static void insert(ART &art, unique_ptr<Node> &node, uint8_t keyByte, unique_ptr<Node> &child);
 	//! Shrink to node 4

diff --git a/src/include/duckdb/execution/index/art/node256.hpp b/src/include/duckdb/execution/index/art/node256.hpp
@@ -28,6 +28,8 @@ class Node256 : public Node {
 	//! Get Node256 Child
 	unique_ptr<Node> *GetChild(index_t pos) override;
 
+    index_t GetMin() override;
+
 	//! Insert node From Node256
 	static void insert(ART &art, unique_ptr<Node> &node, uint8_t keyByte, unique_ptr<Node> &child);
 

diff --git a/src/include/duckdb/execution/index/art/node4.hpp b/src/include/duckdb/execution/index/art/node4.hpp
@@ -29,6 +29,8 @@ class Node4 : public Node {
 	//! Get Node4 Child
 	unique_ptr<Node> *GetChild(index_t pos) override;
 
+	index_t GetMin() override;
+
 	//! Insert Leaf to the Node4
 	static void insert(ART &art, unique_ptr<Node> &node, uint8_t keyByte, unique_ptr<Node> &child);
 	//! Remove Leaf from Node4

diff --git a/src/include/duckdb/execution/index/art/node48.hpp b/src/include/duckdb/execution/index/art/node48.hpp
@@ -29,6 +29,8 @@ class Node48 : public Node {
 	//! Get Node48 Child
 	unique_ptr<Node> *GetChild(index_t pos) override;
 
+	index_t GetMin() override;
+
 	//! Insert node in Node48
 	static void insert(ART &art, unique_ptr<Node> &node, uint8_t keyByte, unique_ptr<Node> &child);