Struct field attributes can be removed without reprocessing.

4 files changed, 74 insertions, 4 deletions

diff --git a/searchcore/src/tests/proton/attribute/attribute_aspect_delayer/attribute_aspect_delayer_test.cpp b/searchcore/src/tests/proton/attribute/attribute_aspect_delayer/attribute_aspect_delayer_test.cpp
index 3ef2461c682..aeafcc26794 100644
--- a/searchcore/src/tests/proton/attribute/attribute_aspect_delayer/attribute_aspect_delayer_test.cpp
+++ b/searchcore/src/tests/proton/attribute/attribute_aspect_delayer/attribute_aspect_delayer_test.cpp
@@ -36,15 +36,24 @@ namespace proton
 
 namespace {
 
-AttributesConfig::Attribute make_sv_cfg(AttributesConfig::Attribute::Datatype dataType)
+AttributesConfig::Attribute make_sv_cfg(const vespalib::string &name, AttributesConfig::Attribute::Datatype dataType)
 {
     AttributesConfig::Attribute attr;
-    attr.name = "a";
+    attr.name = name;
     attr.datatype = dataType;
     attr.collectiontype = AttributesConfig::Attribute::Collectiontype::SINGLE;
     return attr;
 }
 
+AttributesConfig::Attribute make_sv_cfg(AttributesConfig::Attribute::Datatype dataType)
+{
+    return make_sv_cfg("a", dataType);
+}
+
+AttributesConfig::Attribute make_int32_sv_cfg(const vespalib::string &name) {
+    return make_sv_cfg(name, AttributesConfig::Attribute::Datatype::INT32);
+}
+
 AttributesConfig::Attribute make_int32_sv_cfg() {
     return make_sv_cfg(AttributesConfig::Attribute::Datatype::INT32);
 }
@@ -104,6 +113,14 @@ SummarymapConfig::Override make_geopos_override(const vespalib::string &name)
     return override;
 }
 
+SummarymapConfig::Override make_attribute_combiner_override(const vespalib::string &name)
+{
+    SummarymapConfig::Override override;
+    override.field = name;
+    override.command = "attributecombiner";
+    return override;
+}
+
 SummarymapConfig smCfg(std::vector<SummarymapConfig::Override> overrides)
 {
     SummarymapConfigBuilder result;
@@ -306,6 +323,29 @@ TEST_F("require that fast access flag change is not delayed, true->false edge, s
     TEST_DO(f.assertSummarymapConfig({make_attribute_override("a")}));
 }
 
+TEST_F("require that adding attribute aspect to struct field is not delayed if field type is changed", Fixture)
+{
+    f.setup(attrCfg({}), smCfg({}), attrCfg({make_int32_sv_cfg("array.a")}), smCfg({make_attribute_combiner_override("array")}));
+    TEST_DO(f.assertAttributeConfig({make_int32_sv_cfg("array.a")}));
+    TEST_DO(f.assertSummarymapConfig({make_attribute_combiner_override("array")}));
+}
+
+TEST_F("require that adding attribute aspect to struct field is delayed if field type is unchanged", Fixture)
+{
+    f.addFields({"array.a"});
+    f.setup(attrCfg({}), smCfg({}), attrCfg({make_int32_sv_cfg("array.a")}), smCfg({make_attribute_combiner_override("array")}));
+    TEST_DO(f.assertAttributeConfig({}));
+    TEST_DO(f.assertSummarymapConfig({}));
+}
+
+TEST_F("require that removing attribute aspect from struct field is not delayed", Fixture)
+{
+    f.addFields({"array.a"});
+    f.setup(attrCfg({make_int32_sv_cfg("array.a")}), smCfg({make_attribute_combiner_override("array")}), attrCfg({}), smCfg({}));
+    TEST_DO(f.assertAttributeConfig({}));
+    TEST_DO(f.assertSummarymapConfig({}));
+}
+
 }
 
 TEST_MAIN()
diff --git a/searchcore/src/tests/proton/reprocessing/attribute_reprocessing_initializer/attribute_reprocessing_initializer_test.cpp b/searchcore/src/tests/proton/reprocessing/attribute_reprocessing_initializer/attribute_reprocessing_initializer_test.cpp
index 93c192b57e8..7be774f7291 100644
--- a/searchcore/src/tests/proton/reprocessing/attribute_reprocessing_initializer/attribute_reprocessing_initializer_test.cpp
+++ b/searchcore/src/tests/proton/reprocessing/attribute_reprocessing_initializer/attribute_reprocessing_initializer_test.cpp
@@ -306,6 +306,18 @@ TEST("require that added attribute aspect with flushed attribute after interrupt
     EXPECT_TRUE(f.assertAttributes({}));
 }
 
