From ecdd83e09203142cebd08efa4723304b487a7d61 Mon Sep 17 00:00:00 2001 From: Zuleykha Pavlichenkova Date: Thu, 29 Aug 2024 14:51:00 +0200 Subject: [PATCH 01/15] fix REGEX not supported anymore for valid queries (only statement error) #2889 --- test/sqlite/result_helper.cpp | 9 ++++----- test/sqlite/result_helper.hpp | 3 ++- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/test/sqlite/result_helper.cpp b/test/sqlite/result_helper.cpp index 2911c3d06b77..43da9499ff94 100644 --- a/test/sqlite/result_helper.cpp +++ b/test/sqlite/result_helper.cpp @@ -283,7 +283,7 @@ bool TestResultHelper::CheckStatementResult(const Statement &statement, ExecuteC bool success = false; if (StringUtil::StartsWith(statement.expected_error, ":") || StringUtil::StartsWith(statement.expected_error, ":")) { - success = MatchesRegex(logger, result, statement.expected_error); + success = MatchesRegex(logger, result.ToString(), statement.expected_error); } if (!success) { logger.ExpectedErrorMismatch(statement.expected_error, result); @@ -450,7 +450,7 @@ bool TestResultHelper::CompareValues(SQLLogicTestLogger &logger, MaterializedQue return true; } if (StringUtil::StartsWith(rvalue_str, ":") || StringUtil::StartsWith(rvalue_str, ":")) { - if (MatchesRegex(logger, result, rvalue_str)) { + if (MatchesRegex(logger, lvalue_str, rvalue_str)) { return true; } } @@ -521,7 +521,7 @@ bool TestResultHelper::CompareValues(SQLLogicTestLogger &logger, MaterializedQue return true; } -bool TestResultHelper::MatchesRegex(SQLLogicTestLogger &logger, MaterializedQueryResult &result, string rvalue_str) { +bool TestResultHelper::MatchesRegex(SQLLogicTestLogger &logger, string lvalue_str, string rvalue_str) { bool want_match = StringUtil::StartsWith(rvalue_str, ":"); string regex_str = StringUtil::Replace(StringUtil::Replace(rvalue_str, ":", ""), ":", ""); @@ -536,8 +536,7 @@ bool TestResultHelper::MatchesRegex(SQLLogicTestLogger &logger, MaterializedQuer logger.PrintLineSep(); return false; } - auto resString = result.ToString(); - bool regex_matches = RE2::FullMatch(result.ToString(), re); + bool regex_matches = RE2::FullMatch(lvalue_str, re); if ((want_match && regex_matches) || (!want_match && !regex_matches)) { return true; } diff --git a/test/sqlite/result_helper.hpp b/test/sqlite/result_helper.hpp index f313e9b0c087..d5ee19bf32c2 100644 --- a/test/sqlite/result_helper.hpp +++ b/test/sqlite/result_helper.hpp @@ -33,7 +33,8 @@ class TestResultHelper { static bool ResultIsHash(const string &result); static bool ResultIsFile(string result); - bool MatchesRegex(SQLLogicTestLogger &logger, MaterializedQueryResult &result, string rvalue_str); + bool MatchesRegex(SQLLogicTestLogger &logger, string lvalue_str, string rvalue_str); + // bool MatchesRegex(SQLLogicTestLogger &logger, MaterializedQueryResult &result, string rvalue_str); bool CompareValues(SQLLogicTestLogger &logger, MaterializedQueryResult &result, string lvalue_str, string rvalue_str, idx_t current_row, idx_t current_column, vector &values, idx_t expected_column_count, bool row_wise, vector &result_values); From ed3329bf51639147f0eb4c8f2d5f9c1b14fb3c25 Mon Sep 17 00:00:00 2001 From: Zuleykha Pavlichenkova Date: Fri, 30 Aug 2024 09:18:40 +0200 Subject: [PATCH 02/15] add regex_syntax test --- test/fuzzer/duckfuzz/regex_syntax_2889.test | 38 +++++++++++++++++++++ test/sqlite/result_helper.hpp | 1 - 2 files changed, 38 insertions(+), 1 deletion(-) create mode 100644 test/fuzzer/duckfuzz/regex_syntax_2889.test diff --git a/test/fuzzer/duckfuzz/regex_syntax_2889.test b/test/fuzzer/duckfuzz/regex_syntax_2889.test new file mode 100644 index 000000000000..341435e84a09 --- /dev/null +++ b/test/fuzzer/duckfuzz/regex_syntax_2889.test @@ -0,0 +1,38 @@ +# name: test/fuzzer/duckfuzz/regex_syntax_2889.test +# description: Test REGEX syntax +# group: [duckfuzz] + +query I +SELECT 42 +---- +42 + +query I +SELECT 42 +---- +:42 + +query I +SELECT 42 +---- +:.* + +query I +SELECT 42 +---- +:quack + +statement error +SELECT +---- +SELECT clause without selection list + +statement error +SELECT +---- +:Parser Error.*clause without selection list.* + +statement error +SELECT +---- +:Parser Error \ No newline at end of file diff --git a/test/sqlite/result_helper.hpp b/test/sqlite/result_helper.hpp index d5ee19bf32c2..6982fab0ba6e 100644 --- a/test/sqlite/result_helper.hpp +++ b/test/sqlite/result_helper.hpp @@ -34,7 +34,6 @@ class TestResultHelper { static bool ResultIsFile(string result); bool MatchesRegex(SQLLogicTestLogger &logger, string lvalue_str, string rvalue_str); - // bool MatchesRegex(SQLLogicTestLogger &logger, MaterializedQueryResult &result, string rvalue_str); bool CompareValues(SQLLogicTestLogger &logger, MaterializedQueryResult &result, string lvalue_str, string rvalue_str, idx_t current_row, idx_t current_column, vector &values, idx_t expected_column_count, bool row_wise, vector &result_values); From 2e3d796c9a701473945bad71abcb2c60910d3d14 Mon Sep 17 00:00:00 2001 From: Tishj Date: Wed, 11 Sep 2024 15:18:07 +0200 Subject: [PATCH 03/15] add repository/repository_url and version to 'install_extension' --- tools/pythonpkg/duckdb-stubs/__init__.pyi | 4 +- tools/pythonpkg/duckdb_python.cpp | 7 ++- .../pythonpkg/scripts/connection_methods.json | 17 ++++++- .../pyconnection/pyconnection.hpp | 4 +- tools/pythonpkg/src/pyconnection.cpp | 46 +++++++++++++++++-- .../extensions/test_extensions_loading.py | 21 +++++++++ 6 files changed, 88 insertions(+), 11 deletions(-) diff --git a/tools/pythonpkg/duckdb-stubs/__init__.pyi b/tools/pythonpkg/duckdb-stubs/__init__.pyi index 818b7be48d1c..26c88b10deae 100644 --- a/tools/pythonpkg/duckdb-stubs/__init__.pyi +++ b/tools/pythonpkg/duckdb-stubs/__init__.pyi @@ -344,7 +344,7 @@ class DuckDBPyConnection: def get_substrait_json(self, query: str, *, enable_optimizer: bool = True) -> str: ... def from_substrait_json(self, json: str) -> DuckDBPyRelation: ... def get_table_names(self, query: str) -> Set[str]: ... - def install_extension(self, extension: str, *, force_install: bool = False) -> None: ... + def install_extension(self, extension: str, *, force_install: bool = False, repository: Optional[str] = None, repository_url: Optional[str] = None, version: Optional[str] = None) -> None: ... def load_extension(self, extension: str) -> None: ... # END OF CONNECTION METHODS @@ -670,7 +670,7 @@ def get_substrait(query: str, *, enable_optimizer: bool = True, connection: Duck def get_substrait_json(query: str, *, enable_optimizer: bool = True, connection: DuckDBPyConnection = ...) -> str: ... def from_substrait_json(json: str, *, connection: DuckDBPyConnection = ...) -> DuckDBPyRelation: ... def get_table_names(query: str, *, connection: DuckDBPyConnection = ...) -> Set[str]: ... -def install_extension(extension: str, *, force_install: bool = False, connection: DuckDBPyConnection = ...) -> None: ... +def install_extension(extension: str, *, force_install: bool = False, repository: Optional[str] = None, repository_url: Optional[str] = None, version: Optional[str] = None, connection: DuckDBPyConnection = ...) -> None: ... def load_extension(extension: str, *, connection: DuckDBPyConnection = ...) -> None: ... def project(df: pandas.DataFrame, *args: str, groups: str = "", connection: DuckDBPyConnection = ...) -> DuckDBPyRelation: ... def distinct(df: pandas.DataFrame, *, connection: DuckDBPyConnection = ...) -> DuckDBPyRelation: ... diff --git a/tools/pythonpkg/duckdb_python.cpp b/tools/pythonpkg/duckdb_python.cpp index 281619a15e4d..3c9fdb1f2cf4 100644 --- a/tools/pythonpkg/duckdb_python.cpp +++ b/tools/pythonpkg/duckdb_python.cpp @@ -805,13 +805,16 @@ static void InitializeConnectionMethods(py::module_ &m) { py::arg("connection") = py::none()); m.def( "install_extension", - [](const string &extension, bool force_install = false, shared_ptr conn = nullptr) { + [](const string &extension, bool force_install = false, const py::object &repository = py::none(), + const py::object &repository_url = py::none(), const py::object &version = py::none(), + shared_ptr conn = nullptr) { if (!conn) { conn = DuckDBPyConnection::DefaultConnection(); } - conn->InstallExtension(extension, force_install); + conn->InstallExtension(extension, force_install, repository, repository_url, version); }, "Install an extension by name", py::arg("extension"), py::kw_only(), py::arg("force_install") = false, + py::arg("repository") = py::none(), py::arg("repository_url") = py::none(), py::arg("version") = py::none(), py::arg("connection") = py::none()); m.def( "load_extension", diff --git a/tools/pythonpkg/scripts/connection_methods.json b/tools/pythonpkg/scripts/connection_methods.json index b3952065111b..631e9ccd7ce4 100644 --- a/tools/pythonpkg/scripts/connection_methods.json +++ b/tools/pythonpkg/scripts/connection_methods.json @@ -1065,7 +1065,7 @@ { "name": "install_extension", "function": "InstallExtension", - "docs": "Install an extension by name", + "docs": "Install an extension by name, with an optional version and/or repository to get the extension from", "args": [ { "name": "extension", @@ -1077,6 +1077,21 @@ "name": "force_install", "default": "False", "type": "bool" + }, + { + "name": "repository", + "default": "None", + "type": "Optional[str]" + }, + { + "name": "repository_url", + "default": "None", + "type": "Optional[str]" + }, + { + "name": "version", + "default": "None", + "type": "Optional[str]" } ], "return": "None" diff --git a/tools/pythonpkg/src/include/duckdb_python/pyconnection/pyconnection.hpp b/tools/pythonpkg/src/include/duckdb_python/pyconnection/pyconnection.hpp index c430e61c1ba4..13acaffe5883 100644 --- a/tools/pythonpkg/src/include/duckdb_python/pyconnection/pyconnection.hpp +++ b/tools/pythonpkg/src/include/duckdb_python/pyconnection/pyconnection.hpp @@ -218,7 +218,9 @@ struct DuckDBPyConnection : public enable_shared_from_this { shared_ptr RegisterPythonObject(const string &name, const py::object &python_object); - void InstallExtension(const string &extension, bool force_install = false); + void InstallExtension(const string &extension, bool force_install = false, + const py::object &repository = py::none(), const py::object &repository_url = py::none(), + const py::object &version = py::none()); void LoadExtension(const string &extension); diff --git a/tools/pythonpkg/src/pyconnection.cpp b/tools/pythonpkg/src/pyconnection.cpp index 9f3c92a0f91c..2e2f8c35b67f 100644 --- a/tools/pythonpkg/src/pyconnection.cpp +++ b/tools/pythonpkg/src/pyconnection.cpp @@ -58,6 +58,7 @@ #include "duckdb/main/relation/materialized_relation.hpp" #include "duckdb/main/relation/query_relation.hpp" #include "duckdb/main/extension_util.hpp" +#include "duckdb/parser/statement/load_statement.hpp" #include @@ -276,7 +277,8 @@ static void InitializeConnectionMethods(py::class_(); + install_statement->info = make_uniq(); + auto &info = *install_statement->info; + + info.filename = extension; + + const bool has_repository = !py::none().is(repository); + const bool has_repository_url = !py::none().is(repository_url); + if (has_repository && has_repository_url) { + throw InvalidInputException( + "Both 'repository' and 'repository_url' are set which is not allowed, please pick one or the other"); + } + string repository_string; + if (has_repository) { + repository_string = py::str(repository); + } else if (has_repository_url) { + repository_string = py::str(repository_url); + } + + if ((has_repository || has_repository_url) && repository_string.empty()) { + throw InvalidInputException("The provided 'repository' or 'repository_url' can not be empty!"); + } + + string version_string; + if (!py::none().is(version)) { + version_string = py::str(version); + if (version_string.empty()) { + throw InvalidInputException("The provided 'version' can not be empty!"); + } + } + + info.repository = repository_string; + info.repo_is_alias = repository_string.empty() ? false : has_repository; + info.version = version_string; + info.load_type = force_install ? LoadType::FORCE_INSTALL : LoadType::INSTALL; + connection.Query(std::move(install_statement)); } void DuckDBPyConnection::LoadExtension(const string &extension) { diff --git a/tools/pythonpkg/tests/extensions/test_extensions_loading.py b/tools/pythonpkg/tests/extensions/test_extensions_loading.py index c045e4a330cc..bcf54ef5defa 100644 --- a/tools/pythonpkg/tests/extensions/test_extensions_loading.py +++ b/tools/pythonpkg/tests/extensions/test_extensions_loading.py @@ -36,3 +36,24 @@ def test_install_non_existent_extension(): assert value.reason == 'Not Found' assert 'Example Domain' in value.body assert 'Content-Length' in value.headers + + +def test_install_misuse_errors(duckdb_cursor): + with pytest.raises( + duckdb.InvalidInputException, + match="Both 'repository' and 'repository_url' are set which is not allowed, please pick one or the other", + ): + duckdb_cursor.install_extension('name', repository='hello', repository_url='hello.com') + + with pytest.raises( + duckdb.InvalidInputException, match="The provided 'repository' or 'repository_url' can not be empty!" + ): + duckdb_cursor.install_extension('name', repository_url='') + + with pytest.raises( + duckdb.InvalidInputException, match="The provided 'repository' or 'repository_url' can not be empty!" + ): + duckdb_cursor.install_extension('name', repository='') + + with pytest.raises(duckdb.InvalidInputException, match="The provided 'version' can not be empty!"): + duckdb_cursor.install_extension('name', version='') From 98bdcce82021dfe356379efaefe2aa3371a4fb14 Mon Sep 17 00:00:00 2001 From: Tishj Date: Wed, 11 Sep 2024 16:22:54 +0200 Subject: [PATCH 04/15] updated the docs without running make generate-files --- tools/pythonpkg/duckdb_python.cpp | 6 +++--- tools/pythonpkg/src/pyconnection.cpp | 3 ++- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/tools/pythonpkg/duckdb_python.cpp b/tools/pythonpkg/duckdb_python.cpp index 3c9fdb1f2cf4..b08300a1e6c1 100644 --- a/tools/pythonpkg/duckdb_python.cpp +++ b/tools/pythonpkg/duckdb_python.cpp @@ -813,9 +813,9 @@ static void InitializeConnectionMethods(py::module_ &m) { } conn->InstallExtension(extension, force_install, repository, repository_url, version); }, - "Install an extension by name", py::arg("extension"), py::kw_only(), py::arg("force_install") = false, - py::arg("repository") = py::none(), py::arg("repository_url") = py::none(), py::arg("version") = py::none(), - py::arg("connection") = py::none()); + "Install an extension by name, with an optional version and/or repository to get the extension from", + py::arg("extension"), py::kw_only(), py::arg("force_install") = false, py::arg("repository") = py::none(), + py::arg("repository_url") = py::none(), py::arg("version") = py::none(), py::arg("connection") = py::none()); m.def( "load_extension", [](const string &extension, shared_ptr conn = nullptr) { diff --git a/tools/pythonpkg/src/pyconnection.cpp b/tools/pythonpkg/src/pyconnection.cpp index 2e2f8c35b67f..c986e98ba325 100644 --- a/tools/pythonpkg/src/pyconnection.cpp +++ b/tools/pythonpkg/src/pyconnection.cpp @@ -276,7 +276,8 @@ static void InitializeConnectionMethods(py::class_ Date: Thu, 12 Sep 2024 10:09:27 +0200 Subject: [PATCH 05/15] check for error and throw the error caught in the QueryResult --- tools/pythonpkg/src/pyconnection.cpp | 5 ++++- .../tests/extensions/test_extensions_loading.py | 16 ++++++++-------- 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/tools/pythonpkg/src/pyconnection.cpp b/tools/pythonpkg/src/pyconnection.cpp index c986e98ba325..77fff28aa493 100644 --- a/tools/pythonpkg/src/pyconnection.cpp +++ b/tools/pythonpkg/src/pyconnection.cpp @@ -1741,7 +1741,10 @@ void DuckDBPyConnection::InstallExtension(const string &extension, bool force_in info.repo_is_alias = repository_string.empty() ? false : has_repository; info.version = version_string; info.load_type = force_install ? LoadType::FORCE_INSTALL : LoadType::INSTALL; - connection.Query(std::move(install_statement)); + auto res = connection.Query(std::move(install_statement)); + if (res->HasError()) { + res->ThrowError(); + } } void DuckDBPyConnection::LoadExtension(const string &extension) { diff --git a/tools/pythonpkg/tests/extensions/test_extensions_loading.py b/tools/pythonpkg/tests/extensions/test_extensions_loading.py index bcf54ef5defa..2b4eab0c5f0f 100644 --- a/tools/pythonpkg/tests/extensions/test_extensions_loading.py +++ b/tools/pythonpkg/tests/extensions/test_extensions_loading.py @@ -28,14 +28,14 @@ def test_install_non_existent_extension(): with raises(duckdb.IOException) as exc: conn.install_extension('non-existent') - if not isinstance(exc, duckdb.HTTPException): - pytest.skip(reason='This test does not throw an HTTPException, only an IOException') - value = exc.value - - assert value.status_code == 404 - assert value.reason == 'Not Found' - assert 'Example Domain' in value.body - assert 'Content-Length' in value.headers + if not isinstance(exc, duckdb.HTTPException): + pytest.skip(reason='This test does not throw an HTTPException, only an IOException') + value = exc.value + + assert value.status_code == 404 + assert value.reason == 'Not Found' + assert 'Example Domain' in value.body + assert 'Content-Length' in value.headers def test_install_misuse_errors(duckdb_cursor): From 4f92f1616328d61e030e4f71cb631920b027b319 Mon Sep 17 00:00:00 2001 From: Carlo Piovesan Date: Thu, 12 Sep 2024 11:28:21 +0200 Subject: [PATCH 06/15] Remove buffer_manager_allocate.patch and bump spatial --- .github/config/out_of_tree_extensions.cmake | 3 +-- .../spatial/buffer_manager_allocate.patch | 24 ------------------- 2 files changed, 1 insertion(+), 26 deletions(-) delete mode 100644 .github/patches/extensions/spatial/buffer_manager_allocate.patch diff --git a/.github/config/out_of_tree_extensions.cmake b/.github/config/out_of_tree_extensions.cmake index d95657a01a4c..6e1af53494c9 100644 --- a/.github/config/out_of_tree_extensions.cmake +++ b/.github/config/out_of_tree_extensions.cmake @@ -101,10 +101,9 @@ endif() duckdb_extension_load(spatial DONT_LINK LOAD_TESTS GIT_URL https://github.com/duckdb/duckdb_spatial.git - GIT_TAG 58e0fcd09f2306803da36c4b1e8a66bb1e263316 + GIT_TAG 4107eb788f933c9e268b52f6f927a6b36b9ea440 INCLUDE_DIR spatial/include TEST_DIR test/sql - APPLY_PATCHES ) ################# SQLITE_SCANNER diff --git a/.github/patches/extensions/spatial/buffer_manager_allocate.patch b/.github/patches/extensions/spatial/buffer_manager_allocate.patch deleted file mode 100644 index d64063e9dd19..000000000000 --- a/.github/patches/extensions/spatial/buffer_manager_allocate.patch +++ /dev/null @@ -1,24 +0,0 @@ -diff --git a/spatial/include/spatial/core/util/managed_collection.hpp b/spatial/include/spatial/core/util/managed_collection.hpp -index 3535aff..dbf21f1 100644 ---- a/spatial/include/spatial/core/util/managed_collection.hpp -+++ b/spatial/include/spatial/core/util/managed_collection.hpp -@@ -95,7 +95,8 @@ void ManagedCollection::InitializeAppend(ManagedCollectionAppendState &state, - state.block = &blocks.back(); - state.block->item_count = 0; - state.block->item_capacity = block_capacity; -- state.handle = manager.Allocate(MemoryTag::EXTENSION, block_size, true, &state.block->handle); -+ state.handle = manager.Allocate(MemoryTag::EXTENSION, block_size, true); -+ state.block->handle = state.handle.GetBlockHandle(); - } - } - -@@ -108,7 +109,8 @@ void ManagedCollection::Append(ManagedCollectionAppendState &state, const T * - state.block = &blocks.back(); - state.block->item_count = 0; - state.block->item_capacity = block_capacity; -- state.handle = manager.Allocate(MemoryTag::EXTENSION, block_size, true, &state.block->handle); -+ state.handle = manager.Allocate(MemoryTag::EXTENSION, block_size, true); -+ state.block->handle = state.handle.GetBlockHandle(); - } - - // Compute how much we can copy before the block is full or we run out of elements From 594a1412c73e8603a836c3894e463c15d611ebe6 Mon Sep 17 00:00:00 2001 From: Sam Ansmink Date: Thu, 12 Sep 2024 15:22:05 +0200 Subject: [PATCH 07/15] add missing azure secret providers for autoloading --- scripts/generate_extensions_function.py | 2 ++ src/include/duckdb/main/extension_entries.hpp | 17 ++++++++++++----- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/scripts/generate_extensions_function.py b/scripts/generate_extensions_function.py index d5c5ead93289..ed30c115d09d 100644 --- a/scripts/generate_extensions_function.py +++ b/scripts/generate_extensions_function.py @@ -519,8 +519,10 @@ def write_header(data: ExtensionData): {"s3/credential_chain", "aws"}, {"gcs/credential_chain", "aws"}, {"r2/credential_chain", "aws"}, + {"azure/access_token", "azure"}, {"azure/config", "azure"}, {"azure/credential_chain", "azure"}, + {"azure/service_principal", "azure"}, {"huggingface/config", "httfps"}, {"huggingface/credential_chain", "httpfs"}, {"bearer/config", "httpfs"} diff --git a/src/include/duckdb/main/extension_entries.hpp b/src/include/duckdb/main/extension_entries.hpp index 7be9db0c057f..e574ef287700 100644 --- a/src/include/duckdb/main/extension_entries.hpp +++ b/src/include/duckdb/main/extension_entries.hpp @@ -511,11 +511,18 @@ static constexpr ExtensionEntry EXTENSION_SECRET_TYPES[] = { // Note: these are currently hardcoded in scripts/generate_extensions_function.py // TODO: automate by passing though to script via duckdb static constexpr ExtensionEntry EXTENSION_SECRET_PROVIDERS[] = { - {"s3/config", "httpfs"}, {"gcs/config", "httpfs"}, - {"r2/config", "httpfs"}, {"s3/credential_chain", "aws"}, - {"gcs/credential_chain", "aws"}, {"r2/credential_chain", "aws"}, - {"azure/config", "azure"}, {"azure/credential_chain", "azure"}, - {"huggingface/config", "httfps"}, {"huggingface/credential_chain", "httpfs"}, + {"s3/config", "httpfs"}, + {"gcs/config", "httpfs"}, + {"r2/config", "httpfs"}, + {"s3/credential_chain", "aws"}, + {"gcs/credential_chain", "aws"}, + {"r2/credential_chain", "aws"}, + {"azure/access_token", "azure"}, + {"azure/config", "azure"}, + {"azure/credential_chain", "azure"}, + {"azure/service_principal", "azure"}, + {"huggingface/config", "httfps"}, + {"huggingface/credential_chain", "httpfs"}, {"bearer/config", "httpfs"}}; // EXTENSION_SECRET_PROVIDERS static constexpr const char *AUTOLOADABLE_EXTENSIONS[] = { From df86a0fdf5c795283540e3ce54e91c24fe35beca Mon Sep 17 00:00:00 2001 From: Tishj Date: Thu, 12 Sep 2024 17:54:25 +0200 Subject: [PATCH 08/15] off by one error, small (2 bytes) bit length didn't cast to BLOB properly --- src/common/types/bit.cpp | 2 +- test/sql/cast/test_bit_cast.test | 10 ++++++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/src/common/types/bit.cpp b/src/common/types/bit.cpp index ebd4ec3ac6e8..f263c2c42975 100644 --- a/src/common/types/bit.cpp +++ b/src/common/types/bit.cpp @@ -180,7 +180,7 @@ void Bit::BitToBlob(string_t bit, string_t &output_blob) { idx_t size = output_blob.GetSize(); output[0] = UnsafeNumericCast(GetFirstByte(bit)); - if (size > 2) { + if (size >= 2) { ++output; // First byte in bitstring contains amount of padded bits, // second byte in bitstring is the padded byte, diff --git a/test/sql/cast/test_bit_cast.test b/test/sql/cast/test_bit_cast.test index 421ce022e688..76582943af74 100644 --- a/test/sql/cast/test_bit_cast.test +++ b/test/sql/cast/test_bit_cast.test @@ -196,6 +196,16 @@ SELECT '1111'::BIT::BLOB; ---- \x0F +query I +select 'ab'::BLOB::BIT::BLOB; +---- +ab + +query I +select 'a'::BLOB::BIT::BLOB; +---- +a + query I SELECT bitstring('1111', 33)::BLOB; ---- From 38ffc57708f70e020ce2a09fdaa6ad2435d659b4 Mon Sep 17 00:00:00 2001 From: flashmouse <522247671@qq.com> Date: Fri, 13 Sep 2024 00:25:50 +0800 Subject: [PATCH 09/15] minmax use default collation --- .../aggregate/distributive/minmax.cpp | 3 +- test/issues/general/test_13824.test | 37 +++++++++++++++++++ 2 files changed, 39 insertions(+), 1 deletion(-) create mode 100644 test/issues/general/test_13824.test diff --git a/src/core_functions/aggregate/distributive/minmax.cpp b/src/core_functions/aggregate/distributive/minmax.cpp index 9c8db50bb5e3..dba09e5aa0a8 100644 --- a/src/core_functions/aggregate/distributive/minmax.cpp +++ b/src/core_functions/aggregate/distributive/minmax.cpp @@ -4,6 +4,7 @@ #include "duckdb/common/vector_operations/vector_operations.hpp" #include "duckdb/common/operator/comparison_operators.hpp" #include "duckdb/common/types/null_value.hpp" +#include "duckdb/main/config.hpp" #include "duckdb/planner/expression.hpp" #include "duckdb/planner/expression/bound_comparison_expression.hpp" #include "duckdb/planner/expression_binder.hpp" @@ -330,7 +331,7 @@ unique_ptr BindMinMax(ClientContext &context, AggregateFunction &f vector> &arguments) { if (arguments[0]->return_type.id() == LogicalTypeId::VARCHAR) { auto str_collation = StringType::GetCollation(arguments[0]->return_type); - if (!str_collation.empty()) { + if (!str_collation.empty() || !DBConfig::GetConfig(context).options.collation.empty()) { // If aggr function is min/max and uses collations, replace bound_function with arg_min/arg_max // to make sure the result's correctness. string function_name = function.name == "min" ? "arg_min" : "arg_max"; diff --git a/test/issues/general/test_13824.test b/test/issues/general/test_13824.test new file mode 100644 index 000000000000..abaa64c43ce2 --- /dev/null +++ b/test/issues/general/test_13824.test @@ -0,0 +1,37 @@ +# name: test/issues/general/test_13824.test +# description: min() and max() should use default collation +# group: [general] + +require icu + +statement ok +PRAGMA enable_verification + +statement ok +create table test(id int, name text) + +statement ok +insert into test values (1, 'a'), (2, 'b'), (3, 'c'), (4, 'A'), (5, 'G'), (6, 'd') + +query I +select min(name) from test +---- +A + +query I +select max(name) from test +---- +d + +statement ok +set default_collation = 'EN_US'; + +query I +select min(name) from test +---- +a + +query I +select max(name) from test +---- +G \ No newline at end of file From 87d33a3333fbfbc38316440781ac52929d961d69 Mon Sep 17 00:00:00 2001 From: Mark Raasveldt Date: Fri, 13 Sep 2024 14:50:23 +0200 Subject: [PATCH 10/15] Only bind to SQL value functions if there is no alias with this name present we can bind to instead --- src/include/duckdb/planner/expression_binder.hpp | 2 +- .../planner/expression_binder/select_binder.hpp | 1 + src/planner/expression_binder/select_binder.cpp | 9 +++++++++ test/sql/parser/test_value_functions.test | 13 +++++++++++++ 4 files changed, 24 insertions(+), 1 deletion(-) diff --git a/src/include/duckdb/planner/expression_binder.hpp b/src/include/duckdb/planner/expression_binder.hpp index eded041bc8e1..8bdb03b82ad3 100644 --- a/src/include/duckdb/planner/expression_binder.hpp +++ b/src/include/duckdb/planner/expression_binder.hpp @@ -186,7 +186,7 @@ class ExpressionBinder { const optional_ptr bind_lambda_function, const LogicalType &list_child_type); - static unique_ptr GetSQLValueFunction(const string &column_name); + virtual unique_ptr GetSQLValueFunction(const string &column_name); LogicalType ResolveOperatorType(OperatorExpression &op, vector> &children); LogicalType ResolveCoalesceType(OperatorExpression &op, vector> &children); diff --git a/src/include/duckdb/planner/expression_binder/select_binder.hpp b/src/include/duckdb/planner/expression_binder/select_binder.hpp index 64e5914130ea..995ed02992cd 100644 --- a/src/include/duckdb/planner/expression_binder/select_binder.hpp +++ b/src/include/duckdb/planner/expression_binder/select_binder.hpp @@ -22,6 +22,7 @@ class SelectBinder : public BaseSelectBinder { BindResult BindColumnRef(unique_ptr &expr_ptr, idx_t depth, bool root_expression) override; bool QualifyColumnAlias(const ColumnRefExpression &colref) override; + unique_ptr GetSQLValueFunction(const string &column_name) override; protected: idx_t unnest_level = 0; diff --git a/src/planner/expression_binder/select_binder.cpp b/src/planner/expression_binder/select_binder.cpp index 69c1dcdf10c2..aab666a47d53 100644 --- a/src/planner/expression_binder/select_binder.cpp +++ b/src/planner/expression_binder/select_binder.cpp @@ -8,6 +8,15 @@ SelectBinder::SelectBinder(Binder &binder, ClientContext &context, BoundSelectNo : BaseSelectBinder(binder, context, node, info) { } +unique_ptr SelectBinder::GetSQLValueFunction(const string &column_name) { + auto alias_entry = node.bind_state.alias_map.find(column_name); + if (alias_entry != node.bind_state.alias_map.end()) { + // don't replace SQL value functions if they are in the alias map + return nullptr; + } + return ExpressionBinder::GetSQLValueFunction(column_name); +} + BindResult SelectBinder::BindColumnRef(unique_ptr &expr_ptr, idx_t depth, bool root_expression) { // first try to bind the column reference regularly auto result = BaseSelectBinder::BindColumnRef(expr_ptr, depth, root_expression); diff --git a/test/sql/parser/test_value_functions.test b/test/sql/parser/test_value_functions.test index b25be2238c16..0e508ee8b79c 100644 --- a/test/sql/parser/test_value_functions.test +++ b/test/sql/parser/test_value_functions.test @@ -56,3 +56,16 @@ group by one having max(cast('1000-05-01 00:00:00' as timestamp)) <= current_timestamp; ---- 1 1000-05-01 00:00:00 + +query II +select a as "b", "b" + 1 from (VALUES (84), (42)) t(a) ORDER BY ALL; +---- +42 43 +84 85 + +# value function conflict in ORDER BY +query I +select a as "CURRENT_TIMESTAMP" from (VALUES (84), (42)) t(a) order by "CURRENT_TIMESTAMP" + 1; +---- +42 +84 From c43151cf83711da46e86cd87600a3da7b463ef0e Mon Sep 17 00:00:00 2001 From: Carlo Piovesan Date: Fri, 13 Sep 2024 15:34:19 +0200 Subject: [PATCH 11/15] [CI] Invert operations on OSX.yml, deploying nightly artifacts also on failures --- .github/workflows/OSX.yml | 36 ++++++++++++++++++------------------ 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/.github/workflows/OSX.yml b/.github/workflows/OSX.yml index 99a34353fa5f..18fbbd7b5385 100644 --- a/.github/workflows/OSX.yml +++ b/.github/workflows/OSX.yml @@ -136,24 +136,6 @@ jobs: shell: bash run: ./build/release/duckdb -c "PRAGMA platform;" - - name: Unit Test - shell: bash - if: ${{ inputs.skip_tests != 'true' }} - run: make allunit - - - name: Tools Tests - shell: bash - if: ${{ inputs.skip_tests != 'true' }} - run: | - python -m pytest tools/shell/tests --shell-binary build/release/duckdb - - - name: Examples - shell: bash - if: ${{ inputs.skip_tests != 'true' }} - run: | - (cd examples/embedded-c; make) - (cd examples/embedded-c++; make) - # from https://docs.github.com/en/actions/deployment/deploying-xcode-applications/installing-an-apple-certificate-on-macos-runners-for-xcode-development - name: Sign Binaries shell: bash @@ -187,6 +169,24 @@ jobs: libduckdb-osx-universal.zip duckdb_cli-osx-universal.zip + - name: Unit Test + shell: bash + if: ${{ inputs.skip_tests != 'true' }} + run: make allunit + + - name: Tools Tests + shell: bash + if: ${{ inputs.skip_tests != 'true' }} + run: | + python -m pytest tools/shell/tests --shell-binary build/release/duckdb + + - name: Examples + shell: bash + if: ${{ inputs.skip_tests != 'true' }} + run: | + (cd examples/embedded-c; make) + (cd examples/embedded-c++; make) + xcode-extensions: # Builds extensions for osx_arm64 and osx_amd64 name: OSX Extensions Release From ab61d0b3131d9969f0ab72ccfb2e6c2fa557ede2 Mon Sep 17 00:00:00 2001 From: Tishj Date: Fri, 13 Sep 2024 15:39:49 +0200 Subject: [PATCH 12/15] don't use ExplainFormat::HTML for EXPLAIN_ANALYZE --- tools/pythonpkg/src/pyrelation.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tools/pythonpkg/src/pyrelation.cpp b/tools/pythonpkg/src/pyrelation.cpp index 906fa3cc1dc0..9775deb9c110 100644 --- a/tools/pythonpkg/src/pyrelation.cpp +++ b/tools/pythonpkg/src/pyrelation.cpp @@ -1481,8 +1481,8 @@ void DuckDBPyRelation::Print(const Optional &max_width, const Optional py::print(py::str(ToStringInternal(config, invalidate_cache))); } -static ExplainFormat GetExplainFormat() { - if (DuckDBPyConnection::IsJupyter()) { +static ExplainFormat GetExplainFormat(ExplainType type) { + if (DuckDBPyConnection::IsJupyter() && type != ExplainType::EXPLAIN_ANALYZE) { return ExplainFormat::HTML; } else { return ExplainFormat::DEFAULT; @@ -1502,7 +1502,7 @@ string DuckDBPyRelation::Explain(ExplainType type) { AssertRelation(); py::gil_scoped_release release; - auto explain_format = GetExplainFormat(); + auto explain_format = GetExplainFormat(type); auto res = rel->Explain(type, explain_format); D_ASSERT(res->type == duckdb::QueryResultType::MATERIALIZED_RESULT); auto &materialized = res->Cast(); From c4f36900d06ed0e4fe0b0d0c6ac45b9cd1ca1122 Mon Sep 17 00:00:00 2001 From: Carlo Piovesan Date: Fri, 13 Sep 2024 16:33:37 +0200 Subject: [PATCH 13/15] Improve logic for remote extension install on Windows Converting the path before checking for remote file will lead to strange results otherwise --- src/main/extension/extension_install.cpp | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/src/main/extension/extension_install.cpp b/src/main/extension/extension_install.cpp index 4c21da0433c7..b0ca9fb775de 100644 --- a/src/main/extension/extension_install.cpp +++ b/src/main/extension/extension_install.cpp @@ -268,15 +268,20 @@ static unique_ptr DirectInstallExtension(DatabaseInstance const string &local_extension_path, ExtensionInstallOptions &options, optional_ptr context) { - string file = fs.ConvertSeparators(path); - - // Try autoloading httpfs for loading extensions over https - if (context) { - auto &db = DatabaseInstance::GetDatabase(*context); - if (StringUtil::StartsWith(path, "https://") && !db.ExtensionIsLoaded("httpfs") && - db.config.options.autoload_known_extensions) { - ExtensionHelper::AutoLoadExtension(*context, "httpfs"); + string extension; + string file; + if (fs.IsRemoteFile(path, extension)) { + file = path; + // Try autoloading httpfs for loading extensions over https + if (context) { + auto &db = DatabaseInstance::GetDatabase(*context); + if (extension == "httpfs" && !db.ExtensionIsLoaded("httpfs") && + db.config.options.autoload_known_extensions) { + ExtensionHelper::AutoLoadExtension(*context, "httpfs"); + } } + } else { + file = fs.ConvertSeparators(path); } // Check if file exists From 3892258c9cc48caa65e1b53a891f87beee9e22be Mon Sep 17 00:00:00 2001 From: Gabor Szarnyas Date: Sat, 14 Sep 2024 09:11:47 +0200 Subject: [PATCH 14/15] CI: Trigger actions for labeled discussions --- .github/workflows/InternalIssuesCreateMirror.yml | 3 +++ .github/workflows/InternalIssuesUpdateMirror.yml | 3 +++ .github/workflows/NeedsDocumentation.yml | 3 +++ 3 files changed, 9 insertions(+) diff --git a/.github/workflows/InternalIssuesCreateMirror.yml b/.github/workflows/InternalIssuesCreateMirror.yml index b5b35fcbb694..b17cea98a735 100644 --- a/.github/workflows/InternalIssuesCreateMirror.yml +++ b/.github/workflows/InternalIssuesCreateMirror.yml @@ -1,5 +1,8 @@ name: Create or Label Mirror Issue on: + discussion: + types: + - labeled issues: types: - labeled diff --git a/.github/workflows/InternalIssuesUpdateMirror.yml b/.github/workflows/InternalIssuesUpdateMirror.yml index 800babc9f689..724450232fc4 100644 --- a/.github/workflows/InternalIssuesUpdateMirror.yml +++ b/.github/workflows/InternalIssuesUpdateMirror.yml @@ -1,5 +1,8 @@ name: Update Mirror Issue on: + discussion: + types: + - labeled issues: types: - closed diff --git a/.github/workflows/NeedsDocumentation.yml b/.github/workflows/NeedsDocumentation.yml index 08eac41f717f..d8bcc8d6f558 100644 --- a/.github/workflows/NeedsDocumentation.yml +++ b/.github/workflows/NeedsDocumentation.yml @@ -1,5 +1,8 @@ name: Create Documentation issue for the Needs Documentation label on: + discussion: + types: + - labeled issues: types: - labeled From eafa5b77ef33ecf5581d9a30be51c46234a8ac94 Mon Sep 17 00:00:00 2001 From: Tristan Celder Date: Mon, 16 Sep 2024 11:28:06 +0100 Subject: [PATCH 15/15] [Swift] Update README.md in Swift repo - bump DuckDB version in Swift.package example - remove file format stability notice --- tools/swift/duckdb-swift/README.md | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/tools/swift/duckdb-swift/README.md b/tools/swift/duckdb-swift/README.md index a190dfd1730c..65714bb3bab3 100644 --- a/tools/swift/duckdb-swift/README.md +++ b/tools/swift/duckdb-swift/README.md @@ -31,7 +31,7 @@ To use DuckDB in your Swift based project: 1. Add DuckDB to your `Swift.package` dependencies: ```swift - .package(url: "https://github.com/duckdb/duckdb-swift", .upToNextMinor(from: .init(0, x, 0))), + .package(url: "https://github.com/duckdb/duckdb-swift", .upToNextMajor(from: .init(1, 0, 0))), ``` 2. Add `DuckDB` as a dependency to your target: @@ -43,9 +43,6 @@ To use DuckDB in your Swift based project: ]), ``` -## Source and file format stability -DuckDB is in early release mode and API is subject to change between minor version updates. DuckDB's file format is also subject to change and – in the short-term – it is recommended to make use of [DuckDB's CSV import/export capabilities](https://duckdb.org/docs/csv_import.html) to ensure your data remains accessible between DuckDB version updates. - ## Documentation and Playgrounds The DuckDB Swift API is fully documented using DocC and the documentation can be generated from within Xcode via `Product > Build Documentation`.