WIP around problems with Range Index efficiency

exist-core/src/main/java/org/exist/collections/CollectionConfigurationManager.java

@@ -221,7 +221,7 @@ protected CollectionConfiguration getConfiguration(final Collection collection)
             }
 
             // use default configuration
-            return defaultConfig;
+            return defaultConfig;  // TODO(AR) this can result in a lot of the same configs being added to lists and maps
         }
     }
 

exist-core/src/main/java/org/exist/collections/MutableCollection.java

@@ -1970,11 +1970,12 @@ private void releaseReader(final DBBroker broker, final XMLReader reader) {
     public IndexSpec getIndexConfiguration(final DBBroker broker) {
         final CollectionConfiguration conf = getConfiguration(broker);
         //If the collection has its own config...
-        if (conf == null) {
-            return broker.getIndexConfiguration();
+        if (conf != null) {
+            return conf.getIndexConfiguration();
         }
         //... otherwise return the general config (the broker's one)
-        return conf.getIndexConfiguration();
+        // TODO(AR) what is the difference between this and CollectionConfigurationManagement#defaultConfig
+        return broker.getIndexConfiguration();  // TODO(AR) this can result in a lot of the same object being added to lists and maps
     }
 
     @Override

exist-core/src/main/java/org/exist/xquery/value/ValueSequence.java

@@ -879,7 +879,7 @@ public int compare(final Item o1, final Item o2) {
     private class CollectionIterator implements Iterator<Collection> {
         private Collection nextCollection = null;
         private int pos = 0;
-
+//TODO(AR) can this return the same collection multiple times? should it?
         CollectionIterator() {
             next();
         }

extensions/indexes/range/src/main/java/org/exist/indexing/range/RangeIndexConfig.java

@@ -48,11 +48,13 @@ public class RangeIndexConfig {
 
     private static final Logger LOG = LogManager.getLogger(RangeIndexConfig.class);
 
-    private Map<QName, RangeIndexConfigElement> paths = new TreeMap<>();
+    // treemap is used to preserve order
+    private final Map<QName, RangeIndexConfigElement> paths = new TreeMap<>();
 
     private Analyzer analyzer;
 
-    private PathIterator iterator = new PathIterator();
+    // TODO(AR) should start as null
+    private final PathIterator iterator = new PathIterator();
 
     public RangeIndexConfig() {
         // default analyzer

extensions/indexes/range/src/main/java/org/exist/xquery/modules/range/OptimizeFieldPragma.java

@@ -95,7 +95,9 @@ private Expression tryRewriteToFields(LocationStep locationStep, final Predicate
         // without context path, we cannot rewrite the entire query
         if (contextPath != null) {
             final List<Predicate> notOptimizable = new ArrayList<>(preds.length);
-            List<RangeIndexConfig> configs = getConfigurations(contextSequence);
+
+            // TODO(AR) this list contains many duplicates, as the contextSequence can contain nodes from the same collection
+            List<RangeIndexConfig> configs = getConfigurations(contextSequence);  //TODO(AR) this is returning one config per-collection in the db, yet we may know the first call (locationStep.parent) is fn:collection - so this could be much more efficient..
             // walk through the predicates attached to the current location step
             // check if expression can be optimized
 
@@ -292,6 +294,8 @@ private List<ComplexRangeIndexConfigElement> findConfigurations(NodePath path, L
         return rices;
     }
 
+    // TODO(AR) is this returning too many configs? -- 296 are returned for benjamins
+    // TODO(AR) how are these arranged, a list does not seem efficient
     private List<RangeIndexConfig> getConfigurations(Sequence contextSequence) {
         List<RangeIndexConfig> configs = new ArrayList<>();
         for (final Iterator<Collection> i = contextSequence.getCollectionIterator(); i.hasNext(); ) {

extensions/indexes/range/src/main/java/org/exist/xquery/modules/range/RangeQueryRewriter.java

@@ -63,7 +63,7 @@ public Pragma rewriteLocationStep(final LocationStep locationStep) throws XPathE
                 NodePath contextPath = toNodePath(getPrecedingSteps(locationStep));
                 // process the remaining predicates
                 for (Predicate pred : preds) {
-                    if (pred.getLength() != 1) {
+                    if (pred.getSubExpressionCount() != 1) {
                         // can only optimize predicates with one expression
                         break;
                     }
@@ -127,7 +127,7 @@ public Pragma rewriteLocationStep(final LocationStep locationStep) throws XPathE
     }
 
     protected static Lookup rewrite(Expression expression, NodePath path) throws XPathException {
-        ArrayList<Expression> eqArgs = new ArrayList<>(2);
+        final List<Expression> eqArgs = new ArrayList<>(2);
         if (expression instanceof GeneralComparison) {
             GeneralComparison comparison = (GeneralComparison) expression;
             eqArgs.add(comparison.getLeft());