dsmiley commented on a change in pull request #757:
URL: https://github.com/apache/solr/pull/757#discussion_r836876499
##########
File path: solr/core/src/java/org/apache/solr/search/SolrDocumentFetcher.java
##########
@@ -219,6 +221,28 @@ public boolean isLazyFieldLoadingEnabled() {
}
}
+ /** Returns a collection of the names of all indexed fields which the index
reader knows about. */
+ public Collection<String> getIndexedFieldNames() {
+ synchronized (this) {
+ if (indexedFieldNames == null) {
+ indexedFieldNames = new LinkedList<>();
+ for (FieldInfo fieldInfo : searcher.getFieldInfos()) {
+ final String fieldName = fieldInfo.name;
+ try {
+ SchemaField field = searcher.getSchema().getField(fieldName);
Review comment:
This isn't necessary; can't we look at FieldInfo for this without
consulting Solr's schema?
##########
File path: solr/core/src/java/org/apache/solr/search/SolrDocumentFetcher.java
##########
@@ -219,6 +221,28 @@ public boolean isLazyFieldLoadingEnabled() {
}
}
+ /** Returns a collection of the names of all indexed fields which the index
reader knows about. */
+ public Collection<String> getIndexedFieldNames() {
+ synchronized (this) {
+ if (indexedFieldNames == null) {
+ indexedFieldNames = new LinkedList<>();
Review comment:
Let's use an ArrayList instead
##########
File path:
solr/core/src/java/org/apache/solr/highlight/UnifiedSolrHighlighter.java
##########
@@ -431,14 +435,33 @@ protected Locale parseLocale(String language, String
country, String variant) {
@Override
protected Predicate<String> getFieldMatcher(String field) {
- // TODO define hl.queryFieldPattern as a more advanced alternative to
hl.requireFieldMatch.
// note that the UH at Lucene level default to effectively "true"
if (params.getFieldBool(field, HighlightParams.FIELD_MATCH, false)) {
return field::equals; // requireFieldMatch
- } else {
- return NOT_REQUIRED_FIELD_MATCH_PREDICATE;
}
+
+ String[] queryFieldPattern =
+ params.getFieldParams(field, HighlightParams.QUERY_FIELD_PATTERN);
+ if (queryFieldPattern != null && queryFieldPattern.length != 0) {
+
+ Collection<String> indexedFields =
solrIndexSearcher.getDocFetcher().getIndexedFieldNames();
+
+ Predicate<String> predicate = null;
+ for (String f : expandWildcardsInHighlightFields(indexedFields,
queryFieldPattern)) {
+ if (predicate == null) {
+ predicate = f::equals;
+ } else {
+ predicate = predicate.or(f::equals);
+ }
Review comment:
this results in a O(N) predicate. I think it would be more efficient to
simply build up a HashSet and have a simple predicate to check it -- O(1).
##########
File path: solr/solr-ref-guide/modules/query-guide/pages/highlighting.adoc
##########
@@ -116,6 +116,17 @@ If set to `true`, only query terms aligning with the field
being highlighted wil
If the query references fields different from the field being highlighted and
they have different text analysis, the query may not highlight query terms it
should have and vice versa.
The analysis used is that of the field being highlighted (`hl.fl`), not the
query fields.
+`hl.queryFieldPattern`::
++
+[%autowidth,frame=none]
+|===
+|Optional |Default: `none`
+|===
++
+Similar to `hl.requireFieldMatch` but allows for multiple fields to match e.g.
`q=fieldA:one OR fieldB:two OR fieldC:three` `hl.fl=fieldA`
`hl.queryFieldPattern=fieldA,fieldB`
Review comment:
Does this supersede hl.requireFieldMatch when set?
##########
File path:
solr/core/src/java/org/apache/solr/highlight/UnifiedSolrHighlighter.java
##########
@@ -431,14 +435,33 @@ protected Locale parseLocale(String language, String
country, String variant) {
@Override
protected Predicate<String> getFieldMatcher(String field) {
- // TODO define hl.queryFieldPattern as a more advanced alternative to
hl.requireFieldMatch.
// note that the UH at Lucene level default to effectively "true"
if (params.getFieldBool(field, HighlightParams.FIELD_MATCH, false)) {
return field::equals; // requireFieldMatch
- } else {
- return NOT_REQUIRED_FIELD_MATCH_PREDICATE;
}
+
+ String[] queryFieldPattern =
+ params.getFieldParams(field, HighlightParams.QUERY_FIELD_PATTERN);
+ if (queryFieldPattern != null && queryFieldPattern.length != 0) {
+
+ Collection<String> indexedFields =
solrIndexSearcher.getDocFetcher().getIndexedFieldNames();
+
+ Predicate<String> predicate = null;
+ for (String f : expandWildcardsInHighlightFields(indexedFields,
queryFieldPattern)) {
Review comment:
You created a expandWildcardsInHighlightFields method whose first
argument has a name storedHighlightFieldNames. But at this line you pass
indexed fields. This is kind of suspicious.
--
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.
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For additional commands, e-mail: issues-h...@solr.apache.org
