Refactor parameters for predicate attributes.

8 files changed, 96 insertions, 36 deletions

diff --git a/searchcommon/src/vespa/searchcommon/attribute/config.cpp b/searchcommon/src/vespa/searchcommon/attribute/config.cpp
index 4d1cd540033..e7a4c28b27a 100644
--- a/searchcommon/src/vespa/searchcommon/attribute/config.cpp
+++ b/searchcommon/src/vespa/searchcommon/attribute/config.cpp
@@ -18,10 +18,7 @@ Config::Config() :
     _fastAccess(false),
     _growStrategy(),
     _compactionStrategy(),
-    _arity(8),
-    _lower_bound(LLONG_MIN),
-    _upper_bound(LLONG_MAX),
-    _dense_posting_list_threshold(0.4),
+    _predicateParams(),
     _tensorType(vespalib::eval::ValueType::error_type())
 {
 }
@@ -40,10 +37,7 @@ Config::Config(BasicType bt,
       _fastAccess(false),
       _growStrategy(),
       _compactionStrategy(),
-      _arity(8),
-      _lower_bound(LLONG_MIN),
-      _upper_bound(LLONG_MAX),
-      _dense_posting_list_threshold(0.4),
+      _predicateParams(),
       _tensorType(vespalib::eval::ValueType::error_type())
 {
 }
diff --git a/searchcommon/src/vespa/searchcommon/attribute/config.h b/searchcommon/src/vespa/searchcommon/attribute/config.h
index 7a5919620bf..be2a1d2777d 100644
--- a/searchcommon/src/vespa/searchcommon/attribute/config.h
+++ b/searchcommon/src/vespa/searchcommon/attribute/config.h
@@ -7,6 +7,7 @@
 #include <vespa/searchcommon/common/growstrategy.h>
 #include <vespa/searchcommon/common/compaction_strategy.h>
 #include <vespa/eval/eval/value_type.h>
+#include "predicate_params.h"
 
 namespace search {
 namespace attribute {
@@ -25,10 +26,7 @@ public:
     CollectionType collectionType()       const { return _type; }
     bool fastSearch()                     const { return _fastSearch; }
     bool huge()                           const { return _huge; }
-    uint32_t arity()                      const { return _arity; }
-    int64_t lower_bound()                 const { return _lower_bound; }
-    int64_t upper_bound()                 const { return _upper_bound; }
-    double dense_posting_list_threshold() const { return _dense_posting_list_threshold; }
+    const PredicateParams &predicateParams() const { return _predicateParams; }
     vespalib::eval::ValueType tensorType() const { return _tensorType; }
 
     /**
@@ -67,10 +65,7 @@ public:
     const CompactionStrategy &getCompactionStrategy() const { return _compactionStrategy; }
     void setHuge(bool v)                         { _huge = v; }
     void setFastSearch(bool v)                   { _fastSearch = v; }
-    void setArity(uint32_t v)                    { _arity = v; }
-    void setBounds(int64_t lower, int64_t upper) { _lower_bound = lower;
-                                                   _upper_bound = upper; }
-    void setDensePostingListThreshold(double v)  { _dense_posting_list_threshold = v; }
+    void setPredicateParams(const PredicateParams &v) { _predicateParams = v; }
     void setTensorType(const vespalib::eval::ValueType &tensorType_in) {
         _tensorType = tensorType_in;
     }
@@ -124,10 +119,7 @@ public:
                _fastAccess == b._fastAccess &&
                _growStrategy == b._growStrategy &&
                _compactionStrategy == b._compactionStrategy &&
-               _arity == b._arity &&
-               _lower_bound == b._lower_bound &&
-               _upper_bound == b._upper_bound &&
-               _dense_posting_list_threshold == b._dense_posting_list_threshold &&
+               _predicateParams == b._predicateParams &&
             (_basicType.type() != BasicType::Type::TENSOR ||
              _tensorType == b._tensorType);
     }
@@ -143,10 +135,7 @@ private:
     bool           _fastAccess;
     GrowStrategy   _growStrategy;
     CompactionStrategy _compactionStrategy;
-    uint32_t       _arity;
-    int64_t        _lower_bound;
-    int64_t        _upper_bound;
-    double         _dense_posting_list_threshold;
+    PredicateParams    _predicateParams;
     vespalib::eval::ValueType _tensorType;
 };
 }  // namespace attribute
diff --git a/searchcommon/src/vespa/searchcommon/attribute/persistent_predicate_params.h b/searchcommon/src/vespa/searchcommon/attribute/persistent_predicate_params.h
new file mode 100644
index 00000000000..1aed2dad72c
--- /dev/null
+++ b/searchcommon/src/vespa/searchcommon/attribute/persistent_predicate_params.h
@@ -0,0 +1,37 @@
+// Copyright 2017 Yahoo Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+
+#pragma once
+
+namespace search {
+namespace attribute {
+
+/*
+ * Persistent parameters for predicate attributes.
+ */
+class PersistentPredicateParams {
+    uint32_t _arity;
+    int64_t  _lower_bound;
+    int64_t  _upper_bound;
+
+public:
+    PersistentPredicateParams()
+        : _arity(8),
+          _lower_bound(std::numeric_limits<int64_t>::min()),
+          _upper_bound(std::numeric_limits<int64_t>::max())
+    {
+    }
+    uint32_t arity()                      const { return _arity; }
+    int64_t lower_bound()                 const { return _lower_bound; }
+    int64_t upper_bound()                 const { return _upper_bound; }
+    void setArity(uint32_t v)                    { _arity = v; }
+    void setBounds(int64_t lower, int64_t upper) { _lower_bound = lower; _upper_bound = upper; }
+
+    bool operator==(const PersistentPredicateParams &rhs) const {
+        return ((_arity == rhs._arity) &&
+                (_lower_bound == rhs._lower_bound) &&
+                (_upper_bound == rhs._upper_bound));
+    }
+};
+
+}  // namespace attribute
+}  // namespace search
diff --git a/searchcommon/src/vespa/searchcommon/attribute/predicate_params.h b/searchcommon/src/vespa/searchcommon/attribute/predicate_params.h
new file mode 100644
index 00000000000..8e8e0605b17
--- /dev/null
+++ b/searchcommon/src/vespa/searchcommon/attribute/predicate_params.h
@@ -0,0 +1,32 @@
+// Copyright 2017 Yahoo Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+
+#pragma once
+
+#include "persistent_predicate_params.h"
+
+namespace search {
+namespace attribute {
+
+/*
+ * Parameters for predicate attributes.
+ */
+class PredicateParams : public PersistentPredicateParams
+{
+    double _dense_posting_list_threshold;
+public:
+    PredicateParams()
+        : PersistentPredicateParams(),
+          _dense_posting_list_threshold(0.4)
+    {
+    }
+
+    double dense_posting_list_threshold() const { return _dense_posting_list_threshold; }
+    void setDensePostingListThreshold(double v)  { _dense_posting_list_threshold = v; }
+    bool operator==(const PredicateParams &rhs) const {
+        return (PersistentPredicateParams::operator==(rhs) &&
+                (_dense_posting_list_threshold == rhs._dense_posting_list_threshold));
+    }
+};
+
+}  // namespace attribute
+}  // namespace search
diff --git a/searchcore/src/tests/proton/attribute/attribute_manager/attribute_manager_test.cpp b/searchcore/src/tests/proton/attribute/attribute_manager/attribute_manager_test.cpp
index 50d79b42910..7232f997b0e 100644
--- a/searchcore/src/tests/proton/attribute/attribute_manager/attribute_manager_test.cpp
+++ b/searchcore/src/tests/proton/attribute/attribute_manager/attribute_manager_test.cpp
@@ -761,9 +761,14 @@ TEST_F("require that attribute vector of wrong type is dropped", BaseFixture)
     AVConfig dense_tensor(BasicType::TENSOR);
     dense_tensor.setTensorType(ValueType::from_spec("tensor(x[10])"));
     AVConfig predicate(BasicType::PREDICATE);
-    predicate.setArity(2);
+    using PredicateParams = search::attribute::PredicateParams;
+    PredicateParams predicateParams;
+    predicateParams.setArity(2);
+    predicate.setPredicateParams(predicateParams);
     AVConfig predicate2(BasicType::PREDICATE);
-    predicate2.setArity(4);
+    PredicateParams predicateParams2;
+    predicateParams2.setArity(4);
+    predicate2.setPredicateParams(predicateParams2);
 
     auto am1(std::make_shared<proton::AttributeManager>
              (test_dir, "test.subdb", TuneFileAttributes(),
diff --git a/searchcore/src/vespa/searchcore/proton/attribute/attributemanager.cpp b/searchcore/src/vespa/searchcore/proton/attribute/attributemanager.cpp
index 2b519806248..fa4f9258e36 100644
--- a/searchcore/src/vespa/searchcore/proton/attribute/attributemanager.cpp
+++ b/searchcore/src/vespa/searchcore/proton/attribute/attributemanager.cpp
@@ -46,9 +46,10 @@ bool matchingTypes(const AttributeVector::SP &av, const search::attribute::Confi
             }
         }
         if (newConfig.basicType().type() == BasicType::PREDICATE) {
-            if ((oldConfig.arity() != newConfig.arity()) ||
-                (oldConfig.lower_bound() != newConfig.lower_bound()) ||
-                (oldConfig.upper_bound() != newConfig.upper_bound())) {
+            using Params = search::attribute::PersistentPredicateParams;
+            const Params &oldParams = oldConfig.predicateParams();
+            const Params &newParams = newConfig.predicateParams();
+            if (!(oldParams == newParams)) {
                 return false;
             }
         }
diff --git a/searchlib/src/vespa/searchlib/attribute/configconverter.cpp b/searchlib/src/vespa/searchlib/attribute/configconverter.cpp
index 8c911a78d43..9e3c6156dbf 100644
--- a/searchlib/src/vespa/searchlib/attribute/configconverter.cpp
+++ b/searchlib/src/vespa/searchlib/attribute/configconverter.cpp
@@ -65,15 +65,17 @@ ConfigConverter::convert(const AttributesConfig::Attribute & cfg)
     cType.removeIfZero(cfg.removeifzero);
     cType.createIfNonExistant(cfg.createifnonexistent);
     Config retval(bType, cType);
+    PredicateParams predicateParams;
     retval.setFastSearch(cfg.fastsearch);
     retval.setHuge(cfg.huge);
     retval.setEnableBitVectors(cfg.enablebitvectors);
     retval.setEnableOnlyBitVector(cfg.enableonlybitvector);
     retval.setIsFilter(cfg.enableonlybitvector);
     retval.setFastAccess(cfg.fastaccess);
-    retval.setArity(cfg.arity);
-    retval.setBounds(cfg.lowerbound, cfg.upperbound);
-    retval.setDensePostingListThreshold(cfg.densepostinglistthreshold);
+    predicateParams.setArity(cfg.arity);
+    predicateParams.setBounds(cfg.lowerbound, cfg.upperbound);
+    predicateParams.setDensePostingListThreshold(cfg.densepostinglistthreshold);
+    retval.setPredicateParams(predicateParams);
     if (retval.basicType().type() == BasicType::Type::TENSOR) {
         if (!cfg.tensortype.empty()) {
             retval.setTensorType(ValueType::from_spec(cfg.tensortype));
diff --git a/searchlib/src/vespa/searchlib/attribute/predicate_attribute.cpp b/searchlib/src/vespa/searchlib/attribute/predicate_attribute.cpp
index 3d3bbdcae43..2342c374643 100644
--- a/searchlib/src/vespa/searchlib/attribute/predicate_attribute.cpp
+++ b/searchlib/src/vespa/searchlib/attribute/predicate_attribute.cpp
@@ -55,7 +55,7 @@ int64_t adjustUpperBound(int32_t arity, int64_t upper_bound) {
 }
 
 SimpleIndexConfig createSimpleIndexConfig(const search::attribute::Config &config) {
-    return SimpleIndexConfig(config.dense_posting_list_threshold(), config.getGrowStrategy());
+    return SimpleIndexConfig(config.predicateParams().dense_posting_list_threshold(), config.getGrowStrategy());
 }
 
 }  // namespace
@@ -66,9 +66,9 @@ PredicateAttribute::PredicateAttribute(const vespalib::string &base_file_name,
       _base_file_name(base_file_name),
       _limit_provider(*this),
       _index(new PredicateIndex(getGenerationHandler(), getGenerationHolder(),
-                                _limit_provider, createSimpleIndexConfig(config), config.arity())),
-      _lower_bound(adjustLowerBound(config.arity(), config.lower_bound())),
-      _upper_bound(adjustUpperBound(config.arity(), config.upper_bound())),
+                                _limit_provider, createSimpleIndexConfig(config), config.predicateParams().arity())),
+      _lower_bound(adjustLowerBound(config.predicateParams().arity(), config.predicateParams().lower_bound())),
+      _upper_bound(adjustUpperBound(config.predicateParams().arity(), config.predicateParams().upper_bound())),
       _min_feature(config.getGrowStrategy(), getGenerationHolder()),
       _interval_range_vector(config.getGrowStrategy(), getGenerationHolder()),
       _max_interval_range(1)