From d8e1d0bc28d457c7b8f2c6ccbafb9f04ad9abeab Mon Sep 17 00:00:00 2001 From: Mark Raasveldt Date: Mon, 5 Feb 2024 18:18:46 +0100 Subject: [PATCH 1/4] Add more options for opening SQLite databases - busy_timeout and journal_mode --- src/include/sqlite_db.hpp | 3 ++- src/include/storage/sqlite_catalog.hpp | 6 +++--- src/sqlite_db.cpp | 17 +++++++++++++++-- src/sqlite_scanner.cpp | 12 +++++++++--- src/sqlite_storage.cpp | 11 ++++++++++- src/storage/sqlite_catalog.cpp | 6 +++--- src/storage/sqlite_transaction.cpp | 3 +-- test/sql/storage/attach_options.test | 24 ++++++++++++++++++++++++ 8 files changed, 67 insertions(+), 15 deletions(-) create mode 100644 test/sql/storage/attach_options.test diff --git a/src/include/sqlite_db.hpp b/src/include/sqlite_db.hpp index 5350f0d..2858e16 100644 --- a/src/include/sqlite_db.hpp +++ b/src/include/sqlite_db.hpp @@ -9,6 +9,7 @@ #pragma once #include "sqlite_utils.hpp" +#include "storage/sqlite_options.hpp" namespace duckdb { class SQLiteStatement; @@ -29,7 +30,7 @@ class SQLiteDB { sqlite3 *db; public: - static SQLiteDB Open(const string &path, bool is_read_only = true, bool is_shared = false); + static SQLiteDB Open(const string &path, const SQLiteOpenOptions &options, bool is_shared = false); bool TryPrepare(const string &query, SQLiteStatement &result); SQLiteStatement Prepare(const string &query); void Execute(const string &query); diff --git a/src/include/storage/sqlite_catalog.hpp b/src/include/storage/sqlite_catalog.hpp index 05149e5..236923a 100644 --- a/src/include/storage/sqlite_catalog.hpp +++ b/src/include/storage/sqlite_catalog.hpp @@ -9,7 +9,7 @@ #pragma once #include "duckdb/catalog/catalog.hpp" -#include "duckdb/common/enums/access_mode.hpp" +#include "sqlite_options.hpp" #include "sqlite_db.hpp" namespace duckdb { @@ -17,11 +17,11 @@ class SQLiteSchemaEntry; class SQLiteCatalog : public Catalog { public: - explicit SQLiteCatalog(AttachedDatabase &db_p, const string &path, AccessMode access_mode); + explicit SQLiteCatalog(AttachedDatabase &db_p, const string &path, SQLiteOpenOptions options); ~SQLiteCatalog(); string path; - AccessMode access_mode; + SQLiteOpenOptions options; public: void Initialize(bool load_builtin) override; diff --git a/src/sqlite_db.cpp b/src/sqlite_db.cpp index 761b154..d94d251 100644 --- a/src/sqlite_db.cpp +++ b/src/sqlite_db.cpp @@ -28,10 +28,10 @@ SQLiteDB &SQLiteDB::operator=(SQLiteDB &&other) noexcept { return *this; } -SQLiteDB SQLiteDB::Open(const string &path, bool is_read_only, bool is_shared) { +SQLiteDB SQLiteDB::Open(const string &path, const SQLiteOpenOptions &options, bool is_shared) { SQLiteDB result; int flags = SQLITE_OPEN_PRIVATECACHE; - if (is_read_only) { + if (options.access_mode == AccessMode::READ_ONLY) { flags |= SQLITE_OPEN_READONLY; } else { flags |= SQLITE_OPEN_READWRITE | SQLITE_OPEN_CREATE; @@ -46,6 +46,19 @@ SQLiteDB SQLiteDB::Open(const string &path, bool is_read_only, bool is_shared) { if (rc != SQLITE_OK) { throw std::runtime_error("Unable to open database \"" + path + "\": " + string(sqlite3_errstr(rc))); } + // default busy time-out of 5 seconds + if (options.busy_timeout > 0) { + if (options.busy_timeout > NumericLimits::Maximum()) { + throw std::runtime_error("busy_timeout out of range - must be within valid range for type int"); + } + rc = sqlite3_busy_timeout(result.db, int(options.busy_timeout)); + if (rc != SQLITE_OK) { + throw std::runtime_error("Failed to set busy timeout"); + } + } + if (!options.journal_mode.empty()) { + result.Execute("PRAGMA journal_mode=" + KeywordHelper::EscapeQuotes(options.journal_mode, '\'')); + } return result; } diff --git a/src/sqlite_scanner.cpp b/src/sqlite_scanner.cpp index 23d43ce..34ff3de 100644 --- a/src/sqlite_scanner.cpp +++ b/src/sqlite_scanner.cpp @@ -50,7 +50,9 @@ static unique_ptr SqliteBind(ClientContext &context, TableFunction SQLiteDB db; SQLiteStatement stmt; - db = SQLiteDB::Open(result->file_name); + SQLiteOpenOptions options; + options.access_mode = AccessMode::READ_ONLY; + db = SQLiteDB::Open(result->file_name, options); ColumnList columns; vector> constraints; @@ -90,7 +92,9 @@ static void SqliteInitInternal(ClientContext &context, const SqliteBindData &bin // function local_state.stmt.Close(); if (!local_state.db) { - local_state.owned_db = SQLiteDB::Open(bind_data.file_name.c_str()); + SQLiteOpenOptions options; + options.access_mode = AccessMode::READ_ONLY; + local_state.owned_db = SQLiteDB::Open(bind_data.file_name.c_str(), options); local_state.db = &local_state.owned_db; } @@ -292,7 +296,9 @@ static void AttachFunction(ClientContext &context, TableFunctionInput &data_p, D return; } - SQLiteDB db = SQLiteDB::Open(data.file_name); + SQLiteOpenOptions options; + options.access_mode = AccessMode::READ_ONLY; + SQLiteDB db = SQLiteDB::Open(data.file_name, options); auto dconn = Connection(context.db->GetDatabase(context)); { auto tables = db.GetTables(); diff --git a/src/sqlite_storage.cpp b/src/sqlite_storage.cpp index d65bf18..c1522d7 100644 --- a/src/sqlite_storage.cpp +++ b/src/sqlite_storage.cpp @@ -14,7 +14,16 @@ namespace duckdb { static unique_ptr SQLiteAttach(StorageExtensionInfo *storage_info, AttachedDatabase &db, const string &name, AttachInfo &info, AccessMode access_mode) { - return make_uniq(db, info.path, access_mode); + SQLiteOpenOptions options; + options.access_mode = access_mode; + for(auto &entry : info.options) { + if (StringUtil::CIEquals(entry.first, "busy_timeout")) { + options.busy_timeout = entry.second.GetValue(); + } else if (StringUtil::CIEquals(entry.first, "journal_mode")) { + options.journal_mode = entry.second.ToString(); + } + } + return make_uniq(db, info.path, std::move(options)); } static unique_ptr SQLiteCreateTransactionManager(StorageExtensionInfo *storage_info, diff --git a/src/storage/sqlite_catalog.cpp b/src/storage/sqlite_catalog.cpp index fad530c..16ad70f 100644 --- a/src/storage/sqlite_catalog.cpp +++ b/src/storage/sqlite_catalog.cpp @@ -6,10 +6,10 @@ namespace duckdb { -SQLiteCatalog::SQLiteCatalog(AttachedDatabase &db_p, const string &path, AccessMode access_mode) - : Catalog(db_p), path(path), access_mode(access_mode), in_memory(path == ":memory:"), active_in_memory(false) { +SQLiteCatalog::SQLiteCatalog(AttachedDatabase &db_p, const string &path, SQLiteOpenOptions options_p) + : Catalog(db_p), path(path), options(std::move(options_p)), in_memory(path == ":memory:"), active_in_memory(false) { if (InMemory()) { - in_memory_db = SQLiteDB::Open(path, false, true); + in_memory_db = SQLiteDB::Open(path, options, true); } } diff --git a/src/storage/sqlite_transaction.cpp b/src/storage/sqlite_transaction.cpp index 300a1bd..3e7172e 100644 --- a/src/storage/sqlite_transaction.cpp +++ b/src/storage/sqlite_transaction.cpp @@ -17,8 +17,7 @@ SQLiteTransaction::SQLiteTransaction(SQLiteCatalog &sqlite_catalog, TransactionM db = sqlite_catalog.GetInMemoryDatabase(); } else { // on-disk database - open a new database connection - owned_db = SQLiteDB::Open(sqlite_catalog.path, - sqlite_catalog.access_mode == AccessMode::READ_ONLY ? true : false, true); + owned_db = SQLiteDB::Open(sqlite_catalog.path, sqlite_catalog.options, true); db = &owned_db; } } diff --git a/test/sql/storage/attach_options.test b/test/sql/storage/attach_options.test new file mode 100644 index 0000000..ab6a752 --- /dev/null +++ b/test/sql/storage/attach_options.test @@ -0,0 +1,24 @@ +# name: test/sql/storage/attach_options.test +# description: +# group: [sqlite_storage] + +require sqlite_scanner + +statement error +ATTACH ':memory:' AS mem (TYPE SQLITE, BUSY_TIMEOUT 'hello') +---- +Could not convert string + +statement error +ATTACH ':memory:' AS mem (TYPE SQLITE, BUSY_TIMEOUT 99999999999) +---- +busy_timeout out of range + +statement ok +ATTACH ':memory:' AS mem (TYPE SQLITE, BUSY_TIMEOUT 0) + +statement ok +DETACH mem + +statement ok +ATTACH ':memory:' AS mem (TYPE SQLITE, JOURNAL_MODE 'WAL') From 47d83828bb14ef5e0e4e53fc8096dac098eaaaf7 Mon Sep 17 00:00:00 2001 From: Mark Raasveldt Date: Tue, 6 Feb 2024 11:24:32 +0100 Subject: [PATCH 2/4] Add missing file --- src/include/storage/sqlite_options.hpp | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) create mode 100644 src/include/storage/sqlite_options.hpp diff --git a/src/include/storage/sqlite_options.hpp b/src/include/storage/sqlite_options.hpp new file mode 100644 index 0000000..9f7cbeb --- /dev/null +++ b/src/include/storage/sqlite_options.hpp @@ -0,0 +1,26 @@ +//===----------------------------------------------------------------------===// +// DuckDB +// +// storage/sqlite_options.hpp +// +// +//===----------------------------------------------------------------------===// + +#pragma once + +#include "duckdb/common/common.hpp" +#include "duckdb/common/enums/access_mode.hpp" + +namespace duckdb { + +struct SQLiteOpenOptions { + // access mode + AccessMode access_mode = AccessMode::READ_WRITE; + // busy time-out in ms + idx_t busy_timeout = 5000; + // journal mode + string journal_mode; +}; + + +} // namespace duckdb From f43a101f0d054bdae27df44919822a6d73726a41 Mon Sep 17 00:00:00 2001 From: Mark Raasveldt Date: Tue, 6 Feb 2024 11:25:59 +0100 Subject: [PATCH 3/4] Upgrade duckdb --- duckdb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/duckdb b/duckdb index a056c8e..4f04041 160000 --- a/duckdb +++ b/duckdb @@ -1 +1 @@ -Subproject commit a056c8e0a29f2f38d245afeb9e48cc308711aa21 +Subproject commit 4f040413468b2fee84ce4d124dfb058378db1456 From dd814d2edd92e8a4a388efe6a9304684c0a86cd0 Mon Sep 17 00:00:00 2001 From: Mark Raasveldt Date: Tue, 6 Feb 2024 13:21:43 +0100 Subject: [PATCH 4/4] Update DuckDB in CI --- .github/workflows/MainDistributionPipeline.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/MainDistributionPipeline.yml b/.github/workflows/MainDistributionPipeline.yml index ecc7a59..88bea62 100644 --- a/.github/workflows/MainDistributionPipeline.yml +++ b/.github/workflows/MainDistributionPipeline.yml @@ -27,7 +27,7 @@ jobs: uses: duckdb/duckdb/.github/workflows/_extension_deploy.yml@a056c8e0a29f2f38d245afeb9e48cc308711aa21 secrets: inherit with: - duckdb_version: a056c8e0a2 + duckdb_version: 4f040413468b2fee84ce4d124dfb058378db1456 extension_name: sqlite_scanner exclude_archs: 'wasm_mvp;wasm_eh;wasm_threads' deploy_latest: ${{ startsWith(github.ref, 'refs/tags/v') || github.ref == 'refs/heads/main' }}