Fix code review comments

Author: siara-cc

.gitignore

@@ -1,8 +1,10 @@
 *.db
 !sample_data/*.db
 test_sqlite_blaster
-test_sqlite_blaster.dSYM
+*.dSYM
 *.txt
 !sample_data/*.txt
 test.cpp
 a.out
+test
+

README.md

@@ -51,15 +51,16 @@ In this mode, a table is created with just 2 columns, `key` and `value` as shown
 
 ```c++
 #include "sqlite_index_blaster.h"
+#include <string>
+#include <vector>
 
 int main() {
 
-    // Use new and delete to have control of when the database is closed
-    sqlite_index_blaster *sqib = new sqlite_index_blaster(2, 1, 
-                (const char *[]) {"key", "value"}, "kv_index", 4096, 40, "kv_idx.db");
-    sqib->put("hello", 5, "world", 5);
-    delete sqib; // Close kv_kdx.db
+    std::vector<std::string> col_names = {"key", "value"}; // -std >= c++11
+    sqlite_index_blaster sqib(2, 1, col_names, "kv_index", 4096, 40, "kv_idx.db");
+    sqib.put_string("hello", "world");
     return 0;
+    // db file is flushed and closed when sqib is destroyed
 
 }
 ```
@@ -84,18 +85,14 @@ To retrieve the inserted values, use `get` method as shown below
 
 ```c++
 #include "sqlite_index_blaster.h"
+#include <string>
+#include <vector>
 
 int main() {
-    sqlite_index_blaster *sqib = new sqlite_index_blaster(2, 1, 
-                (const char *[]) {"key", "value"}, "kv_index", 4096, 40, "kv_idx.db");
-    sqib->put("hello", 5, "world", 5);
-    char out_val[10];
-    int out_val_len = 10;
-    if (sqib->get("hello", 5, &out_val_len, out_val)) {
-        out_val[out_val_len] = '\0';
-        cout << "Value of hello is " << out_val << endl;
-    }
-    delete sqib;
+    std::vector<std::string> col_names = {"key", "value"}; // -std >= c++11
+    sqlite_index_blaster sqib(2, 1, col_names, "kv_index", 4096, 40, "kv_idx.db");
+    sqib.put_string("hello", "world");
+    cout << "Value of hello is " << sqib.get_string("hello", "not_found") << endl;
     return 0;
 }
 ```
@@ -106,16 +103,17 @@ In this mode, a table is created with just 2 columns, `key` and `doc` as shown b
 
 ```c++
 #include "sqlite_index_blaster.h"
+#include <string>
+#include <vector>
 
 const char * json1 = "{\"name\": \"Alice\", \"age\": 25, \"email\": \"alice@example.com\"}";
 const char * json2 = "{\"name\": \"George\", \"age\": 32, \"email\": \"george@example.com\"}";
 
 int main() {
-    sqlite_index_blaster *sqib = new sqlite_index_blaster(2, 1,
-                (const char *[]) {"key", "doc"}, "doc_index", 4096, 40, "doc_store.db");
-    sqib->put("primary_contact", 15, json1, strlen(json1));
-    sqib->put("secondary_contact", 17, json2, strlen(json2));
-    delete sqib;
+    std::vector<std::string> col_names = {"key", "doc"}; // -std >= c++11
+    sqlite_index_blaster sqib(2, 1, col_names, "doc_index", 4096, 40, "doc_store.db");
+    sqib.put_string("primary_contact", json1);
+    sqib.put_string("secondary_contact", json2);
     return 0;
 }
 ```
@@ -134,34 +132,35 @@ This repo can be used to create regular tables with primary key(s) as shown belo
 
 ```c++
 #include <cmath>
+#include <string>
+#include <vector>
+
 #include "sqlite_index_blaster.h"
 
 const uint8_t col_types[] = {SQLT_TYPE_TEXT, SQLT_TYPE_INT8, SQLT_TYPE_INT8, SQLT_TYPE_INT8, SQLT_TYPE_INT8, SQLT_TYPE_REAL};
 
 int main() {
 
-    sqlite_index_blaster *sqib = new sqlite_index_blaster(6, 2,
-            (const char *[]) {"student_name", "age", "maths_marks", "physics_marks", "chemistry_marks", "average_marks"},
-            "student_marks", 4096, 40, "student_marks.db");
+    std::vector<std::string> col_names = {"student_name", "age", "maths_marks", "physics_marks", "chemistry_marks", "average_marks"};
+    sqlite_index_blaster sqib(6, 2, col_names, "student_marks", 4096, 40, "student_marks.db");
 
     int8_t maths, physics, chemistry, age;
     double average;
     uint8_t rec_buf[500];
     int rec_len;
 
     age = 19; maths = 80; physics = 69; chemistry = 98; average = round((maths + physics + chemistry) * 100 / 3) / 100;
-    rec_len = sqib->make_new_rec(rec_buf, 6, (const void *[]) {"Robert", &age, &maths, &physics, &chemistry, &average}, NULL, col_types);
-    sqib->put(rec_buf, -rec_len, NULL, 0);
+    rec_len = sqib.make_new_rec(rec_buf, 6, (const void *[]) {"Robert", &age, &maths, &physics, &chemistry, &average}, NULL, col_types);
+    sqib.put(rec_buf, -rec_len, NULL, 0);
 
     age = 20; maths = 82; physics = 99; chemistry = 83; average = round((maths + physics + chemistry) * 100 / 3) / 100;
-    rec_len = sqib->make_new_rec(rec_buf, 6, (const void *[]) {"Barry", &age, &maths, &physics, &chemistry, &average}, NULL, col_types);
-    sqib->put(rec_buf, -rec_len, NULL, 0);
+    rec_len = sqib.make_new_rec(rec_buf, 6, (const void *[]) {"Barry", &age, &maths, &physics, &chemistry, &average}, NULL, col_types);
+    sqib.put(rec_buf, -rec_len, NULL, 0);
 
     age = 23; maths = 84; physics = 89; chemistry = 74; average = round((maths + physics + chemistry) * 100 / 3) / 100;
-    rec_len = sqib->make_new_rec(rec_buf, 6, (const void *[]) {"Elizabeth", &age, &maths, &physics, &chemistry, &average}, NULL, col_types);
-    sqib->put(rec_buf, -rec_len, NULL, 0);
+    rec_len = sqib.make_new_rec(rec_buf, 6, (const void *[]) {"Elizabeth", &age, &maths, &physics, &chemistry, &average}, NULL, col_types);
+    sqib.put(rec_buf, -rec_len, NULL, 0);
 
-    delete sqib;
     return 0;
 }
 ```
@@ -233,7 +232,7 @@ Valentine's Day,Comedy,Warner Bros.
 
 This code has been tested with more than 200 million records, so it is expected to be quite stable, but bear in mind that this is so fast because there is no crash recovery.
 
-So this repo is best suited for one time inserts of large datasets, power backed systems such as those hosted in Cloud and battery backed systems.
+So this repo is best suited for one time inserts of large datasets.  It may be suitable for power backed systems such as those hosted in Cloud and battery backed systems.
 
 # License
 

src/btree_handler.h

@@ -6,8 +6,6 @@
 #include <stdint.h>
 #include "lru_cache.h"
 
-using namespace std;
-
 #define BPT_LEAF0_LVL 14
 #define BPT_STAGING_LVL 15
 #define BPT_PARENT0_LVL 16
@@ -41,8 +39,6 @@ class util {
     }
 };
 
-#define descendant static_cast<T*>(this)
-
 union page_ptr {
     unsigned long page;
     uint8_t *ptr;
@@ -83,6 +79,7 @@ class btree_handler {
     int max_key_len;
     int is_block_given;
     int root_page_num;
+    bool is_closed;
 
 public:
     lru_cache *cache;
@@ -105,6 +102,7 @@ class btree_handler {
             block_size (block_sz), cache_size (cache_sz_kb), filename (fname) {
         descendant->init_derived();
         init_stats();
+        is_closed = false;
         is_block_given = 0;
         root_page_num = start_page_num;
         is_btree = whether_btree;
@@ -129,6 +127,7 @@ class btree_handler {
     btree_handler(uint32_t block_sz, uint8_t *block, bool is_leaf, bool should_init = true) :
             block_size (block_sz), cache_size (0), filename (NULL) {
         is_block_given = 1;
+        is_closed = false;
         root_block = current_block = block;
         if (should_init) {
             descendant->set_leaf(is_leaf ? 1 : 0);
@@ -139,11 +138,17 @@ class btree_handler {
     }
 
     ~btree_handler() {
+        if (!is_closed)
+            close();
+    }
+
+    void close() {
         descendant->cleanup();
         if (cache_size > 0)
             delete cache;
         else if (!is_block_given)
             free(root_block);
+        is_closed = true;
     }
 
     void init_current_block() {
@@ -166,6 +171,17 @@ class btree_handler {
         return current_block;
     }
 
+    std::string get_string(std::string key, std::string not_found_value) {
+        bool ret = get(key.c_str(), key.length(), NULL, NULL);
+        if (ret) {
+            uint8_t *val = (uint8_t *) malloc(key_at_len);
+            int val_len;
+            descendant->copy_value(val, &val_len);
+            return std::string((const char *) val, val_len);
+        }
+        return not_found_value;
+    }
+
     bool get(const char *key, int key_len, int *in_size_out_val_len = NULL,
                  char *val = NULL, bptree_iter_ctx *ctx = NULL) {
         return get((uint8_t *) key, key_len, in_size_out_val_len, (uint8_t *) val, ctx);
@@ -208,6 +224,9 @@ class btree_handler {
         return descendant->search_current_block(ctx);
     }
 
+    bool put_string(std::string key, std::string value) {
+        return put(key.c_str(), key.length(), value.c_str(), value.length());
+    }
     bool put(const char *key, int key_len, const char *value,
             int value_len, bptree_iter_ctx *ctx = NULL) {
         return put((const uint8_t *) key, key_len, (const uint8_t *) value, value_len, ctx);

src/lru_cache.h

@@ -15,15 +15,11 @@
 #include <errno.h>
 #include <cstring>
 #include <time.h>
-#include <chrono>
-#include <brotli/encode.h>
+//#include <brotli/encode.h>
 //#include <snappy.h>
 
 #define USE_FOPEN 1
 
-using namespace std;
-using namespace chrono;
-
 typedef struct dbl_lnklst_st {
     int disk_page;
     int cache_loc;
@@ -52,7 +48,7 @@ class lru_cache {
     dbl_lnklst **disk_to_cache_map;
     size_t disk_to_cache_map_size;
     dbl_lnklst *llarr;
-    set<int> new_pages;
+    std::set<int> new_pages;
     char filename[100];
 #if USE_FOPEN == 1
     FILE *fp;
@@ -65,10 +61,8 @@ class lru_cache {
     void *(*malloc_fn)(size_t);
     bool (*const is_changed_fn)(uint8_t *, int);
     void (*const set_changed_fn)(uint8_t *, int, bool);
-    void write_pages(set<int>& pages_to_write) {
-        time_point<steady_clock> start;
-        start = steady_clock::now();
-        for (set<int>::iterator it = pages_to_write.begin(); it != pages_to_write.end(); it++) {
+    void write_pages(std::set<int>& pages_to_write) {
+        for (std::set<int>::iterator it = pages_to_write.begin(); it != pages_to_write.end(); it++) {
             uint8_t *block = &page_cache[page_size * disk_to_cache_map[*it]->cache_loc];
             set_changed_fn(block, page_size, false);
             //if (page_size < 65537 && block[5] < 255)
@@ -147,7 +141,7 @@ if (page_size == 4096) {
         if (lnklst_last_entry == NULL)
             return;
         stats.cache_flush_count++;
-        set<int> pages_to_write(new_pages);
+        std::set<int> pages_to_write(new_pages);
         calc_flush_count();
         int pages_to_check = stats.last_pages_to_flush * 3;
         dbl_lnklst *cur_entry = lnklst_last_entry;
@@ -157,7 +151,7 @@ if (page_size == 4096) {
               if (is_changed_fn(block, page_size)) {
                 pages_to_write.insert(cur_entry->disk_page);
                 if (cur_entry->disk_page == 0 || !disk_to_cache_map[cur_entry->disk_page])
-                    cout << "Disk cache map entry missing" << endl;
+                    std::cout << "Disk cache map entry missing" << std::endl;
               }
               if (pages_to_write.size() > (stats.last_pages_to_flush + new_pages.size()))
                 break;
@@ -243,7 +237,7 @@ if (page_size == 4096) {
         if (file_page_count > 0)
            file_page_count /= page_size;
         //cout << "File page count: " << file_page_count << endl;
-        disk_to_cache_map_size = max(file_page_count + 1000, (size_t) cache_size_in_pages);
+        disk_to_cache_map_size = std::max(file_page_count + 1000, (size_t) cache_size_in_pages);
         disk_to_cache_map = (dbl_lnklst **) alloc_fn(disk_to_cache_map_size * sizeof(dbl_lnklst *));
         memset(disk_to_cache_map, '\0', disk_to_cache_map_size * sizeof(dbl_lnklst *));
         empty = 0;
@@ -274,7 +268,7 @@ if (page_size == 4096) {
     }
     ~lru_cache() {
         flush_pages_in_seq(0);
-        set<int> pages_to_write;
+        std::set<int> pages_to_write;
         for (size_t ll = 0; ll < cache_size_in_pages; ll++) {
             if (llarr[ll].disk_page == 0)
               continue;
@@ -307,7 +301,7 @@ if (page_size == 4096) {
             }
             return read_count;
         } else {
-            cout << "file_pos: " << file_pos << errno << endl;
+            std::cout << "file_pos: " << file_pos << errno << std::endl;
         }
 #else
         if (lseek(fd, file_pos, SEEK_SET) != -1) {
@@ -343,7 +337,7 @@ if (page_size == 4096) {
     }
     uint8_t *get_disk_page_in_cache(int disk_page, uint8_t *block_to_keep = NULL, bool is_new = false) {
         if (disk_page < skip_page_count)
-            cout << "WARNING: asking disk_page: " << disk_page << endl;
+            std::cout << "WARNING: asking disk_page: " << disk_page << std::endl;
         if (disk_page == skip_page_count)
             return root_block;
         int cache_pos = 0;
@@ -436,12 +430,12 @@ if (page_size == 4096) {
                 off_t file_pos = page_size;
                 file_pos *= disk_page;
                 if (read_page(&page_cache[page_size * cache_pos], file_pos, page_size) != page_size)
-                    cout << "Unable to read: " << disk_page << endl;
+                    std::cout << "Unable to read: " << disk_page << std::endl;
                 stats.pages_read++;
             }
         } else {
             if (is_new)
-                cout << "WARNING: How was new page found in cache?" << endl;
+                std::cout << "WARNING: How was new page found in cache?" << std::endl;
             dbl_lnklst *current_entry = disk_to_cache_map[disk_page];
             if (lnklst_last_free == current_entry && lnklst_last_free != NULL
                     && lnklst_last_free->prev != NULL)