Snap for 11698527 from 5ce594dcc1cf6aaddbf59a31aadda999fdf6a589 to mainline-appsearch-release Change-Id: I1b1b926e9eaf92bf95a4c8ce1bdce61305bf4718
diff --git a/icing/file/version-util.cc b/icing/file/version-util.cc index dd233e0..95af552 100644 --- a/icing/file/version-util.cc +++ b/icing/file/version-util.cc
@@ -135,6 +135,10 @@ // version 2 -> version 3 upgrade, no need to rebuild break; } + case 3: { + // version 3 -> version 4 upgrade, no need to rebuild + break; + } default: // This should not happen. Rebuild anyway if unsure. should_rebuild |= true;
diff --git a/icing/file/version-util.h b/icing/file/version-util.h index b2d51df..553042f 100644 --- a/icing/file/version-util.h +++ b/icing/file/version-util.h
@@ -32,13 +32,15 @@ // - Version 2: M-2023-09, M-2023-11, M-2024-01. Schema is compatible with v1. // (There were no M-2023-10, M-2023-12). // - Version 3: M-2024-02. Schema is compatible with v1 and v2. +// - Version 4: M-2024-06. Schema is compatible with v1, v2 and v3. // // LINT.IfChange(kVersion) -inline static constexpr int32_t kVersion = 3; +inline static constexpr int32_t kVersion = 4; // LINT.ThenChange(//depot/google3/icing/schema/schema-store.cc:min_overlay_version_compatibility) inline static constexpr int32_t kVersionOne = 1; inline static constexpr int32_t kVersionTwo = 2; inline static constexpr int32_t kVersionThree = 3; +inline static constexpr int32_t kVersionFour = 4; inline static constexpr int kVersionZeroFlashIndexMagic = 0x6dfba6ae;
diff --git a/icing/icing-search-engine.cc b/icing/icing-search-engine.cc index 72be4e9..88e2998 100644 --- a/icing/icing-search-engine.cc +++ b/icing/icing-search-engine.cc
@@ -231,8 +231,7 @@ } bool IsV2QualifiedIdJoinIndexEnabled(const IcingSearchEngineOptions& options) { - return options.use_new_qualified_id_join_index() && - options.document_store_namespace_id_fingerprint(); + return true; } libtextclassifier3::StatusOr<std::unique_ptr<QualifiedIdJoinIndex>> @@ -690,7 +689,7 @@ // Discard index directory and instantiate a new one. Index::Options index_options( index_dir, options_.index_merge_size(), - options_.lite_index_sort_at_indexing(), options_.lite_index_sort_size(), + /*lite_index_sort_at_indexing=*/true, options_.lite_index_sort_size(), options_.build_property_existence_metadata_hits()); if (!filesystem_->DeleteDirectoryRecursively(index_dir.c_str()) || !filesystem_->CreateDirectoryRecursively(index_dir.c_str())) { @@ -823,8 +822,8 @@ DocumentStore::Create( filesystem_.get(), document_dir, clock_.get(), schema_store_.get(), force_recovery_and_revalidate_documents, - options_.document_store_namespace_id_fingerprint(), - options_.pre_mapping_fbv(), options_.use_persistent_hash_map(), + /*document_store_namespace_id_fingerprint=*/true, + /*pre_mapping_fbv=*/false, /*use_persistent_hash_map=*/true, options_.compression_level(), initialize_stats)); document_store_ = std::move(create_result.document_store); @@ -844,7 +843,7 @@ } Index::Options index_options( index_dir, options_.index_merge_size(), - options_.lite_index_sort_at_indexing(), options_.lite_index_sort_size(), + /*lite_index_sort_at_indexing=*/true, options_.lite_index_sort_size(), options_.build_property_existence_metadata_hits()); // Term index @@ -2391,8 +2390,8 @@ auto create_result_or = DocumentStore::Create( filesystem_.get(), current_document_dir, clock_.get(), schema_store_.get(), /*force_recovery_and_revalidate_documents=*/false, - options_.document_store_namespace_id_fingerprint(), - options_.pre_mapping_fbv(), options_.use_persistent_hash_map(), + /*document_store_namespace_id_fingerprint=*/true, + /*pre_mapping_fbv=*/false, /*use_persistent_hash_map=*/true, options_.compression_level(), /*initialize_stats=*/nullptr); // TODO(b/144458732): Implement a more robust version of // TC_ASSIGN_OR_RETURN that can support error logging. @@ -2419,8 +2418,8 @@ auto create_result_or = DocumentStore::Create( filesystem_.get(), current_document_dir, clock_.get(), schema_store_.get(), /*force_recovery_and_revalidate_documents=*/false, - options_.document_store_namespace_id_fingerprint(), - options_.pre_mapping_fbv(), options_.use_persistent_hash_map(), + /*document_store_namespace_id_fingerprint=*/true, + /*pre_mapping_fbv=*/false, /*use_persistent_hash_map=*/true, options_.compression_level(), /*initialize_stats=*/nullptr); if (!create_result_or.ok()) { // Unable to create DocumentStore from the new file. Mark as uninitialized
diff --git a/icing/icing-search-engine_initialization_test.cc b/icing/icing-search-engine_initialization_test.cc index 122e4af..00b7e5b 100644 --- a/icing/icing-search-engine_initialization_test.cc +++ b/icing/icing-search-engine_initialization_test.cc
@@ -1084,10 +1084,9 @@ DocumentStore::Create( filesystem(), GetDocumentDir(), &fake_clock, schema_store.get(), /*force_recovery_and_revalidate_documents=*/false, - /*namespace_id_fingerprint=*/ - icing_options.document_store_namespace_id_fingerprint(), + /*namespace_id_fingerprint=*/true, /*pre_mapping_fbv=*/false, - /*use_persistent_hash_map=*/false, + /*use_persistent_hash_map=*/true, PortableFileBackedProtoLog< DocumentWrapper>::kDeflateCompressionLevel, /*initialize_stats=*/nullptr)); @@ -5403,169 +5402,6 @@ } } -// TODO(b/275121148): deprecate this test after rollout join index v2. -class IcingSearchEngineInitializationSwitchJoinIndexTest - : public IcingSearchEngineInitializationTest, - public ::testing::WithParamInterface<bool> {}; -TEST_P(IcingSearchEngineInitializationSwitchJoinIndexTest, SwitchJoinIndex) { - bool use_join_index_v2 = GetParam(); - - SchemaProto schema = - SchemaBuilder() - .AddType(SchemaTypeConfigBuilder().SetType("Person").AddProperty( - PropertyConfigBuilder() - .SetName("name") - .SetDataTypeString(TERM_MATCH_PREFIX, TOKENIZER_PLAIN) - .SetCardinality(CARDINALITY_REQUIRED))) - .AddType(SchemaTypeConfigBuilder() - .SetType("Message") - .AddProperty(PropertyConfigBuilder() - .SetName("body") - .SetDataTypeString(TERM_MATCH_PREFIX, - TOKENIZER_PLAIN) - .SetCardinality(CARDINALITY_REQUIRED)) - .AddProperty(PropertyConfigBuilder() - .SetName("indexableInteger") - .SetDataTypeInt64(NUMERIC_MATCH_RANGE) - .SetCardinality(CARDINALITY_REQUIRED)) - .AddProperty(PropertyConfigBuilder() - .SetName("senderQualifiedId") - .SetDataTypeJoinableString( - JOINABLE_VALUE_TYPE_QUALIFIED_ID) - .SetCardinality(CARDINALITY_OPTIONAL))) - .Build(); - - DocumentProto person = - DocumentBuilder() - .SetKey("namespace", "person") - .SetSchema("Person") - .AddStringProperty("name", "person") - .SetCreationTimestampMs(kDefaultCreationTimestampMs) - .Build(); - DocumentProto message = - DocumentBuilder() - .SetKey("namespace", "message/1") - .SetSchema("Message") - .AddStringProperty("body", kIpsumText) - .AddInt64Property("indexableInteger", 123) - .AddStringProperty("senderQualifiedId", "namespace#person") - .SetCreationTimestampMs(kDefaultCreationTimestampMs) - .Build(); - - // 1. Create an index with message 3 documents. - { - IcingSearchEngineOptions options = GetDefaultIcingOptions(); - options.set_document_store_namespace_id_fingerprint(true); - options.set_use_new_qualified_id_join_index(use_join_index_v2); - - TestIcingSearchEngine icing(options, std::make_unique<Filesystem>(), - std::make_unique<IcingFilesystem>(), - std::make_unique<FakeClock>(), - GetTestJniCache()); - - ASSERT_THAT(icing.Initialize().status(), ProtoIsOk()); - ASSERT_THAT(icing.SetSchema(schema).status(), ProtoIsOk()); - - EXPECT_THAT(icing.Put(person).status(), ProtoIsOk()); - EXPECT_THAT(icing.Put(message).status(), ProtoIsOk()); - message = DocumentBuilder(message).SetUri("message/2").Build(); - EXPECT_THAT(icing.Put(message).status(), ProtoIsOk()); - message = DocumentBuilder(message).SetUri("message/3").Build(); - EXPECT_THAT(icing.Put(message).status(), ProtoIsOk()); - } - - // 2. Create the index again changing join index version. This should trigger - // join index restoration. - { - // Mock filesystem to observe and check the behavior of all indices. - auto mock_filesystem = std::make_unique<MockFilesystem>(); - EXPECT_CALL(*mock_filesystem, DeleteDirectoryRecursively(_)) - .WillRepeatedly(DoDefault()); - // Ensure term index directory should never be discarded. - EXPECT_CALL(*mock_filesystem, - DeleteDirectoryRecursively(EndsWith("/index_dir"))) - .Times(0); - // Ensure integer index directory should never be discarded, and Clear() - // should never be called (i.e. storage sub directory - // "*/integer_index_dir/*" should never be discarded). - EXPECT_CALL(*mock_filesystem, - DeleteDirectoryRecursively(EndsWith("/integer_index_dir"))) - .Times(0); - EXPECT_CALL(*mock_filesystem, - DeleteDirectoryRecursively(HasSubstr("/integer_index_dir/"))) - .Times(0); - // Ensure qualified id join index directory should be discarded once, and - // Clear() should never be called (i.e. storage sub directory - // "*/qualified_id_join_index_dir/*" should never be discarded). - EXPECT_CALL(*mock_filesystem, DeleteDirectoryRecursively( - EndsWith("/qualified_id_join_index_dir"))) - .Times(1); - EXPECT_CALL( - *mock_filesystem, - DeleteDirectoryRecursively(HasSubstr("/qualified_id_join_index_dir/"))) - .Times(0); - - IcingSearchEngineOptions options = GetDefaultIcingOptions(); - options.set_document_store_namespace_id_fingerprint(true); - options.set_use_new_qualified_id_join_index(!use_join_index_v2); - - TestIcingSearchEngine icing(options, std::move(mock_filesystem), - std::make_unique<IcingFilesystem>(), - std::make_unique<FakeClock>(), - GetTestJniCache()); - InitializeResultProto initialize_result = icing.Initialize(); - ASSERT_THAT(initialize_result.status(), ProtoIsOk()); - EXPECT_THAT(initialize_result.initialize_stats().index_restoration_cause(), - Eq(InitializeStatsProto::NONE)); - EXPECT_THAT( - initialize_result.initialize_stats().integer_index_restoration_cause(), - Eq(InitializeStatsProto::NONE)); - EXPECT_THAT(initialize_result.initialize_stats() - .qualified_id_join_index_restoration_cause(), - Eq(InitializeStatsProto::INCONSISTENT_WITH_GROUND_TRUTH)); - - // Verify qualified id join index works normally: join a query for - // `name:person` with a child query for `body:consectetur` based on the - // child's `senderQualifiedId` field. - SearchSpecProto search_spec; - search_spec.set_term_match_type(TermMatchType::EXACT_ONLY); - search_spec.set_query("name:person"); - JoinSpecProto* join_spec = search_spec.mutable_join_spec(); - join_spec->set_parent_property_expression( - std::string(JoinProcessor::kQualifiedIdExpr)); - join_spec->set_child_property_expression("senderQualifiedId"); - join_spec->set_aggregation_scoring_strategy( - JoinSpecProto::AggregationScoringStrategy::COUNT); - JoinSpecProto::NestedSpecProto* nested_spec = - join_spec->mutable_nested_spec(); - SearchSpecProto* nested_search_spec = nested_spec->mutable_search_spec(); - nested_search_spec->set_term_match_type(TermMatchType::EXACT_ONLY); - nested_search_spec->set_query("body:consectetur"); - *nested_spec->mutable_scoring_spec() = GetDefaultScoringSpec(); - *nested_spec->mutable_result_spec() = ResultSpecProto::default_instance(); - - ResultSpecProto result_spec = ResultSpecProto::default_instance(); - result_spec.set_max_joined_children_per_parent_to_return( - std::numeric_limits<int32_t>::max()); - - SearchResultProto results = icing.Search( - search_spec, ScoringSpecProto::default_instance(), result_spec); - ASSERT_THAT(results.results(), SizeIs(1)); - EXPECT_THAT(results.results(0).document().uri(), Eq("person")); - EXPECT_THAT(results.results(0).joined_results(), SizeIs(3)); - EXPECT_THAT(results.results(0).joined_results(0).document().uri(), - Eq("message/3")); - EXPECT_THAT(results.results(0).joined_results(1).document().uri(), - Eq("message/2")); - EXPECT_THAT(results.results(0).joined_results(2).document().uri(), - Eq("message/1")); - } -} - -INSTANTIATE_TEST_SUITE_P(IcingSearchEngineInitializationSwitchJoinIndexTest, - IcingSearchEngineInitializationSwitchJoinIndexTest, - testing::Values(true, false)); - class IcingSearchEngineInitializationVersionChangeTest : public IcingSearchEngineInitializationTest, public ::testing::WithParamInterface<version_util::VersionInfo> {}; @@ -5659,10 +5495,9 @@ DocumentStore::Create( filesystem(), GetDocumentDir(), &fake_clock, schema_store.get(), /*force_recovery_and_revalidate_documents=*/false, - /*namespace_id_fingerprint=*/ - icing_options.document_store_namespace_id_fingerprint(), + /*namespace_id_fingerprint=*/true, /*pre_mapping_fbv=*/false, - /*use_persistent_hash_map=*/false, + /*use_persistent_hash_map=*/true, PortableFileBackedProtoLog< DocumentWrapper>::kDeflateCompressionLevel, /*initialize_stats=*/nullptr));
diff --git a/icing/schema/schema-store.cc b/icing/schema/schema-store.cc index e17e388..6830787 100644 --- a/icing/schema/schema-store.cc +++ b/icing/schema/schema-store.cc
@@ -486,7 +486,7 @@ ICING_RETURN_IF_ERROR(schema_file_->Write(std::move(base_schema_ptr))); // LINT.IfChange(min_overlay_version_compatibility) - // Although the current version is 3, the schema is compatible with + // Although the current version is 4, the schema is compatible with // version 1, so min_overlay_version_compatibility should be 1. int32_t min_overlay_version_compatibility = version_util::kVersionOne; // LINT.ThenChange(//depot/google3/icing/file/version-util.h:kVersion)