From 7955f2b0d9ac5360e9efa24ebe33b165e9e22b3b Mon Sep 17 00:00:00 2001 From: Tim Owen Date: Fri, 4 Oct 2024 15:49:18 +0100 Subject: [PATCH 1/3] SOLR-17477: Support custom aggregates in plugin code --- .../solr/search/facet/DocValuesAcc.java | 2 +- .../apache/solr/search/facet/FacetModule.java | 11 ++--- .../apache/solr/search/facet/FacetParser.java | 40 ++++++++++++------- .../search/facet/UniqueMultiDvSlotAcc.java | 2 +- .../facet/UniqueMultivaluedSlotAcc.java | 2 +- .../facet/UniqueSinglevaluedSlotAcc.java | 2 +- .../solr/search/facet/UniqueSlotAcc.java | 2 +- .../search/function/AggValueSourceTest.java | 33 +++++++-------- 8 files changed, 53 insertions(+), 41 deletions(-) diff --git a/solr/core/src/java/org/apache/solr/search/facet/DocValuesAcc.java b/solr/core/src/java/org/apache/solr/search/facet/DocValuesAcc.java index fddfcf92118..5b89915b7eb 100644 --- a/solr/core/src/java/org/apache/solr/search/facet/DocValuesAcc.java +++ b/solr/core/src/java/org/apache/solr/search/facet/DocValuesAcc.java @@ -37,7 +37,7 @@ * org.apache.lucene.index.DocValues} */ public abstract class DocValuesAcc extends SlotAcc { - SchemaField sf; + protected SchemaField sf; public DocValuesAcc(FacetContext fcontext, SchemaField sf) throws IOException { super(fcontext); diff --git a/solr/core/src/java/org/apache/solr/search/facet/FacetModule.java b/solr/core/src/java/org/apache/solr/search/facet/FacetModule.java index 5a84339a775..b4156e00326 100644 --- a/solr/core/src/java/org/apache/solr/search/facet/FacetModule.java +++ b/solr/core/src/java/org/apache/solr/search/facet/FacetModule.java @@ -370,7 +370,7 @@ static class FacetComponentState { } // base class for facet functions that can be used in a sort - abstract static class FacetSortableMerger extends FacetMerger { + public abstract static class FacetSortableMerger extends FacetMerger { public void prepareSort() {} @Override @@ -386,7 +386,7 @@ public void finish(Context mcontext) { public abstract int compareTo(FacetSortableMerger other, FacetRequest.SortDirection direction); } - abstract static class FacetDoubleMerger extends FacetSortableMerger { + public abstract static class FacetDoubleMerger extends FacetSortableMerger { @Override public abstract void merge(Object facetResult, Context mcontext); @@ -422,7 +422,7 @@ public static int compare(double a, double b, FacetRequest.SortDirection directi } } - static class FacetLongMerger extends FacetSortableMerger { + public static class FacetLongMerger extends FacetSortableMerger { long val; @Override @@ -442,7 +442,8 @@ public int compareTo(FacetSortableMerger other, FacetRequest.SortDirection direc } // base class for facets that create buckets (and can hence have sub-facets) - abstract static class FacetBucketMerger extends FacetMerger { + public abstract static class FacetBucketMerger + extends FacetMerger { FacetRequestT freq; public FacetBucketMerger(FacetRequestT freq) { @@ -483,7 +484,7 @@ FacetMerger createFacetMerger(String key, Object val) { } } - static class FacetQueryMerger extends FacetBucketMerger { + public static class FacetQueryMerger extends FacetBucketMerger { FacetBucket bucket; public FacetQueryMerger(FacetQuery freq) { diff --git a/solr/core/src/java/org/apache/solr/search/facet/FacetParser.java b/solr/core/src/java/org/apache/solr/search/facet/FacetParser.java index 6ef55cb4be1..854f353bcd2 100644 --- a/solr/core/src/java/org/apache/solr/search/facet/FacetParser.java +++ b/solr/core/src/java/org/apache/solr/search/facet/FacetParser.java @@ -22,6 +22,7 @@ import java.util.List; import java.util.Map; import java.util.Optional; +import java.util.concurrent.ConcurrentHashMap; import java.util.stream.Collectors; import org.apache.solr.common.SolrException; import org.apache.solr.common.params.SolrParams; @@ -33,7 +34,7 @@ import org.apache.solr.search.QParser; import org.apache.solr.search.SyntaxError; -abstract class FacetParser { +public abstract class FacetParser { protected T facet; protected FacetParser parent; protected String key; @@ -138,21 +139,30 @@ public Object parseFacetOrStat(String key, Object o) throws SyntaxError { return parseFacetOrStat(key, type, args); } + public interface ParseHandler { + Object doParse(FacetParser parent, String key, Object args) throws SyntaxError; + } + + private static final Map REGISTERED_TYPES = new ConcurrentHashMap<>(); + + static { + ParseHandler fieldParser = (p, k, a) -> new FacetFieldParser(p, k).parse(a); + REGISTERED_TYPES.put("field", fieldParser); + REGISTERED_TYPES.put("terms", fieldParser); + REGISTERED_TYPES.put("query", (p, k, a) -> new FacetQueryParser(p, k).parse(a)); + REGISTERED_TYPES.put("range", (p, k, a) -> new FacetRangeParser(p, k).parse(a)); + REGISTERED_TYPES.put("heatmap", (p, k, a) -> new FacetHeatmap.Parser(p, k).parse(a)); + REGISTERED_TYPES.put("func", (p, k, a) -> p.parseStat(k, a)); + } + + public static void registerParseHandler(String type, ParseHandler parseHandler) { + REGISTERED_TYPES.put(type, parseHandler); + } + public Object parseFacetOrStat(String key, String type, Object args) throws SyntaxError { - // TODO: a place to register all these facet types? - - switch (type) { - case "field": - case "terms": - return new FacetFieldParser(this, key).parse(args); - case "query": - return new FacetQueryParser(this, key).parse(args); - case "range": - return new FacetRangeParser(this, key).parse(args); - case "heatmap": - return new FacetHeatmap.Parser(this, key).parse(args); - case "func": - return parseStat(key, args); + ParseHandler parseHandler = REGISTERED_TYPES.get(type); + if (parseHandler != null) { + return parseHandler.doParse(this, key, args); } throw err("Unknown facet or stat. key=" + key + " type=" + type + " args=" + args); diff --git a/solr/core/src/java/org/apache/solr/search/facet/UniqueMultiDvSlotAcc.java b/solr/core/src/java/org/apache/solr/search/facet/UniqueMultiDvSlotAcc.java index 4670bc54b31..b0be52b90ae 100644 --- a/solr/core/src/java/org/apache/solr/search/facet/UniqueMultiDvSlotAcc.java +++ b/solr/core/src/java/org/apache/solr/search/facet/UniqueMultiDvSlotAcc.java @@ -28,7 +28,7 @@ import org.apache.lucene.util.LongValues; import org.apache.solr.schema.SchemaField; -class UniqueMultiDvSlotAcc extends UniqueSlotAcc { +public class UniqueMultiDvSlotAcc extends UniqueSlotAcc { SortedSetDocValues topLevel; SortedSetDocValues[] subDvs; OrdinalMap ordMap; diff --git a/solr/core/src/java/org/apache/solr/search/facet/UniqueMultivaluedSlotAcc.java b/solr/core/src/java/org/apache/solr/search/facet/UniqueMultivaluedSlotAcc.java index 6a013147cbc..6839102de0c 100644 --- a/solr/core/src/java/org/apache/solr/search/facet/UniqueMultivaluedSlotAcc.java +++ b/solr/core/src/java/org/apache/solr/search/facet/UniqueMultivaluedSlotAcc.java @@ -24,7 +24,7 @@ import org.apache.solr.schema.SchemaField; import org.apache.solr.search.SolrIndexSearcher; -class UniqueMultivaluedSlotAcc extends UniqueSlotAcc implements UnInvertedField.Callback { +public class UniqueMultivaluedSlotAcc extends UniqueSlotAcc implements UnInvertedField.Callback { private UnInvertedField uif; private UnInvertedField.DocToTerm docToTerm; diff --git a/solr/core/src/java/org/apache/solr/search/facet/UniqueSinglevaluedSlotAcc.java b/solr/core/src/java/org/apache/solr/search/facet/UniqueSinglevaluedSlotAcc.java index dd51050e4ca..c94f162338e 100644 --- a/solr/core/src/java/org/apache/solr/search/facet/UniqueSinglevaluedSlotAcc.java +++ b/solr/core/src/java/org/apache/solr/search/facet/UniqueSinglevaluedSlotAcc.java @@ -28,7 +28,7 @@ import org.apache.lucene.util.LongValues; import org.apache.solr.schema.SchemaField; -class UniqueSinglevaluedSlotAcc extends UniqueSlotAcc { +public class UniqueSinglevaluedSlotAcc extends UniqueSlotAcc { SortedDocValues topLevel; SortedDocValues[] subDvs; OrdinalMap ordMap; diff --git a/solr/core/src/java/org/apache/solr/search/facet/UniqueSlotAcc.java b/solr/core/src/java/org/apache/solr/search/facet/UniqueSlotAcc.java index 84a1ea835f1..ffeebe9f3f2 100644 --- a/solr/core/src/java/org/apache/solr/search/facet/UniqueSlotAcc.java +++ b/solr/core/src/java/org/apache/solr/search/facet/UniqueSlotAcc.java @@ -27,7 +27,7 @@ import org.apache.solr.schema.SchemaField; import org.apache.solr.util.hll.HLL; -abstract class UniqueSlotAcc extends SlotAcc { +public abstract class UniqueSlotAcc extends SlotAcc { HLLAgg.HLLFactory factory; SchemaField field; FixedBitSet[] arr; diff --git a/solr/core/src/test/org/apache/solr/search/function/AggValueSourceTest.java b/solr/core/src/test/org/apache/solr/search/function/AggValueSourceTest.java index 5946d4dc955..19926f00ae7 100644 --- a/solr/core/src/test/org/apache/solr/search/function/AggValueSourceTest.java +++ b/solr/core/src/test/org/apache/solr/search/function/AggValueSourceTest.java @@ -23,6 +23,7 @@ import org.apache.solr.SolrTestCase; import org.apache.solr.search.facet.FacetContext; import org.apache.solr.search.facet.FacetMerger; +import org.apache.solr.search.facet.FacetModule; import org.apache.solr.search.facet.SimpleAggValueSource; import org.apache.solr.search.facet.SlotAcc; import org.junit.Test; @@ -76,22 +77,22 @@ public Object getValue(int slot) { @Override public FacetMerger createFacetMerger(Object prototype) { - return new FacetMerger() { - double total = 0.0D; - - @Override - public void merge(Object facetResult, Context mcontext) { - total += (Double) facetResult; - } - - @Override - public void finish(Context mcontext) {} - - @Override - public Object getMergedResult() { - return total; - } - }; + // check these inner classes can be referenced by name + FacetModule.FacetSortableMerger merger = + new FacetModule.FacetDoubleMerger() { + double total = 0.0D; + + @Override + public void merge(Object facetResult, Context mcontext) { + total += (Double) facetResult; + } + + @Override + public double getDouble() { + return total; + } + }; + return merger; } @Override From 90a251c020782c342e7a0a79526769dc89ad4144 Mon Sep 17 00:00:00 2001 From: Tim Owen Date: Tue, 15 Oct 2024 15:12:54 +0100 Subject: [PATCH 2/3] SOLR-17477: Set private/protected/final on public classes in facet package --- .../src/java/org/apache/solr/search/facet/DocValuesAcc.java | 2 +- .../src/java/org/apache/solr/search/facet/FacetModule.java | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/solr/core/src/java/org/apache/solr/search/facet/DocValuesAcc.java b/solr/core/src/java/org/apache/solr/search/facet/DocValuesAcc.java index 5b89915b7eb..25c108ebfb6 100644 --- a/solr/core/src/java/org/apache/solr/search/facet/DocValuesAcc.java +++ b/solr/core/src/java/org/apache/solr/search/facet/DocValuesAcc.java @@ -37,7 +37,7 @@ * org.apache.lucene.index.DocValues} */ public abstract class DocValuesAcc extends SlotAcc { - protected SchemaField sf; + protected final SchemaField sf; public DocValuesAcc(FacetContext fcontext, SchemaField sf) throws IOException { super(fcontext); diff --git a/solr/core/src/java/org/apache/solr/search/facet/FacetModule.java b/solr/core/src/java/org/apache/solr/search/facet/FacetModule.java index b4156e00326..f9e5e177418 100644 --- a/solr/core/src/java/org/apache/solr/search/facet/FacetModule.java +++ b/solr/core/src/java/org/apache/solr/search/facet/FacetModule.java @@ -423,7 +423,7 @@ public static int compare(double a, double b, FacetRequest.SortDirection directi } public static class FacetLongMerger extends FacetSortableMerger { - long val; + private long val; @Override public void merge(Object facetResult, Context mcontext) { @@ -444,7 +444,7 @@ public int compareTo(FacetSortableMerger other, FacetRequest.SortDirection direc // base class for facets that create buckets (and can hence have sub-facets) public abstract static class FacetBucketMerger extends FacetMerger { - FacetRequestT freq; + protected final FacetRequestT freq; public FacetBucketMerger(FacetRequestT freq) { this.freq = freq; @@ -485,7 +485,7 @@ FacetMerger createFacetMerger(String key, Object val) { } public static class FacetQueryMerger extends FacetBucketMerger { - FacetBucket bucket; + private FacetBucket bucket; public FacetQueryMerger(FacetQuery freq) { super(freq); From 0502e601dd978882f801d5385cabb94049ffdea2 Mon Sep 17 00:00:00 2001 From: Tim Owen Date: Fri, 1 Nov 2024 12:03:08 +0000 Subject: [PATCH 3/3] SOLR-17477: Revert registration of agg parser changes, not needed --- .../apache/solr/search/facet/FacetParser.java | 38 +++++++------------ 1 file changed, 14 insertions(+), 24 deletions(-) diff --git a/solr/core/src/java/org/apache/solr/search/facet/FacetParser.java b/solr/core/src/java/org/apache/solr/search/facet/FacetParser.java index 854f353bcd2..079b049759a 100644 --- a/solr/core/src/java/org/apache/solr/search/facet/FacetParser.java +++ b/solr/core/src/java/org/apache/solr/search/facet/FacetParser.java @@ -22,7 +22,6 @@ import java.util.List; import java.util.Map; import java.util.Optional; -import java.util.concurrent.ConcurrentHashMap; import java.util.stream.Collectors; import org.apache.solr.common.SolrException; import org.apache.solr.common.params.SolrParams; @@ -139,30 +138,21 @@ public Object parseFacetOrStat(String key, Object o) throws SyntaxError { return parseFacetOrStat(key, type, args); } - public interface ParseHandler { - Object doParse(FacetParser parent, String key, Object args) throws SyntaxError; - } - - private static final Map REGISTERED_TYPES = new ConcurrentHashMap<>(); - - static { - ParseHandler fieldParser = (p, k, a) -> new FacetFieldParser(p, k).parse(a); - REGISTERED_TYPES.put("field", fieldParser); - REGISTERED_TYPES.put("terms", fieldParser); - REGISTERED_TYPES.put("query", (p, k, a) -> new FacetQueryParser(p, k).parse(a)); - REGISTERED_TYPES.put("range", (p, k, a) -> new FacetRangeParser(p, k).parse(a)); - REGISTERED_TYPES.put("heatmap", (p, k, a) -> new FacetHeatmap.Parser(p, k).parse(a)); - REGISTERED_TYPES.put("func", (p, k, a) -> p.parseStat(k, a)); - } - - public static void registerParseHandler(String type, ParseHandler parseHandler) { - REGISTERED_TYPES.put(type, parseHandler); - } - public Object parseFacetOrStat(String key, String type, Object args) throws SyntaxError { - ParseHandler parseHandler = REGISTERED_TYPES.get(type); - if (parseHandler != null) { - return parseHandler.doParse(this, key, args); + // TODO: a place to register all these facet types? + + switch (type) { + case "field": + case "terms": + return new FacetFieldParser(this, key).parse(args); + case "query": + return new FacetQueryParser(this, key).parse(args); + case "range": + return new FacetRangeParser(this, key).parse(args); + case "heatmap": + return new FacetHeatmap.Parser(this, key).parse(args); + case "func": + return parseStat(key, args); } throw err("Unknown facet or stat. key=" + key + " type=" + type + " args=" + args);