gsmiller commented on a change in pull request #138:
URL: https://github.com/apache/lucene/pull/138#discussion_r641848546
##########
File path: lucene/facet/src/java/org/apache/lucene/facet/DrillDownQuery.java
##########
@@ -170,16 +185,36 @@ private BooleanQuery getBooleanQuery() {
return bq.build();
}
- Query getBaseQuery() {
+ /**
+ * Returns the internal baseQuery of the DrillDownQuery
+ *
+ * @return The baseQuery used on initialization of DrillDownQuery
+ */
+ public Query getBaseQuery() {
return baseQuery;
}
- Query[] getDrillDownQueries() {
- Query[] dimQueries = new Query[this.dimQueries.size()];
- for (int i = 0; i < dimQueries.length; ++i) {
- dimQueries[i] = this.dimQueries.get(i).build();
+ /**
+ * Returns the dimension queries added either via {@link #add(String,
Query)} or {@link
+ * #add(String, String...)}
+ *
+ * @return The array of dimQueries
+ */
+ public Query[] getDrillDownQueries() {
+ if (dirtyDimQueryIndex.isEmpty()) {
+ // returns previously built dimQueries
+ Query[] builtDimQueriesCopy = new Query[builtDimQueries.size()];
+ return builtDimQueries.toArray(builtDimQueriesCopy);
+ }
+ for (int i = 0; i < this.dimQueries.size(); ++i) {
+ if (dirtyDimQueryIndex.contains(i)) {
+ builtDimQueries.set(i, this.dimQueries.get(i).build());
+ dirtyDimQueryIndex.remove(i);
+ }
}
- return dimQueries;
+ assert dirtyDimQueryIndex.isEmpty();
+ // by this time builtDimQueries has all the built queries and
dirtyDimQueryIndex is empty
+ return getDrillDownQueries();
Review comment:
What about something like:
```
public Query[] getDrillDownQueries() {
for (Integer dirtyIndex : dirtyDimQueryIndex) {
builtDimQueries.set(dirtyIndex, dimQueries.get(dirtyIndex).build());
}
dirtyDimQueryIndex.clear();
return builtDimQueries.toArray(new Query[0]);
}
```
##########
File path: lucene/facet/src/java/org/apache/lucene/facet/DrillDownQuery.java
##########
@@ -170,16 +185,36 @@ private BooleanQuery getBooleanQuery() {
return bq.build();
}
- Query getBaseQuery() {
+ /**
+ * Returns the internal baseQuery of the DrillDownQuery
+ *
+ * @return The baseQuery used on initialization of DrillDownQuery
+ */
+ public Query getBaseQuery() {
return baseQuery;
}
- Query[] getDrillDownQueries() {
- Query[] dimQueries = new Query[this.dimQueries.size()];
- for (int i = 0; i < dimQueries.length; ++i) {
- dimQueries[i] = this.dimQueries.get(i).build();
+ /**
+ * Returns the dimension queries added either via {@link #add(String,
Query)} or {@link
+ * #add(String, String...)}
+ *
+ * @return The array of dimQueries
+ */
+ public Query[] getDrillDownQueries() {
Review comment:
Let's take advantage of the built query caching in `getBooleanQuery` as
well! (Sorry for missing this earlier). We should be able to replace the
explicit building of the drill down queries with something like:
```
for (Query query : getDrillDownQueries()) {
bq.add(query, Occur.FILTER);
}
```
##########
File path: lucene/facet/src/java/org/apache/lucene/facet/DrillDownQuery.java
##########
@@ -53,6 +55,8 @@ public static Term term(String field, String dim, String...
path) {
private final Query baseQuery;
private final List<BooleanQuery.Builder> dimQueries = new ArrayList<>();
private final Map<String, Integer> drillDownDims = new LinkedHashMap<>();
+ private List<Query> builtDimQueries = new ArrayList<>();
+ private Set<Integer> dirtyDimQueryIndex = new HashSet<>();
Review comment:
Can you make these final please?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
