diff --git a/src/Interpreters/InterpreterCreateQuery.cpp b/src/Interpreters/InterpreterCreateQuery.cpp index 9a31d343cd5..66936dc25d7 100644 --- a/src/Interpreters/InterpreterCreateQuery.cpp +++ b/src/Interpreters/InterpreterCreateQuery.cpp @@ -756,7 +756,7 @@ InterpreterCreateQuery::TableProperties InterpreterCreateQuery::getTableProperti throw Exception(ErrorCodes::SUPPORT_IS_DISABLED, "Experimental full-text index feature is not enabled (the setting 'allow_experimental_full_text_index')"); /// ---- /// Temporary check during a transition period. Please remove at the end of 2024. - if (index_desc.type == INVERTED_INDEX_NAME && settings.allow_experimental_inverted_index) /// The funny condition is not a mistake, see 02346_fulltext_index_old_name.sql + if (index_desc.type == INVERTED_INDEX_NAME && !settings.allow_experimental_inverted_index) throw Exception(ErrorCodes::ILLEGAL_INDEX, "Please use index type 'full_text' instead of 'inverted'"); /// ---- if (index_desc.type == "annoy" && !settings.allow_experimental_annoy_index) diff --git a/src/Storages/MergeTree/MergeTreeIndexFullText.cpp b/src/Storages/MergeTree/MergeTreeIndexFullText.cpp index af9ee710f88..451971cff98 100644 --- a/src/Storages/MergeTree/MergeTreeIndexFullText.cpp +++ b/src/Storages/MergeTree/MergeTreeIndexFullText.cpp @@ -742,6 +742,7 @@ bool MergeTreeConditionFullText::tryPrepareSetGinFilter( MergeTreeIndexGranulePtr MergeTreeIndexFullText::createIndexGranule() const { + /// ------ /// Index type 'inverted' was renamed to 'full_text' in May 2024. /// Tables with old indexes can be loaded during a transition period. We still want let users know that they should drop existing /// indexes and re-create them. Function `createIndexGranule` is called whenever the index is used by queries. Reject the query if we @@ -749,6 +750,7 @@ MergeTreeIndexGranulePtr MergeTreeIndexFullText::createIndexGranule() const /// TODO: remove this at the end of 2024. if (index.type == INVERTED_INDEX_NAME) throw Exception(ErrorCodes::ILLEGAL_INDEX, "Indexes of type 'inverted' are no longer supported. Please drop and recreate the index as type 'full-text'"); + /// ------ return std::make_shared(index.name, index.column_names.size(), params); } diff --git a/tests/queries/0_stateless/02346_fulltext_index_old_name.sql b/tests/queries/0_stateless/02346_fulltext_index_old_name.sql index bc641caf237..4e52e689211 100644 --- a/tests/queries/0_stateless/02346_fulltext_index_old_name.sql +++ b/tests/queries/0_stateless/02346_fulltext_index_old_name.sql @@ -1,22 +1,16 @@ +-- Index type 'inverted' was renamed to 'full_text' in April 2024. +-- Such indexes are experimental. Test what happens when ClickHouse encounters tables with the old index type. + DROP TABLE IF EXISTS tab; --- Index type 'inverted' was renamed to 'full_text' in April 2024. --- Such indexes are experimental. Nevertheless test what happens when ClickHouse encounters tables with the old index type. +-- It must be possible to load old tables with 'inverted'-type indexes +-- In stateless tests, we cannot use old persistences. Emulate "loading an old index" by creating it (internally, similar code executes). --- Create a full text index with the old type --- This was how it was done in the old days. These days this throws an exception. -SET allow_experimental_inverted_index = 1; -CREATE TABLE tab(k UInt64, s String, INDEX idx(s) TYPE inverted(2)) ENGINE = MergeTree() ORDER BY k; -- { serverError ILLEGAL_INDEX }; - --- There are unfortunately side effects of this behavior. In particular, if ClickHouse's automatic table load during --- startup finds a table with 'inverted'-type indexes created by an older version, it immediately halts as it thinks --- the persistence is corrupt. Similarly (but less severely), tables with 'inverted' index cannot be attached. --- A backdoor avoids this. Just set allow_experimental_inverted_index = 0 (which is the default). --- --- Note that the backdoor will exist only temporarily during a transition period. It will be removed in future. Its only purpose is --- to simplify the migrationn of experimental inverted indexes to experimental full-text indexes instead of simply breaking existing --- tables. +-- Creation only works with the (old) setting enabled. SET allow_experimental_inverted_index = 0; +CREATE TABLE tab(k UInt64, s String, INDEX idx(s) TYPE inverted(2)) ENGINE = MergeTree() ORDER BY k; -- { serverError ILLEGAL_INDEX } + +SET allow_experimental_inverted_index = 1; CREATE TABLE tab(k UInt64, s String, INDEX idx(s) TYPE inverted(2)) ENGINE = MergeTree() ORDER BY k; INSERT INTO tab VALUES (1, 'ab') (2, 'bc'); @@ -24,14 +18,12 @@ INSERT INTO tab VALUES (1, 'ab') (2, 'bc'); DETACH TABLE tab; ATTACH TABLE tab; --- No, the backdoor does not make 'inverted' indexes non-experimental. --- On the one hand, the backdoor is undocumented, on the other hand, SELECTs that use such indexes now throw an exception, --- making 'inverted' indexes useless. +-- To encourage users to migrate to the new index type, we now throw an exception when the index is used by queries. SELECT * from tab WHERE s = 'bc'; -- { serverError ILLEGAL_INDEX } -- The exception recommends to drop the index and create a 'full_text' index instead. Let's try. ALTER TABLE tab DROP INDEX idx; -SET allow_experimental_full_text_index = 1; -- note that this is a different setting +SET allow_experimental_full_text_index = 1; -- the new setting ALTER TABLE tab ADD INDEX idx(s) TYPE full_text(2); SELECT * from tab WHERE s = 'bc';