From 16911d96038ca4ffa5a64a416de3fd86fa1bb5b5 Mon Sep 17 00:00:00 2001 From: Clayton Sims Date: Tue, 26 May 2020 14:43:39 -0400 Subject: [PATCH 1/4] Create logical or optimization --- .../query/LogicalIndexedValuesLookup.java | 44 +++++ .../commcare/cases/query/QueryHandler.java | 2 +- .../commcare/cases/query/QueryPlanner.java | 4 +- .../handlers/LogicalValueIndexHandler.java | 186 ++++++++++++++++++ .../cases/util/StorageBackedTreeRoot.java | 10 +- 5 files changed, 241 insertions(+), 5 deletions(-) create mode 100644 src/main/java/org/commcare/cases/query/LogicalIndexedValuesLookup.java create mode 100644 src/main/java/org/commcare/cases/query/handlers/LogicalValueIndexHandler.java diff --git a/src/main/java/org/commcare/cases/query/LogicalIndexedValuesLookup.java b/src/main/java/org/commcare/cases/query/LogicalIndexedValuesLookup.java new file mode 100644 index 0000000000..583f7bdf90 --- /dev/null +++ b/src/main/java/org/commcare/cases/query/LogicalIndexedValuesLookup.java @@ -0,0 +1,44 @@ +package org.commcare.cases.query; + +import org.javarosa.xpath.expr.XPathBoolExpr; +import org.javarosa.xpath.expr.XPathOpExpr; + +/** + * + * Captures logical expressions which combine indexed value lookups + * + * Created by ctsims on 05/22/2020 + */ + +public class LogicalIndexedValuesLookup implements PredicateProfile { + IndexedValueLookup a; + IndexedValueLookup b; + + int operator; + + public LogicalIndexedValuesLookup(IndexedValueLookup a, IndexedValueLookup b, int operator) { + this.a = a; + this.b = b; + if(operator != XPathBoolExpr.OR && operator!= XPathBoolExpr.AND) { + throw new RuntimeException("Must be OR | AND"); + } + this.operator = operator; + } + + public IndexedValueLookup getA() { + return a; + } + + public IndexedValueLookup getB() { + return b; + } + + public int getOperator() { + return operator; + } + + @Override + public String getKey() { + return a.getKey(); + } +} diff --git a/src/main/java/org/commcare/cases/query/QueryHandler.java b/src/main/java/org/commcare/cases/query/QueryHandler.java index ba256d1161..722e5f5ba7 100644 --- a/src/main/java/org/commcare/cases/query/QueryHandler.java +++ b/src/main/java/org/commcare/cases/query/QueryHandler.java @@ -81,7 +81,7 @@ Collection collectPredicateProfiles( * can return null, which will signal the the query couldn't be run and no predicates have * been evaluated. */ - List loadProfileMatches(T querySet, QueryContext queryContext); + Collection loadProfileMatches(T querySet, QueryContext queryContext); /** * Given a succesful profile match, this method updates the predicateprofiles to remove profiles diff --git a/src/main/java/org/commcare/cases/query/QueryPlanner.java b/src/main/java/org/commcare/cases/query/QueryPlanner.java index 0f98566e23..8f4ba4089b 100644 --- a/src/main/java/org/commcare/cases/query/QueryPlanner.java +++ b/src/main/java/org/commcare/cases/query/QueryPlanner.java @@ -27,13 +27,13 @@ public class QueryPlanner { * * Note: Should profiles that have been run should be removed by the handler */ - public List attemptProfiledQuery(Vector profiles, + public Collection attemptProfiledQuery(Vector profiles, QueryContext currentQueryContext){ for (int i = 0 ; i < handlers.size() ; ++i) { QueryHandler handler = handlers.get(i); Object queryPlan = handler.profileHandledQuerySet(profiles); if (queryPlan != null) { - List retVal = handler.loadProfileMatches(queryPlan, currentQueryContext); + Collection retVal = handler.loadProfileMatches(queryPlan, currentQueryContext); if (retVal != null) { handler.updateProfiles(queryPlan, profiles); return retVal; diff --git a/src/main/java/org/commcare/cases/query/handlers/LogicalValueIndexHandler.java b/src/main/java/org/commcare/cases/query/handlers/LogicalValueIndexHandler.java new file mode 100644 index 0000000000..127bf46fc4 --- /dev/null +++ b/src/main/java/org/commcare/cases/query/handlers/LogicalValueIndexHandler.java @@ -0,0 +1,186 @@ +package org.commcare.cases.query.handlers; + +import org.commcare.cases.query.IndexedSetMemberLookup; +import org.commcare.cases.query.IndexedValueLookup; +import org.commcare.cases.query.LogicalIndexedValuesLookup; +import org.commcare.cases.query.PredicateProfile; +import org.commcare.cases.query.QueryContext; +import org.commcare.cases.query.QueryHandler; +import org.commcare.cases.query.queryset.ModelQueryLookup; +import org.commcare.cases.query.queryset.ModelQuerySetMatcher; +import org.commcare.cases.query.queryset.QuerySetLookup; +import org.commcare.cases.util.StorageBackedTreeRoot; +import org.javarosa.core.model.condition.EvaluationContext; +import org.javarosa.core.model.instance.TreeReference; +import org.javarosa.core.model.trace.EvaluationTrace; +import org.javarosa.core.services.storage.IStorageUtilityIndexed; +import org.javarosa.core.util.DataUtil; +import org.javarosa.xpath.expr.FunctionUtils; +import org.javarosa.xpath.expr.XPathBoolExpr; +import org.javarosa.xpath.expr.XPathEqExpr; +import org.javarosa.xpath.expr.XPathExpression; +import org.javarosa.xpath.expr.XPathPathExpr; + +import java.util.Collection; +import java.util.Enumeration; +import java.util.Hashtable; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Vector; + +import kotlin.collections.IndexedValue; + +/** + * Performs a "selected in" lookup operation from an appropriately backed storage table + * against a + * + * Created by ctsims on 1/31/2017. + */ + +public class LogicalValueIndexHandler implements QueryHandler { + Hashtable indices; + + private IStorageUtilityIndexed storage; + + public LogicalValueIndexHandler(Hashtable indices, + IStorageUtilityIndexed storage) { + this.indices = indices; + this.storage = storage; + } + + @Override + public int getExpectedRuntime() { + return 1; + } + + @Override + public LogicalIndexedValuesLookup profileHandledQuerySet(Vector profiles) { + if (profiles.get(0) instanceof LogicalIndexedValuesLookup) { + return (LogicalIndexedValuesLookup)profiles.get(0); + } + return null; + } + + @Override + public Collection loadProfileMatches(LogicalIndexedValuesLookup lookupExpr, QueryContext queryContext) { + if (lookupExpr.getOperator() == XPathBoolExpr.AND) { + LinkedHashSet ids = new LinkedHashSet<>(); + + String[] namesToMatch = new String[2]; + String[] valuesToMatch = new String[2]; + + namesToMatch[0] = lookupExpr.getA().key; + valuesToMatch[0] = (String)lookupExpr.getA().value; + + namesToMatch[1] = lookupExpr.getB().key; + valuesToMatch[1] = (String)lookupExpr.getB().value; + + String cacheKey = namesToMatch[0] + "=" + valuesToMatch[0] + + " AND " + namesToMatch[1] + "=" + valuesToMatch[1]; + + EvaluationTrace trace = + new EvaluationTrace("Logical Combination Lookup:|" + cacheKey); + + List results = storage.getIDsForValues(namesToMatch, valuesToMatch, ids); + + trace.setOutcome("Results: " + ids.size()); + queryContext.reportTrace(trace); + + return results; + } else if (lookupExpr.getOperator() == XPathBoolExpr.OR) { + LinkedHashSet ids = new LinkedHashSet<>(); + + String cacheKey = lookupExpr.getA().key + "=" + lookupExpr.getA().value + + " AND " + lookupExpr.getB().key + "=" + lookupExpr.getB().value; + + EvaluationTrace trace = + new EvaluationTrace("Logical Combination Lookup:|" + cacheKey); + + storage.getIDsForValues(new String[] {lookupExpr.getA().key}, new String[] {(String)lookupExpr.getA().value}, ids); + + storage.getIDsForValues(new String[] {lookupExpr.getB().key}, new String[] {(String)lookupExpr.getB().value}, ids); + + trace.setOutcome("Results: " + ids.size()); + queryContext.reportTrace(trace); + + return ids; + } + return null; + } + + @Override + public void updateProfiles(LogicalIndexedValuesLookup querySet, Vector profiles) { + profiles.remove(querySet); + } + + @Override + public Collection collectPredicateProfiles(Vector predicates, + QueryContext context, + EvaluationContext evalContext) { + + LogicalIndexedValuesLookup lookup = + getLogicalIndexedValueLookupIfExists(predicates.elementAt(0), evalContext); + + if (lookup == null) { + return null; + } + + Vector newProfile = new Vector<>(); + newProfile.add(lookup); + return newProfile; + + } + + public LogicalIndexedValuesLookup getLogicalIndexedValueLookupIfExists(XPathExpression inExpr, + EvaluationContext evalContext) { + if(!(inExpr instanceof XPathBoolExpr)) { + return null; + } + + XPathBoolExpr expr = (XPathBoolExpr)inExpr; + + IndexedValueLookup aLookup = identifyIndexedValuePredicate(expr.a, evalContext); + + if(aLookup == null) { + return null; + } + + IndexedValueLookup bLookup = identifyIndexedValuePredicate(expr.b, evalContext); + + if(bLookup == null) { + return null; + } + LogicalIndexedValuesLookup lookup = + new LogicalIndexedValuesLookup(aLookup, bLookup, expr.op); + + return lookup; + } + + public IndexedValueLookup identifyIndexedValuePredicate(XPathExpression xpe, + EvaluationContext evalContext) { + if (xpe instanceof XPathEqExpr && ((XPathEqExpr)xpe).op == XPathEqExpr.EQ) { + XPathExpression left = ((XPathEqExpr)xpe).a; + if (left instanceof XPathPathExpr) { + + for (Enumeration en = indices.keys(); en.hasMoreElements(); ) { + XPathPathExpr expr = (XPathPathExpr)en.nextElement(); + if (expr.matches(left)) { + String filterIndex = translateFilterExpr(expr, (XPathPathExpr)left, indices); + + //TODO: We need a way to determine that this value does not also depend on anything in the current context, not + //sure the best way to do that....? Maybe tell the evaluation context to skip out here if it detects a request + //to resolve in a certain area? + Object o = FunctionUtils.unpack(((XPathEqExpr)xpe).b.eval(evalContext)); + return new IndexedValueLookup(filterIndex, o); + } + } + } + } + return null; + } + + private String translateFilterExpr(XPathPathExpr expressionTemplate, XPathPathExpr matchingExpr, + Hashtable indices) { + return indices.get(expressionTemplate); + } +} diff --git a/src/main/java/org/commcare/cases/util/StorageBackedTreeRoot.java b/src/main/java/org/commcare/cases/util/StorageBackedTreeRoot.java index 2f3a7091eb..7b1bf044ba 100644 --- a/src/main/java/org/commcare/cases/util/StorageBackedTreeRoot.java +++ b/src/main/java/org/commcare/cases/util/StorageBackedTreeRoot.java @@ -5,6 +5,7 @@ import org.commcare.cases.query.IndexedValueLookup; import org.commcare.cases.query.PredicateProfile; import org.commcare.cases.query.handlers.BasicStorageBackedCachingQueryHandler; +import org.commcare.cases.query.handlers.LogicalValueIndexHandler; import org.commcare.modern.engine.cases.RecordSetResultCache; import org.commcare.modern.util.PerformanceTuningUtil; import org.javarosa.core.model.condition.EvaluationContext; @@ -172,7 +173,12 @@ private void collectNativePredicateProfiles(Vector predicates, } } } - + LogicalIndexedValuesLookup lookup = new LogicalValueIndexHandler(indices, getStorage()). + getLogicalIndexedValueLookupIfExists(xpe, evalContext); + if (lookup != null) { + optimizations.addElement(lookup); + continue predicate; + } //There's only one case where we want to keep moving along, and we would have triggered it if it were going to happen, //so otherwise, just get outta here. @@ -205,7 +211,7 @@ private Collection processPredicates(Vector toRemove, int predicatesProcessed = 0; while (profiles.size() > 0) { int startCount = profiles.size(); - List plannedQueryResults = + Collection plannedQueryResults = this.getQueryPlanner().attemptProfiledQuery(profiles, currentQueryContext); if (plannedQueryResults != null) { From e0d97574328bcf514ff3e0a95ff00071914a782a Mon Sep 17 00:00:00 2001 From: Clayton Sims Date: Tue, 26 May 2020 17:34:21 -0400 Subject: [PATCH 2/4] Clearer Trace output --- .../cases/query/handlers/LogicalValueIndexHandler.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/main/java/org/commcare/cases/query/handlers/LogicalValueIndexHandler.java b/src/main/java/org/commcare/cases/query/handlers/LogicalValueIndexHandler.java index 127bf46fc4..dc2a0ffde3 100644 --- a/src/main/java/org/commcare/cases/query/handlers/LogicalValueIndexHandler.java +++ b/src/main/java/org/commcare/cases/query/handlers/LogicalValueIndexHandler.java @@ -79,7 +79,7 @@ public Collection loadProfileMatches(LogicalIndexedValuesLookup lookupE " AND " + namesToMatch[1] + "=" + valuesToMatch[1]; EvaluationTrace trace = - new EvaluationTrace("Logical Combination Lookup:|" + cacheKey); + new EvaluationTrace("Logical Combination Lookup |" + cacheKey); List results = storage.getIDsForValues(namesToMatch, valuesToMatch, ids); @@ -91,16 +91,16 @@ public Collection loadProfileMatches(LogicalIndexedValuesLookup lookupE LinkedHashSet ids = new LinkedHashSet<>(); String cacheKey = lookupExpr.getA().key + "=" + lookupExpr.getA().value + - " AND " + lookupExpr.getB().key + "=" + lookupExpr.getB().value; + " OR " + lookupExpr.getB().key + "=" + lookupExpr.getB().value; EvaluationTrace trace = - new EvaluationTrace("Logical Combination Lookup:|" + cacheKey); + new EvaluationTrace("Logical Combination Lookup |" + cacheKey); storage.getIDsForValues(new String[] {lookupExpr.getA().key}, new String[] {(String)lookupExpr.getA().value}, ids); storage.getIDsForValues(new String[] {lookupExpr.getB().key}, new String[] {(String)lookupExpr.getB().value}, ids); - trace.setOutcome("Results: " + ids.size()); + trace.setOutcome("Matches: " + ids.size()); queryContext.reportTrace(trace); return ids; From 6d4cebbb428efa15365c4ccaa497dcde96fa7f64 Mon Sep 17 00:00:00 2001 From: Clayton Sims Date: Tue, 26 May 2020 17:46:33 -0400 Subject: [PATCH 3/4] docstring --- .../query/handlers/LogicalValueIndexHandler.java | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/commcare/cases/query/handlers/LogicalValueIndexHandler.java b/src/main/java/org/commcare/cases/query/handlers/LogicalValueIndexHandler.java index dc2a0ffde3..485c8c8d9a 100644 --- a/src/main/java/org/commcare/cases/query/handlers/LogicalValueIndexHandler.java +++ b/src/main/java/org/commcare/cases/query/handlers/LogicalValueIndexHandler.java @@ -31,8 +31,15 @@ import kotlin.collections.IndexedValue; /** - * Performs a "selected in" lookup operation from an appropriately backed storage table - * against a + * Optimizes predicates which are single logical operations that combine indexed values. + * + * Essentially enables one to query + * + * [index = a OR index = b] + * + * with a query optimization plan of + * + * O([index=a]) + O([index=b]) * * Created by ctsims on 1/31/2017. */ From 75730b1778d7125e23d7fee76c0b6afdaf2cd61d Mon Sep 17 00:00:00 2001 From: Clayton Sims Date: Tue, 26 May 2020 17:46:40 -0400 Subject: [PATCH 4/4] docstring --- .../commcare/cases/query/handlers/LogicalValueIndexHandler.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/org/commcare/cases/query/handlers/LogicalValueIndexHandler.java b/src/main/java/org/commcare/cases/query/handlers/LogicalValueIndexHandler.java index 485c8c8d9a..8fe42189f6 100644 --- a/src/main/java/org/commcare/cases/query/handlers/LogicalValueIndexHandler.java +++ b/src/main/java/org/commcare/cases/query/handlers/LogicalValueIndexHandler.java @@ -41,7 +41,7 @@ * * O([index=a]) + O([index=b]) * - * Created by ctsims on 1/31/2017. + * Created by ctsims on 5/26/2020. */ public class LogicalValueIndexHandler implements QueryHandler {