Merge pull request #84 from deepcrawl/feat/ODIN-416-duckdb-v0.2.5

feat: [ODIN-416] upgrade to duckdb v0.2.5
anothersmith · Mar 26, 2021 · d91b884 · d91b884
2 parents 4331d8e + c4dbdc9
commit d91b884
Show file tree

Hide file tree

Showing 23 changed files with 189 additions and 99 deletions.
diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml
@@ -4,14 +4,17 @@ on:
   push:
     branches:
       - master
+      - release/*
   pull_request:
 
 jobs:
   build:
     runs-on: ubuntu-latest
     steps:
-      - name: Checkout master
+      - name: Checkout
         uses: actions/checkout@master
+        with:
+          fetch-depth: 0
       - name: Setup node
         uses: actions/setup-node@v1
         with:

diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,6 +2,13 @@
 
 All notable changes to this project will be documented in this file. See [standard-version](https://github.com/conventional-changelog/standard-version) for commit guidelines.
 
+### [0.0.61](https://github.com/deepcrawl/node-duckdb/compare/v0.0.59...v0.0.61) (2021-02-03)
+
+
+### Features
+
+* duckdb v  6fe573 ([#74](https://github.com/deepcrawl/node-duckdb/issues/74)) ([3b5ceca](https://github.com/deepcrawl/node-duckdb/commit/3b5cecad3fc5b692ca13c35cb4b76d5440604810))
+
 ### [0.0.59](https://github.com/deepcrawl/node-duckdb/compare/v0.0.58...v0.0.59) (2020-12-16)
 
 ### [0.0.58](https://github.com/deepcrawl/node-duckdb/compare/v0.0.57...v0.0.58) (2020-12-16)

diff --git a/addon/connection.cc b/addon/connection.cc
@@ -106,18 +106,23 @@ Napi::Value Connection::Execute(const Napi::CallbackInfo &info) {
 }
 
 Napi::Value Connection::Close(const Napi::CallbackInfo &info) {
-  for (auto &result : *results) {
-    if (result != nullptr) {
-      result->close();
-    }
+  // the following gives segfaults for some reason now
+  // for (auto &result : *results) {
+  //   if (result != nullptr) {
+  //     result->close();
+  //   }
+  // }
+  if (connection) {
+    connection.reset();
+  }
+  if (database) {
+    database.reset();
   }
-  connection.reset();
-  database.reset();
   return info.Env().Undefined();
 }
 Napi::Value Connection::IsClosed(const Napi::CallbackInfo &info) {
   Napi::Env env = info.Env();
-  bool isClosed = connection == nullptr || connection->context->is_invalidated;
+  bool isClosed = connection == nullptr;
   return Napi::Boolean::New(env, isClosed);
 }
 } // namespace NodeDuckDB
diff --git a/addon/connection.h b/addon/connection.h
@@ -18,8 +18,8 @@ class Connection : public Napi::ObjectWrap<Connection> {
   Napi::Value Close(const Napi::CallbackInfo &info);
   Napi::Value IsClosed(const Napi::CallbackInfo &info);
 
-  shared_ptr<duckdb::DuckDB> database;
-  shared_ptr<duckdb::Connection> connection;
+  duckdb::shared_ptr<duckdb::DuckDB> database;
+  duckdb::shared_ptr<duckdb::Connection> connection;
   std::shared_ptr<std::vector<ResultIterator *>> results;
 };
 } // namespace NodeDuckDB

diff --git a/addon/duckdb.cc b/addon/duckdb.cc
@@ -66,7 +66,9 @@ DuckDB::DuckDB(const Napi::CallbackInfo &info)
 }
 
 Napi::Value DuckDB::Close(const Napi::CallbackInfo &info) {
-  database.reset();
+  if (database) {
+    database.reset();
+  }
   return info.Env().Undefined();
 }
 Napi::Value DuckDB::IsClosed(const Napi::CallbackInfo &info) {
@@ -78,45 +80,46 @@ bool DuckDB::IsClosed() { return database == nullptr; }
 
 Napi::Value DuckDB::GetAccessMode(const Napi::CallbackInfo &info) {
   Napi::Env env = info.Env();
-  return Napi::Number::New(env,
-                           static_cast<double>(database->config.access_mode));
+  return Napi::Number::New(
+      env, static_cast<double>(database->instance->config.access_mode));
 }
 Napi::Value DuckDB::GetCheckPointWALSize(const Napi::CallbackInfo &info) {
   Napi::Env env = info.Env();
-  return Napi::Number::New(env, database->config.checkpoint_wal_size);
+  return Napi::Number::New(env, database->instance->config.checkpoint_wal_size);
 }
 Napi::Value DuckDB::GetUseDirectIO(const Napi::CallbackInfo &info) {
   Napi::Env env = info.Env();
-  return Napi::Boolean::New(env, database->config.use_direct_io);
+  return Napi::Boolean::New(env, database->instance->config.use_direct_io);
 }
 Napi::Value DuckDB::GetMaximumMemory(const Napi::CallbackInfo &info) {
   Napi::Env env = info.Env();
-  return Napi::Number::New(env, database->config.maximum_memory);
+  return Napi::Number::New(env, database->instance->config.maximum_memory);
 }
 Napi::Value DuckDB::GetUseTemporaryDirectory(const Napi::CallbackInfo &info) {
   Napi::Env env = info.Env();
-  return Napi::Boolean::New(env, database->config.use_temporary_directory);
+  return Napi::Boolean::New(env,
+                            database->instance->config.use_temporary_directory);
 }
 Napi::Value DuckDB::GetTemporaryDirectory(const Napi::CallbackInfo &info) {
   Napi::Env env = info.Env();
-  return Napi::String::New(env, database->config.temporary_directory);
+  return Napi::String::New(env, database->instance->config.temporary_directory);
 }
 Napi::Value DuckDB::GetCollation(const Napi::CallbackInfo &info) {
   Napi::Env env = info.Env();
-  return Napi::String::New(env, database->config.collation);
+  return Napi::String::New(env, database->instance->config.collation);
 }
 Napi::Value DuckDB::GetDefaultOrderType(const Napi::CallbackInfo &info) {
   Napi::Env env = info.Env();
   return Napi::Number::New(
-      env, static_cast<double>(database->config.default_order_type));
+      env, static_cast<double>(database->instance->config.default_order_type));
 }
 Napi::Value DuckDB::GetDefaultNullOrder(const Napi::CallbackInfo &info) {
   Napi::Env env = info.Env();
   return Napi::Number::New(
-      env, static_cast<double>(database->config.default_null_order));
+      env, static_cast<double>(database->instance->config.default_null_order));
 }
 Napi::Value DuckDB::GetEnableCopy(const Napi::CallbackInfo &info) {
   Napi::Env env = info.Env();
-  return Napi::Boolean::New(env, database->config.enable_copy);
+  return Napi::Boolean::New(env, database->instance->config.enable_copy);
 }
 } // namespace NodeDuckDB
diff --git a/addon/duckdb.h b/addon/duckdb.h
@@ -9,7 +9,7 @@ class DuckDB : public Napi::ObjectWrap<DuckDB> {
 public:
   static Napi::Object Init(Napi::Env env, Napi::Object exports);
   DuckDB(const Napi::CallbackInfo &info);
-  shared_ptr<duckdb::DuckDB> database;
+  duckdb::shared_ptr<duckdb::DuckDB> database;
   static Napi::FunctionReference constructor;
   bool IsClosed(void);
 

diff --git a/addon/result_iterator.cc b/addon/result_iterator.cc
@@ -2,6 +2,7 @@
 #include "duckdb.hpp"
 #include "duckdb/common/types/hugeint.hpp"
 #include <iostream>
+#include <string.h>
 using namespace std;
 
 namespace NodeDuckDB {
@@ -32,16 +33,16 @@ Napi::Object ResultIterator::Create() { return constructor.New({}); }
 
 typedef uint64_t idx_t;
 
-int32_t GetDate(int64_t timestamp) {
-  return (int32_t)(((int64_t)timestamp) >> 32);
-}
+int64_t GetDate(int64_t timestamp) { return timestamp; }
 
-int32_t GetTime(int64_t timestamp) { return (int32_t)(timestamp & 0xFFFFFFFF); }
+int64_t GetTime(int64_t timestamp) {
+  return (int64_t)(timestamp & 0xFFFFFFFFFFFFFFFF);
+}
 
 #define EPOCH_DATE 719528
 #define SECONDS_PER_DAY (60 * 60 * 24)
 
-int64_t Epoch(int32_t date) {
+int64_t Epoch(int64_t date) {
   return ((int64_t)date - EPOCH_DATE) * SECONDS_PER_DAY;
 }
 
@@ -52,17 +53,25 @@ Napi::Value ResultIterator::FetchRow(const Napi::CallbackInfo &info) {
     return env.Undefined();
   }
   if (!current_chunk || chunk_offset >= current_chunk->size()) {
-    current_chunk = result->Fetch();
+    try {
+      current_chunk = result->Fetch();
+    } catch (const duckdb::InvalidInputException &e) {
+      if (strncmp(e.what(),
+                  "Invalid Input Error: Attempting to fetch from an "
+                  "unsuccessful or closed streaming query result",
+                  50) == 0) {
+        Napi::Error::New(
+            env, "Attempting to fetch from an unsuccessful or closed streaming "
+                 "query result: only "
+                 "one stream can be active on one connection at a time)")
+            .ThrowAsJavaScriptException();
+        return env.Undefined();
+      }
+      throw e;
+    }
     chunk_offset = 0;
   }
-  if (!current_chunk) {
-    Napi::Error::New(
-        env, "No data has been returned (possibly stream has been closed: only "
-             "one stream can be active on one connection at a time)")
-        .ThrowAsJavaScriptException();
-    return env.Undefined();
-  }
-  if (current_chunk->size() == 0) {
+  if (!current_chunk || current_chunk->size() == 0) {
     return env.Null();
   }
   Napi::Value row;
@@ -132,12 +141,12 @@ Napi::Value ResultIterator::getRowObject(Napi::Env env) {
 }
 
 Napi::Value ResultIterator::getCellValue(Napi::Env env, duckdb::idx_t col_idx) {
-  auto &nullmask = duckdb::FlatVector::Nullmask(current_chunk->data[col_idx]);
-  if (nullmask[chunk_offset]) {
+  auto val = current_chunk->data[col_idx].GetValue(chunk_offset);
+
+  if (val.is_null) {
     return env.Null();
   }
 
-  auto val = current_chunk->data[col_idx].GetValue(chunk_offset);
   switch (result->types[col_idx].id()) {
   case duckdb::LogicalTypeId::BOOLEAN:
     return Napi::Boolean::New(env, val.GetValue<bool>());
@@ -182,26 +191,25 @@ Napi::Value ResultIterator::getCellValue(Napi::Env env, duckdb::idx_t col_idx) {
       throw runtime_error("expected int64 for timestamp");
     }
     int64_t tval = val.GetValue<int64_t>();
-    int64_t date = Epoch(GetDate(tval)) * 1000;
-    int32_t time = GetTime(tval);
-    return Napi::Number::New(env, date + time);
-  }
-  case duckdb::LogicalTypeId::DATE: {
-    if (result->types[col_idx].InternalType() != duckdb::PhysicalType::INT32) {
-      throw runtime_error("expected int32 for date");
-    }
-    return Napi::Number::New(env, Epoch(val.GetValue<int32_t>()) * 1000);
+    return Napi::Number::New(env, tval / 1000);
   }
   case duckdb::LogicalTypeId::TIME: {
-    if (result->types[col_idx].InternalType() != duckdb::PhysicalType::INT32) {
-      throw runtime_error("expected int32 for time");
+    if (result->types[col_idx].InternalType() != duckdb::PhysicalType::INT64) {
+      throw runtime_error("expected int64 for time");
     }
-    int64_t tval = val.GetValue<int32_t>();
+    int64_t tval = val.GetValue<int64_t>();
     return Napi::Number::New(env, GetTime(tval));
   }
   case duckdb::LogicalTypeId::INTERVAL: {
     return Napi::String::New(env, val.ToString());
   }
+  case duckdb::LogicalTypeId::UTINYINT:
+    return Napi::Number::New(env, val.GetValue<uint8_t>());
+  case duckdb::LogicalTypeId::USMALLINT:
+    return Napi::Number::New(env, val.GetValue<uint16_t>());
+  case duckdb::LogicalTypeId::UINTEGER:
+    // GetValue is not supported for uint32_t, so using the wider type
+    return Napi::Number::New(env, val.GetValue<int64_t>());
   default:
     // default to getting string representation
     return Napi::String::New(env, val.ToString());

diff --git a/addon/result_iterator.h b/addon/result_iterator.h
@@ -12,7 +12,7 @@ class ResultIterator : public Napi::ObjectWrap<ResultIterator> {
   static Napi::Object Init(Napi::Env env, Napi::Object exports);
   ResultIterator(const Napi::CallbackInfo &info);
   static Napi::Object Create();
-  unique_ptr<duckdb::QueryResult> result;
+  duckdb::unique_ptr<duckdb::QueryResult> result;
   ResultFormat rowResultFormat;
   void close();
 
@@ -23,7 +23,7 @@ class ResultIterator : public Napi::ObjectWrap<ResultIterator> {
   Napi::Value GetType(const Napi::CallbackInfo &info);
   Napi::Value Close(const Napi::CallbackInfo &info);
   Napi::Value IsClosed(const Napi::CallbackInfo &info);
-  unique_ptr<duckdb::DataChunk> current_chunk;
+  duckdb::unique_ptr<duckdb::DataChunk> current_chunk;
   uint64_t chunk_offset = 0;
   Napi::Value getCellValue(Napi::Env env, duckdb::idx_t col_idx);
   Napi::Value getRowArray(Napi::Env env);

diff --git a/addon/type-converters.cc b/addon/type-converters.cc
@@ -2,29 +2,31 @@
 #include "duckdb.h"
 #include "duckdb.hpp"
 
-string NodeDuckDB::TypeConverters::convertString(const Napi::Env &env,
-                                                 const Napi::Object &options,
-                                                 const string propertyName) {
+duckdb::string
+NodeDuckDB::TypeConverters::convertString(const Napi::Env &env,
+                                          const Napi::Object &options,
+                                          const std::string propertyName) {
   if (!options.Get(propertyName).IsString()) {
     throw Napi::TypeError::New(env, "Invalid " + propertyName +
                                         ": must be a string");
   }
   return options.Get(propertyName).ToString().Utf8Value();
 }
 
-int32_t NodeDuckDB::TypeConverters::convertNumber(const Napi::Env &env,
-                                                  const Napi::Object &options,
-                                                  const string propertyName) {
+int32_t
+NodeDuckDB::TypeConverters::convertNumber(const Napi::Env &env,
+                                          const Napi::Object &options,
+                                          const std::string propertyName) {
   if (!options.Get(propertyName).IsNumber()) {
     throw Napi::TypeError::New(env, "Invalid " + propertyName +
                                         ": must be a number");
   }
   return options.Get(propertyName).ToNumber().Int32Value();
 }
 
-bool NodeDuckDB::TypeConverters::convertBoolean(const Napi::Env &env,
-                                                const Napi::Object &options,
-                                                const string propertyName) {
+bool NodeDuckDB::TypeConverters::convertBoolean(
+    const Napi::Env &env, const Napi::Object &options,
+    const std::string propertyName) {
   if (!options.Get(propertyName).IsBoolean()) {
     throw Napi::TypeError::New(env, "Invalid " + propertyName +
                                         ": must be a boolean");
@@ -34,9 +36,9 @@ bool NodeDuckDB::TypeConverters::convertBoolean(const Napi::Env &env,
 
 int32_t NodeDuckDB::TypeConverters::convertEnum(const Napi::Env &env,
                                                 const Napi::Object &options,
-                                                const string propertyName,
+                                                const std::string propertyName,
                                                 const int min, const int max) {
-  const string errorMessage =
+  const std::string errorMessage =
       "Invalid " + propertyName + ": must be of appropriate enum type";
   if (!options.Get(propertyName).IsNumber()) {
     throw Napi::TypeError::New(env, errorMessage);

diff --git a/addon/type-converters.h b/addon/type-converters.h
@@ -3,14 +3,15 @@
 
 namespace NodeDuckDB {
 namespace TypeConverters {
-string convertString(const Napi::Env &env, const Napi::Object &options,
-                     const string propertyName);
+duckdb::string convertString(const Napi::Env &env, const Napi::Object &options,
+                             const std::string propertyName);
 int32_t convertNumber(const Napi::Env &env, const Napi::Object &options,
-                      const string propertyName);
+                      const std::string propertyName);
 bool convertBoolean(const Napi::Env &env, const Napi::Object &options,
-                    const string propertyName);
+                    const std::string propertyName);
 int32_t convertEnum(const Napi::Env &env, const Napi::Object &options,
-                    const string propertyName, const int min, const int max);
+                    const std::string propertyName, const int min,
+                    const int max);
 void setDBConfig(const Napi::Env &env, const Napi::Object &config,
                  duckdb::DBConfig &nativeConfig);
 } // namespace TypeConverters

diff --git a/package.json b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "node-duckdb",
-  "version": "0.0.59",
+  "version": "0.0.63",
   "private": false,
   "description": "DuckDB for Node.JS",
   "keywords": [
@@ -39,7 +39,7 @@
     "clang:check": "yarn clang-format --dry-run --Werror addon/**",
     "clang:fix": "yarn clang-format -i addon/**",
     "cleanup:binaries": "rm -rf build prebuilds duckdb",
-    "download-duckdb": "rm -rf duckdb && curl -L https://github.com/cwida/duckdb/archive/v0.2.2.tar.gz > duckdb.tar.gz && tar xf duckdb.tar.gz && mv duckdb-0.2.2 duckdb && rm duckdb.tar.gz",
+    "download-duckdb": "rm -rf duckdb && curl -L https://github.com/cwida/duckdb/archive/v0.2.5.tar.gz > duckdb.tar.gz && tar xf duckdb.tar.gz && mv duckdb-0.2.5 duckdb && rm duckdb.tar.gz",
     "eslint:check": "eslint --ext .js,.json,.ts ./",
     "eslint:fix": "eslint --fix --ext .js,.json,.ts ./",
     "generate-doc": "yarn build:ts && rm -rf temp etc && mkdir etc && yarn api-extractor run --local --verbose &&  yarn api-documenter markdown -i temp -o docs/api && ./docs/replace.sh",

diff --git a/src/tests/async.test.ts b/src/tests/async.test.ts
@@ -6,7 +6,7 @@ const query2 = "SELECT count(*) FROM read_csv_auto('src/tests/test-fixtures/web_
 const expectedResult1 = [
   1,
   "AAAAAAAABAAAAAAA",
-  873244800000,
+  "1997-09-03",
   null,
   2450810,
   2452620,

diff --git a/src/tests/csv.test.ts b/src/tests/csv.test.ts
@@ -33,7 +33,7 @@ describe("executeIterator on csv", () => {
     expect(result.fetchRow()).toMatchObject([
       1,
       "AAAAAAAABAAAAAAA",
-      873244800000,
+      "1997-09-03",
       null,
       2450810,
       2452620,