+TEST_F("require that removed attribute aspect from struct field does not require document field populate", Fixture)
+{
+    f.addOldConfig({"array.a"}, {"array.a"}).addNewConfig({"array.a"}, {}).init();
+    EXPECT_TRUE(f.assertFields({}));
+}
+
+TEST_F("require that added attribute aspect to struct field requires attribute populate", Fixture)
+{
+    f.addOldConfig({"array.a"}, {}).addNewConfig({"array.a"}, {"array.a"}).init();
+    EXPECT_TRUE(f.assertAttributes({"array.a"}));
+}
+
 TEST_MAIN()
 {
     TEST_RUN_ALL();
diff --git a/searchcore/src/vespa/searchcore/proton/attribute/attribute_aspect_delayer.cpp b/searchcore/src/vespa/searchcore/proton/attribute/attribute_aspect_delayer.cpp
index 9807faa5021..cf803ec0368 100644
--- a/searchcore/src/vespa/searchcore/proton/attribute/attribute_aspect_delayer.cpp
+++ b/searchcore/src/vespa/searchcore/proton/attribute/attribute_aspect_delayer.cpp
@@ -29,11 +29,15 @@ bool fastPartialUpdateAttribute(const search::attribute::Config &cfg) {
             (basicType != BasicType::Type::REFERENCE));
 }
 
+bool isStructFieldAttribute(const vespalib::string &name) {
+    return name.find('.') != vespalib::string::npos;
+}
+
 bool willTriggerReprocessOnAttributeAspectRemoval(const search::attribute::Config &cfg,
                                                   const IIndexschemaInspector &indexschemaInspector,
                                                   const vespalib::string &name)
 {
-    return fastPartialUpdateAttribute(cfg) && !indexschemaInspector.isStringIndex(name);
+    return fastPartialUpdateAttribute(cfg) && !indexschemaInspector.isStringIndex(name) && !isStructFieldAttribute(name);
 }
 
 
@@ -73,6 +77,7 @@ handleNewAttributes(const AttributesConfig &oldAttributesConfig,
                     SummarymapConfigBuilder &summarymapConfig)
 {
     vespalib::hash_set<vespalib::string> delayed;
+    vespalib::hash_set<vespalib::string> delayedStruct;
     AttributesConfigHash oldAttrs(oldAttributesConfig.attribute);
     for (const auto &newAttr : newAttributesConfig.attribute) {
         search::attribute::Config newCfg = ConfigConverter::convert(newAttr);
@@ -102,6 +107,10 @@ handleNewAttributes(const AttributesConfig &oldAttributesConfig,
             } else {
                 // Delay addition of attribute aspect
                 delayed.insert(newAttr.name);
+                auto pos = newAttr.name.find('.');
+                if (pos != vespalib::string::npos) {
+                    delayedStruct.insert(newAttr.name.substr(0, pos));
+                }
             }
         }
     }
@@ -111,6 +120,11 @@ handleNewAttributes(const AttributesConfig &oldAttributesConfig,
             if (itr == delayed.end()) {
                 summarymapConfig.override.emplace_back(override);
             }
+        } else if (override.command == "attributecombiner") {
+            auto itr = delayedStruct.find(override.field);
+            if (itr == delayedStruct.end()) {
+                summarymapConfig.override.emplace_back(override);
+            }
         } else {
             summarymapConfig.override.emplace_back(override);
         }
diff --git a/searchcore/src/vespa/searchcore/proton/reprocessing/attribute_reprocessing_initializer.cpp b/searchcore/src/vespa/searchcore/proton/reprocessing/attribute_reprocessing_initializer.cpp
index 711f8ebdcaf..1df812b0c61 100644
--- a/searchcore/src/vespa/searchcore/proton/reprocessing/attribute_reprocessing_initializer.cpp
+++ b/searchcore/src/vespa/searchcore/proton/reprocessing/attribute_reprocessing_initializer.cpp
@@ -38,6 +38,9 @@ bool fastPartialUpdateAttribute(BasicType::Type attrType) {
             (attrType != BasicType::Type::REFERENCE));
 }
 
+bool isStructFieldAttribute(const vespalib::string &name) {
+    return name.find('.') != vespalib::string::npos;
+}
 
 FilterAttributeManager::AttributeSet
 getAttributeSetToPopulate(const ARIConfig &newCfg,
@@ -100,7 +103,8 @@ getFieldsToPopulate(const ARIConfig &newCfg,
         // keep the original in order to preserve annotations.
         bool wasStringIndexField = oldIndexschemaInspector.isStringIndex(name);
         bool populateField = !inNewAttrMgr && unchangedField && !wasStringIndexField &&
-                             fastPartialUpdateAttribute(attrType.type());
+                             fastPartialUpdateAttribute(attrType.type()) &&
+                             !isStructFieldAttribute(name);
         LOG(debug, "getFieldsToPopulate(): name='%s', inNewAttrMgr=%s, unchangedField=%s, "
                 "wasStringIndexField=%s, dataType=%s, populate=%s",
                 name.c_str(), toStr(inNewAttrMgr), toStr(unchangedField),