aboutsummaryrefslogtreecommitdiff
path: root/icing/schema
diff options
context:
space:
mode:
Diffstat (limited to 'icing/schema')
-rw-r--r--icing/schema/schema-store.cc2
-rw-r--r--icing/schema/schema-util.cc58
-rw-r--r--icing/schema/schema-util.h16
-rw-r--r--icing/schema/schema-util_test.cc111
4 files changed, 154 insertions, 33 deletions
diff --git a/icing/schema/schema-store.cc b/icing/schema/schema-store.cc
index 85ee6b6..e17e388 100644
--- a/icing/schema/schema-store.cc
+++ b/icing/schema/schema-store.cc
@@ -486,7 +486,7 @@ libtextclassifier3::Status SchemaStore::RegenerateDerivedFiles(
ICING_RETURN_IF_ERROR(schema_file_->Write(std::move(base_schema_ptr)));
// LINT.IfChange(min_overlay_version_compatibility)
- // Although the current version is 2, the schema is compatible with
+ // Although the current version is 3, 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)
diff --git a/icing/schema/schema-util.cc b/icing/schema/schema-util.cc
index af6feda..72287a8 100644
--- a/icing/schema/schema-util.cc
+++ b/icing/schema/schema-util.cc
@@ -189,6 +189,18 @@ bool CardinalityLessThanEq(PropertyConfigProto::Cardinality::Code C1,
return false;
}
+// Check if set1 is a subset of set2.
+template <typename T>
+bool IsSubset(const std::unordered_set<T>& set1,
+ const std::unordered_set<T>& set2) {
+ for (const auto& item : set1) {
+ if (set2.find(item) == set2.end()) {
+ return false;
+ }
+ }
+ return true;
+}
+
} // namespace
libtextclassifier3::Status CalculateTransitiveNestedTypeRelations(
@@ -929,31 +941,32 @@ SchemaUtil::ParsedPropertyConfigs SchemaUtil::ParsePropertyConfigs(
// TODO(cassiewang): consider caching property_config_map for some properties,
// e.g. using LRU cache. Or changing schema.proto to use go/protomap.
for (const PropertyConfigProto& property_config : type_config.properties()) {
- parsed_property_configs.property_config_map.emplace(
- property_config.property_name(), &property_config);
+ std::string_view property_name = property_config.property_name();
+ parsed_property_configs.property_config_map.emplace(property_name,
+ &property_config);
if (property_config.cardinality() ==
PropertyConfigProto::Cardinality::REQUIRED) {
- ++parsed_property_configs.num_required_properties;
+ parsed_property_configs.required_properties.insert(property_name);
}
// A non-default term_match_type indicates that this property is meant to be
// indexed.
if (IsIndexedProperty(property_config)) {
- ++parsed_property_configs.num_indexed_properties;
+ parsed_property_configs.indexed_properties.insert(property_name);
}
// A non-default value_type indicates that this property is meant to be
// joinable.
if (property_config.joinable_config().value_type() !=
JoinableConfig::ValueType::NONE) {
- ++parsed_property_configs.num_joinable_properties;
+ parsed_property_configs.joinable_properties.insert(property_name);
}
// Also keep track of how many nested document properties there are. Adding
// new nested document properties will result in join-index rebuild.
if (property_config.data_type() ==
PropertyConfigProto::DataType::DOCUMENT) {
- ++parsed_property_configs.num_nested_document_properties;
+ parsed_property_configs.nested_document_properties.insert(property_name);
}
}
@@ -990,10 +1003,10 @@ const SchemaUtil::SchemaDelta SchemaUtil::ComputeCompatibilityDelta(
// We only need to check the old, existing properties to see if they're
// compatible since we'll have old data that may be invalidated or need to
// be reindexed.
- int32_t old_required_properties = 0;
- int32_t old_indexed_properties = 0;
- int32_t old_joinable_properties = 0;
- int32_t old_nested_document_properties = 0;
+ std::unordered_set<std::string_view> old_required_properties;
+ std::unordered_set<std::string_view> old_indexed_properties;
+ std::unordered_set<std::string_view> old_joinable_properties;
+ std::unordered_set<std::string_view> old_nested_document_properties;
// If there is a different number of properties, then there must have been a
// change.
@@ -1004,23 +1017,24 @@ const SchemaUtil::SchemaDelta SchemaUtil::ComputeCompatibilityDelta(
bool is_index_incompatible = false;
bool is_join_incompatible = false;
for (const auto& old_property_config : old_type_config.properties()) {
+ std::string_view property_name = old_property_config.property_name();
if (old_property_config.cardinality() ==
PropertyConfigProto::Cardinality::REQUIRED) {
- ++old_required_properties;
+ old_required_properties.insert(property_name);
}
// A non-default term_match_type indicates that this property is meant to
// be indexed.
bool is_indexed_property = IsIndexedProperty(old_property_config);
if (is_indexed_property) {
- ++old_indexed_properties;
+ old_indexed_properties.insert(property_name);
}
bool is_joinable_property =
old_property_config.joinable_config().value_type() !=
JoinableConfig::ValueType::NONE;
if (is_joinable_property) {
- ++old_joinable_properties;
+ old_joinable_properties.insert(property_name);
}
// A nested-document property is a property of DataType::DOCUMENT.
@@ -1028,7 +1042,7 @@ const SchemaUtil::SchemaDelta SchemaUtil::ComputeCompatibilityDelta(
old_property_config.data_type() ==
PropertyConfigProto::DataType::DOCUMENT;
if (is_nested_document_property) {
- ++old_nested_document_properties;
+ old_nested_document_properties.insert(property_name);
}
auto new_property_name_and_config =
@@ -1088,8 +1102,8 @@ const SchemaUtil::SchemaDelta SchemaUtil::ComputeCompatibilityDelta(
// guaranteed from our previous checks that all the old properties are also
// present in the new property config, so we can do a simple int comparison
// here to detect new required properties.
- if (new_parsed_property_configs.num_required_properties >
- old_required_properties) {
+ if (!IsSubset(new_parsed_property_configs.required_properties,
+ old_required_properties)) {
ICING_VLOG(1) << absl_ports::StrCat(
"New schema '", old_type_config.schema_type(),
"' has REQUIRED properties that are not "
@@ -1101,8 +1115,8 @@ const SchemaUtil::SchemaDelta SchemaUtil::ComputeCompatibilityDelta(
// indexed nested document properties), then the section ids may change.
// Since the section ids are stored in the index, we'll need to
// reindex everything.
- if (new_parsed_property_configs.num_indexed_properties >
- old_indexed_properties) {
+ if (!IsSubset(new_parsed_property_configs.indexed_properties,
+ old_indexed_properties)) {
ICING_VLOG(1) << "Set of indexed properties in schema type '"
<< old_type_config.schema_type()
<< "' has changed, required reindexing.";
@@ -1116,10 +1130,10 @@ const SchemaUtil::SchemaDelta SchemaUtil::ComputeCompatibilityDelta(
// join index. This is because we index all nested joinable properties, so
// adding a nested document property will most probably result in having
// more joinable properties.
- if (new_parsed_property_configs.num_joinable_properties >
- old_joinable_properties ||
- new_parsed_property_configs.num_nested_document_properties >
- old_nested_document_properties) {
+ if (!IsSubset(new_parsed_property_configs.joinable_properties,
+ old_joinable_properties) ||
+ !IsSubset(new_parsed_property_configs.nested_document_properties,
+ old_nested_document_properties)) {
ICING_VLOG(1) << "Set of joinable properties in schema type '"
<< old_type_config.schema_type()
<< "' has changed, required reconstructing joinable cache.";
diff --git a/icing/schema/schema-util.h b/icing/schema/schema-util.h
index 6d0ff73..4f09915 100644
--- a/icing/schema/schema-util.h
+++ b/icing/schema/schema-util.h
@@ -113,17 +113,17 @@ class SchemaUtil {
std::unordered_map<std::string_view, const PropertyConfigProto*>
property_config_map;
- // Total number of properties that have an indexing config
- int32_t num_indexed_properties = 0;
+ // Properties that have an indexing config
+ std::unordered_set<std::string_view> indexed_properties;
- // Total number of properties that were REQUIRED
- int32_t num_required_properties = 0;
+ // Properties that were REQUIRED
+ std::unordered_set<std::string_view> required_properties;
- // Total number of properties that have joinable config
- int32_t num_joinable_properties = 0;
+ // Properties that have joinable config
+ std::unordered_set<std::string_view> joinable_properties;
- // Total number of properties that have DataType::DOCUMENT
- int32_t num_nested_document_properties = 0;
+ // Properties that have DataType::DOCUMENT
+ std::unordered_set<std::string_view> nested_document_properties;
};
// This function validates:
diff --git a/icing/schema/schema-util_test.cc b/icing/schema/schema-util_test.cc
index 564bbc0..82683ba 100644
--- a/icing/schema/schema-util_test.cc
+++ b/icing/schema/schema-util_test.cc
@@ -2564,6 +2564,114 @@ TEST_P(SchemaUtilTest, DifferentSchemaTypeIsIncompatible) {
EXPECT_THAT(actual.schema_types_deleted, testing::IsEmpty());
}
+TEST_P(SchemaUtilTest, SameNumberOfRequiredFieldsCanBeIncompatible) {
+ SchemaProto old_schema =
+ SchemaBuilder()
+ .AddType(SchemaTypeConfigBuilder()
+ .SetType(kEmailType)
+ .AddProperty(PropertyConfigBuilder()
+ .SetName("Property1")
+ .SetDataType(TYPE_STRING)
+ .SetCardinality(CARDINALITY_REQUIRED)))
+ .Build();
+
+ SchemaProto new_schema =
+ SchemaBuilder()
+ .AddType(SchemaTypeConfigBuilder()
+ .SetType(kEmailType)
+ .AddProperty(
+ PropertyConfigBuilder()
+ .SetName("Property1")
+ .SetDataType(TYPE_STRING)
+ // Changing required to optional should be fine
+ .SetCardinality(CARDINALITY_OPTIONAL))
+ .AddProperty(
+ PropertyConfigBuilder()
+ .SetName("Property2")
+ .SetDataType(TYPE_STRING)
+ // Adding a new required property is incompatible
+ .SetCardinality(CARDINALITY_REQUIRED)))
+ .Build();
+
+ SchemaUtil::SchemaDelta delta = SchemaUtil::ComputeCompatibilityDelta(
+ old_schema, new_schema, /*new_schema_dependent_map=*/{});
+ EXPECT_THAT(delta.schema_types_incompatible,
+ testing::ElementsAre(kEmailType));
+ EXPECT_THAT(delta.schema_types_index_incompatible, testing::IsEmpty());
+ EXPECT_THAT(delta.schema_types_deleted, testing::IsEmpty());
+}
+
+TEST_P(SchemaUtilTest, SameNumberOfIndexedPropertiesCanMakeIndexIncompatible) {
+ SchemaProto old_schema =
+ SchemaBuilder()
+ .AddType(SchemaTypeConfigBuilder()
+ .SetType(kEmailType)
+ .AddProperty(PropertyConfigBuilder()
+ .SetName("Property1")
+ .SetDataTypeString(TERM_MATCH_EXACT,
+ TOKENIZER_PLAIN)
+ .SetCardinality(CARDINALITY_OPTIONAL)))
+ .Build();
+
+ SchemaProto new_schema =
+ SchemaBuilder()
+ .AddType(SchemaTypeConfigBuilder()
+ .SetType(kEmailType)
+ .AddProperty(PropertyConfigBuilder()
+ .SetName("Property1")
+ .SetDataType(TYPE_STRING)
+ .SetCardinality(CARDINALITY_OPTIONAL))
+ .AddProperty(PropertyConfigBuilder()
+ .SetName("Property2")
+ .SetDataTypeString(TERM_MATCH_EXACT,
+ TOKENIZER_PLAIN)
+ .SetCardinality(CARDINALITY_OPTIONAL)))
+ .Build();
+
+ SchemaUtil::SchemaDelta delta = SchemaUtil::ComputeCompatibilityDelta(
+ old_schema, new_schema, /*new_schema_dependent_map=*/{});
+ EXPECT_THAT(delta.schema_types_incompatible, testing::IsEmpty());
+ EXPECT_THAT(delta.schema_types_index_incompatible,
+ testing::ElementsAre(kEmailType));
+ EXPECT_THAT(delta.schema_types_deleted, testing::IsEmpty());
+}
+
+TEST_P(SchemaUtilTest, SameNumberOfJoinablePropertiesCanMakeJoinIncompatible) {
+ SchemaProto old_schema =
+ SchemaBuilder()
+ .AddType(SchemaTypeConfigBuilder()
+ .SetType(kEmailType)
+ .AddProperty(PropertyConfigBuilder()
+ .SetName("Property1")
+ .SetDataTypeJoinableString(
+ JOINABLE_VALUE_TYPE_QUALIFIED_ID)
+ .SetCardinality(CARDINALITY_OPTIONAL)))
+ .Build();
+
+ SchemaProto new_schema =
+ SchemaBuilder()
+ .AddType(SchemaTypeConfigBuilder()
+ .SetType(kEmailType)
+ .AddProperty(PropertyConfigBuilder()
+ .SetName("Property1")
+ .SetDataType(TYPE_STRING)
+ .SetCardinality(CARDINALITY_OPTIONAL))
+ .AddProperty(PropertyConfigBuilder()
+ .SetName("Property2")
+ .SetDataTypeJoinableString(
+ JOINABLE_VALUE_TYPE_QUALIFIED_ID)
+ .SetCardinality(CARDINALITY_OPTIONAL)))
+ .Build();
+
+ SchemaUtil::SchemaDelta delta = SchemaUtil::ComputeCompatibilityDelta(
+ old_schema, new_schema, /*new_schema_dependent_map=*/{});
+ EXPECT_THAT(delta.schema_types_incompatible, testing::IsEmpty());
+ EXPECT_THAT(delta.schema_types_index_incompatible, testing::IsEmpty());
+ EXPECT_THAT(delta.schema_types_deleted, testing::IsEmpty());
+ EXPECT_THAT(delta.schema_types_join_incompatible,
+ testing::ElementsAre(kEmailType));
+}
+
TEST_P(SchemaUtilTest, ChangingIndexedStringPropertiesMakesIndexIncompatible) {
// Configure old schema
SchemaProto schema_with_indexed_property =
@@ -3017,8 +3125,7 @@ TEST_P(SchemaUtilTest, DeletingIndexedDocumentPropertyIsIncompatible) {
EXPECT_THAT(result_schema_delta, Eq(schema_delta));
}
-TEST_P(SchemaUtilTest,
- DeletingNonIndexedDocumentPropertyIsIncompatible) {
+TEST_P(SchemaUtilTest, DeletingNonIndexedDocumentPropertyIsIncompatible) {
SchemaTypeConfigProto nested_schema =
SchemaTypeConfigBuilder()
.SetType(kEmailType)
generated by cgit v1.2.3 (git 2.25.1) at 2024-07-05 06:20:24 +0000