From f686803f0acc6a9898c5377d7a2de728c9b4f6fb Mon Sep 17 00:00:00 2001 From: Max Gabrielsson Date: Tue, 16 Apr 2024 18:22:12 +0200 Subject: [PATCH 1/4] bump vss --- .github/config/extensions.csv | 2 +- .github/config/out_of_tree_extensions.cmake | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/config/extensions.csv b/.github/config/extensions.csv index 50efd8809ff4..453ee59d3a80 100644 --- a/.github/config/extensions.csv +++ b/.github/config/extensions.csv @@ -15,4 +15,4 @@ aws,https://github.com/duckdb/duckdb_aws,f7b8729f1cce5ada5d4add70e1486de50763fb9 azure,https://github.com/duckdb/duckdb_azure,09623777a366572bfb8fa53e47acdf72133a360e, spatial,https://github.com/duckdb/duckdb_spatial,8ac803e986ccda34f32dee82a7faae95b72b3492, iceberg,https://github.com/duckdb/duckdb_iceberg,d89423c2ff90a0b98a093a133c8dfe2a55b9e092, -vss,https://github.com/duckdb/duckdb_vss,9038b50cefb8bfd6b8ab8a254d3c728f3f172d15, +vss,https://github.com/duckdb/duckdb_vss,8145f41d97178e82bed3376215eb8d02bcf1eec5, diff --git a/.github/config/out_of_tree_extensions.cmake b/.github/config/out_of_tree_extensions.cmake index a95bd411a18c..a1e7d240cac1 100644 --- a/.github/config/out_of_tree_extensions.cmake +++ b/.github/config/out_of_tree_extensions.cmake @@ -106,6 +106,6 @@ endif() duckdb_extension_load(vss LOAD_TESTS GIT_URL https://github.com/duckdb/duckdb_vss - GIT_TAG 9038b50cefb8bfd6b8ab8a254d3c728f3f172d15 + GIT_TAG 8145f41d97178e82bed3376215eb8d02bcf1eec5 TEST_DIR test/sql ) From 0528a0f4e3e922d39bc6af599002fc57ed9a0dc4 Mon Sep 17 00:00:00 2001 From: Max Gabrielsson Date: Tue, 16 Apr 2024 22:48:00 +0200 Subject: [PATCH 2/4] catch errors when vacuuming index, dont try to rever unknown indexes --- src/storage/data_table.cpp | 8 ++++++-- src/storage/local_storage.cpp | 6 +++++- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/src/storage/data_table.cpp b/src/storage/data_table.cpp index 60f16502b0bc..aeadf22cc77f 100644 --- a/src/storage/data_table.cpp +++ b/src/storage/data_table.cpp @@ -860,7 +860,9 @@ void DataTable::RevertAppend(idx_t start_row, idx_t count) { row_data[i] = current_row_base + i; } info->indexes.Scan([&](Index &index) { - index.Delete(chunk, row_identifiers); + if (!index.IsUnknown()) { + index.Delete(chunk, row_identifiers); + } return false; }); current_row_base += chunk.size(); @@ -870,7 +872,9 @@ void DataTable::RevertAppend(idx_t start_row, idx_t count) { // we need to vacuum the indexes to remove any buffers that are now empty // due to reverting the appends info->indexes.Scan([&](Index &index) { - index.Vacuum(); + if (!index.IsUnknown()) { + index.Vacuum(); + } return false; }); diff --git a/src/storage/local_storage.cpp b/src/storage/local_storage.cpp index ee249ce2d29f..b8482e2de327 100644 --- a/src/storage/local_storage.cpp +++ b/src/storage/local_storage.cpp @@ -197,7 +197,11 @@ void LocalTableStorage::AppendToIndexes(DuckTransaction &transaction, TableAppen // we need to vacuum the indexes to remove any buffers that are now empty // due to reverting the appends table.info->indexes.Scan([&](Index &index) { - index.Vacuum(); + try { + index.Vacuum(); + } catch (std::exception &ex) { // LCOV_EXCL_START + error = ErrorData(ex); + } // LCOV_EXCL_STOP return false; }); error.Throw(); From 49099e877a4196fa2d45cb56079eda9232a055c1 Mon Sep 17 00:00:00 2001 From: Max Gabrielsson Date: Wed, 17 Apr 2024 00:36:53 +0200 Subject: [PATCH 3/4] handle updates, deletes, more index initialization --- src/execution/index/unknown_index.cpp | 26 +++++++++---------- .../duckdb/storage/table/data_table_info.hpp | 5 ++-- .../duckdb/storage/table/table_index_list.hpp | 5 ++-- src/storage/data_table.cpp | 9 +++++-- src/storage/table_index_list.cpp | 5 +++- src/transaction/undo_buffer.cpp | 4 ++- 6 files changed, 33 insertions(+), 21 deletions(-) diff --git a/src/execution/index/unknown_index.cpp b/src/execution/index/unknown_index.cpp index 09b14581abd3..e2b6a0cbd3c3 100644 --- a/src/execution/index/unknown_index.cpp +++ b/src/execution/index/unknown_index.cpp @@ -22,44 +22,44 @@ string UnknownIndex::GenerateErrorMessage() const { } ErrorData UnknownIndex::Append(IndexLock &, DataChunk &, Vector &) { - throw NotImplementedException(GenerateErrorMessage()); + throw MissingExtensionException(GenerateErrorMessage()); } void UnknownIndex::VerifyAppend(DataChunk &) { - throw NotImplementedException(GenerateErrorMessage()); + throw MissingExtensionException(GenerateErrorMessage()); } void UnknownIndex::VerifyAppend(DataChunk &, ConflictManager &) { - throw NotImplementedException(GenerateErrorMessage()); + throw MissingExtensionException(GenerateErrorMessage()); } void UnknownIndex::CommitDrop(IndexLock &) { - throw NotImplementedException(GenerateErrorMessage()); + throw MissingExtensionException(GenerateErrorMessage()); } void UnknownIndex::Delete(IndexLock &, DataChunk &, Vector &) { - throw NotImplementedException(GenerateErrorMessage()); + throw MissingExtensionException(GenerateErrorMessage()); } ErrorData UnknownIndex::Insert(IndexLock &, DataChunk &, Vector &) { - throw NotImplementedException(GenerateErrorMessage()); + throw MissingExtensionException(GenerateErrorMessage()); } IndexStorageInfo UnknownIndex::GetStorageInfo(bool) { - throw NotImplementedException(GenerateErrorMessage()); + throw MissingExtensionException(GenerateErrorMessage()); } bool UnknownIndex::MergeIndexes(IndexLock &, Index &) { - throw NotImplementedException(GenerateErrorMessage()); + throw MissingExtensionException(GenerateErrorMessage()); } void UnknownIndex::Vacuum(IndexLock &) { - throw NotImplementedException(GenerateErrorMessage()); + throw MissingExtensionException(GenerateErrorMessage()); } idx_t UnknownIndex::GetInMemorySize(IndexLock &) { - throw NotImplementedException(GenerateErrorMessage()); + throw MissingExtensionException(GenerateErrorMessage()); } void UnknownIndex::CheckConstraintsForChunk(DataChunk &, ConflictManager &) { - throw NotImplementedException(GenerateErrorMessage()); + throw MissingExtensionException(GenerateErrorMessage()); } string UnknownIndex::VerifyAndToString(IndexLock &, bool) { - throw NotImplementedException(GenerateErrorMessage()); + throw MissingExtensionException(GenerateErrorMessage()); } string UnknownIndex::GetConstraintViolationMessage(VerifyExistenceType, idx_t, DataChunk &) { - throw NotImplementedException(GenerateErrorMessage()); + throw MissingExtensionException(GenerateErrorMessage()); } } // namespace duckdb diff --git a/src/include/duckdb/storage/table/data_table_info.hpp b/src/include/duckdb/storage/table/data_table_info.hpp index 2785486b5da3..598d80e222b4 100644 --- a/src/include/duckdb/storage/table/data_table_info.hpp +++ b/src/include/duckdb/storage/table/data_table_info.hpp @@ -19,8 +19,9 @@ class TableIOManager; struct DataTableInfo { DataTableInfo(AttachedDatabase &db, shared_ptr table_io_manager_p, string schema, string table); - //! Initialize any unknown indexes whose types might now be present after an extension load - void InitializeIndexes(ClientContext &context); + //! Initialize any unknown indexes whose types might now be present after an extension load, optionally throwing an + //! exception if an index can't be initialized + void InitializeIndexes(ClientContext &context, bool throw_on_failure = false); //! The database instance of the table AttachedDatabase &db; diff --git a/src/include/duckdb/storage/table/table_index_list.hpp b/src/include/duckdb/storage/table/table_index_list.hpp index be15497ad9fe..397bf28243cd 100644 --- a/src/include/duckdb/storage/table/table_index_list.hpp +++ b/src/include/duckdb/storage/table/table_index_list.hpp @@ -41,8 +41,9 @@ class TableIndexList { void CommitDrop(const string &name); //! Returns true, if the index name does not exist bool NameIsUnique(const string &name); - //! Initializes unknown indexes that might now be present after an extension load - void InitializeIndexes(ClientContext &context, DataTableInfo &table_info); + //! Initializes unknown indexes that might now be present after an extension load, optionally throwing an exception + //! if a index cant be initialized + void InitializeIndexes(ClientContext &context, DataTableInfo &table_info, bool throw_on_failure = false); bool Empty(); idx_t Count(); void Move(TableIndexList &other); diff --git a/src/storage/data_table.cpp b/src/storage/data_table.cpp index aeadf22cc77f..2b09cfa60b9d 100644 --- a/src/storage/data_table.cpp +++ b/src/storage/data_table.cpp @@ -34,8 +34,8 @@ DataTableInfo::DataTableInfo(AttachedDatabase &db, shared_ptr ta table(std::move(table)) { } -void DataTableInfo::InitializeIndexes(ClientContext &context) { - indexes.InitializeIndexes(context, *this); +void DataTableInfo::InitializeIndexes(ClientContext &context, bool throw_on_failure) { + indexes.InitializeIndexes(context, *this, throw_on_failure); } bool DataTableInfo::IsTemporary() const { @@ -1006,6 +1006,8 @@ idx_t DataTable::Delete(TableCatalogEntry &table, ClientContext &context, Vector return 0; } + info->InitializeIndexes(context, true); + auto &transaction = DuckTransaction::Get(context, db); auto &local_storage = LocalStorage::Get(transaction); bool has_delete_constraints = TableHasDeleteConstraints(table); @@ -1163,6 +1165,9 @@ void DataTable::Update(TableCatalogEntry &table, ClientContext &context, Vector throw TransactionException("Transaction conflict: cannot update a table that has been altered!"); } + // check that there are no unknown indexes + info->InitializeIndexes(context, true); + // first verify that no constraints are violated VerifyUpdateConstraints(context, table, updates, column_ids); diff --git a/src/storage/table_index_list.cpp b/src/storage/table_index_list.cpp index ef14073f302b..b15a7aab10e3 100644 --- a/src/storage/table_index_list.cpp +++ b/src/storage/table_index_list.cpp @@ -55,7 +55,7 @@ bool TableIndexList::NameIsUnique(const string &name) { return true; } -void TableIndexList::InitializeIndexes(ClientContext &context, DataTableInfo &table_info) { +void TableIndexList::InitializeIndexes(ClientContext &context, DataTableInfo &table_info, bool throw_on_failure) { lock_guard lock(indexes_lock); for (auto &index : indexes) { if (!index->IsUnknown()) { @@ -68,6 +68,9 @@ void TableIndexList::InitializeIndexes(ClientContext &context, DataTableInfo &ta // Do we know the type of this index now? auto index_type = context.db->config.GetIndexTypes().FindByName(index_type_name); if (!index_type) { + if (throw_on_failure) { + throw MissingExtensionException("Cannot initialize index '%s', unknown index type '%s'. You probably need to load an extension.", unknown_index.name, index_type_name); + } continue; } diff --git a/src/transaction/undo_buffer.cpp b/src/transaction/undo_buffer.cpp index 148bd3d04913..02f044cc4af2 100644 --- a/src/transaction/undo_buffer.cpp +++ b/src/transaction/undo_buffer.cpp @@ -142,7 +142,9 @@ void UndoBuffer::Cleanup() { // possibly vacuum indexes for (const auto &table : state.indexed_tables) { table.second->info->indexes.Scan([&](Index &index) { - index.Vacuum(); + if(!index.IsUnknown()) { + index.Vacuum(); + } return false; }); } From b33810e72ba07ac0d9b175ec248dc76f470747e6 Mon Sep 17 00:00:00 2001 From: Max Gabrielsson Date: Wed, 17 Apr 2024 00:47:51 +0200 Subject: [PATCH 4/4] format --- src/storage/table_index_list.cpp | 4 +++- src/transaction/undo_buffer.cpp | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/storage/table_index_list.cpp b/src/storage/table_index_list.cpp index b15a7aab10e3..1e07ce6fff6f 100644 --- a/src/storage/table_index_list.cpp +++ b/src/storage/table_index_list.cpp @@ -69,7 +69,9 @@ void TableIndexList::InitializeIndexes(ClientContext &context, DataTableInfo &ta auto index_type = context.db->config.GetIndexTypes().FindByName(index_type_name); if (!index_type) { if (throw_on_failure) { - throw MissingExtensionException("Cannot initialize index '%s', unknown index type '%s'. You probably need to load an extension.", unknown_index.name, index_type_name); + throw MissingExtensionException( + "Cannot initialize index '%s', unknown index type '%s'. You probably need to load an extension.", + unknown_index.name, index_type_name); } continue; } diff --git a/src/transaction/undo_buffer.cpp b/src/transaction/undo_buffer.cpp index 02f044cc4af2..2e94aa7fef66 100644 --- a/src/transaction/undo_buffer.cpp +++ b/src/transaction/undo_buffer.cpp @@ -142,7 +142,7 @@ void UndoBuffer::Cleanup() { // possibly vacuum indexes for (const auto &table : state.indexed_tables) { table.second->info->indexes.Scan([&](Index &index) { - if(!index.IsUnknown()) { + if (!index.IsUnknown()) { index.Vacuum(); } return false;