This is an automated email from the ASF dual-hosted git repository. dsmiley pushed a commit to branch branch_10x in repository https://gitbox.apache.org/repos/asf/solr.git
commit 0f7afb11798b2f605d1d3ac928ff0ccb8cc9dfac Author: David Smiley <[email protected]> AuthorDate: Mon Oct 26 17:44:27 2026 -0400 SOLR-11364: fl with globs: return explicit fields, load transformer sources (#5188) When fl has globs (e.g. fl=*_s,myfield), an explicitly named docValues field with useDocValuesAsStored=false is now returned, and fields read by document transformers like [json f=...] are now loaded. Fields that are only alias or transformer names are no longer needlessly loaded. - Export: an explicitly named field that cannot be exported now errors even next to a glob or *, instead of being silently skipped. - ReturnFields gains loadsField and loadsFieldExplicitly, and wantsField, wantsAllFields and wantsScore are renamed returnsField, returnsAllFields and returnsScore (the old names are deprecated). - SolrDocumentFetcher.getNonStoredDVs(boolean) is split into getNonStoredDVs() and getNonStoredDVsUsedAsStored(). Co-authored-by: Claude Opus 5.5 (1M context) <[email protected]> (cherry picked from commit 311dde8a9798f28ee4e6f9d579f488d69806c829) --- changelog/unreleased/SOLR-11364-fl-glob-fixes.yml | 12 + .../apache/solr/handler/MoreLikeThisHandler.java | 2 +- .../solr/handler/component/ExpandComponent.java | 2 +- .../handler/component/MoreLikeThisComponent.java | 2 +- .../solr/handler/component/QueryComponent.java | 8 +- .../handler/component/RealTimeGetComponent.java | 2 +- .../handler/component/TermVectorComponent.java | 2 +- .../apache/solr/handler/export/ExportWriter.java | 23 +- .../apache/solr/response/CSVResponseWriter.java | 2 +- .../org/apache/solr/response/DocsStreamer.java | 13 +- .../solr/response/GeoJSONResponseWriter.java | 2 +- .../java/org/apache/solr/response/JSONWriter.java | 2 +- .../solr/response/JavaBinResponseWriter.java | 4 +- .../org/apache/solr/response/ResultContext.java | 2 +- .../solr/response/TabularResponseWriter.java | 2 +- .../java/org/apache/solr/response/XMLWriter.java | 2 +- .../transform/SubQueryAugmenterFactory.java | 2 +- .../java/org/apache/solr/search/ReturnFields.java | 64 +++- .../apache/solr/search/SolrDocumentFetcher.java | 34 +- .../org/apache/solr/search/SolrReturnFields.java | 186 ++++++----- .../StoredFieldsShardRequestFactory.java | 2 +- .../solr/handler/export/TestExportWriter.java | 27 ++ .../org/apache/solr/search/ReturnFieldsTest.java | 356 ++++++++++++--------- .../apache/solr/search/TestPseudoReturnFields.java | 33 ++ 24 files changed, 463 insertions(+), 323 deletions(-) diff --git a/changelog/unreleased/SOLR-11364-fl-glob-fixes.yml b/changelog/unreleased/SOLR-11364-fl-glob-fixes.yml new file mode 100644 index 00000000000..9f3cb28ea0d --- /dev/null +++ b/changelog/unreleased/SOLR-11364-fl-glob-fixes.yml @@ -0,0 +1,12 @@ +title: > + When fl has globs (e.g. fl=*_s,myfield), an explicitly named docValues field with useDocValuesAsStored=false + is now returned, and fields read by document transformers like [json f=...] are now loaded. + Fields that are only alias or transformer names are no longer needlessly loaded. +type: fixed +authors: + - name: David Smiley +links: + - name: SOLR-11364 + url: https://issues.apache.org/jira/browse/SOLR-11364 + - name: PR#5188 + url: https://github.com/apache/solr/pull/5188 diff --git a/solr/core/src/java/org/apache/solr/handler/MoreLikeThisHandler.java b/solr/core/src/java/org/apache/solr/handler/MoreLikeThisHandler.java index d7ac47c14b5..73ac96b19f9 100644 --- a/solr/core/src/java/org/apache/solr/handler/MoreLikeThisHandler.java +++ b/solr/core/src/java/org/apache/solr/handler/MoreLikeThisHandler.java @@ -107,7 +107,7 @@ public class MoreLikeThisHandler extends RequestHandlerBase { ReturnFields returnFields = new SolrReturnFields(req); rsp.setReturnFields(returnFields); int flags = 0; - if (returnFields.wantsScore()) { + if (returnFields.returnsScore()) { flags |= SolrIndexSearcher.GET_SCORES; } diff --git a/solr/core/src/java/org/apache/solr/handler/component/ExpandComponent.java b/solr/core/src/java/org/apache/solr/handler/component/ExpandComponent.java index d10f810a0d5..ab2d29e437d 100644 --- a/solr/core/src/java/org/apache/solr/handler/component/ExpandComponent.java +++ b/solr/core/src/java/org/apache/solr/handler/component/ExpandComponent.java @@ -767,7 +767,7 @@ public class ExpandComponent extends SearchComponent implements PluginInfoInitia TopDocs topDocs = topDocsCollector.topDocs(); ScoreDoc[] scoreDocs = topDocs.scoreDocs; if (scoreDocs.length > 0) { - if (returnFields.wantsScore() && sort != null) { + if (returnFields.returnsScore() && sort != null) { TopFieldCollector.populateScores(scoreDocs, searcher, query); } int[] docs = new int[scoreDocs.length]; diff --git a/solr/core/src/java/org/apache/solr/handler/component/MoreLikeThisComponent.java b/solr/core/src/java/org/apache/solr/handler/component/MoreLikeThisComponent.java index d1dae9dd20c..0103d7de863 100644 --- a/solr/core/src/java/org/apache/solr/handler/component/MoreLikeThisComponent.java +++ b/solr/core/src/java/org/apache/solr/handler/component/MoreLikeThisComponent.java @@ -81,7 +81,7 @@ public class MoreLikeThisComponent extends SearchComponent { ReturnFields returnFields = new SolrReturnFields(rb.req); int flags = 0; - if (returnFields.wantsScore()) { + if (returnFields.returnsScore()) { flags |= SolrIndexSearcher.GET_SCORES; } diff --git a/solr/core/src/java/org/apache/solr/handler/component/QueryComponent.java b/solr/core/src/java/org/apache/solr/handler/component/QueryComponent.java index 9d51160bba2..283df3e26ed 100644 --- a/solr/core/src/java/org/apache/solr/handler/component/QueryComponent.java +++ b/solr/core/src/java/org/apache/solr/handler/component/QueryComponent.java @@ -170,7 +170,7 @@ public class QueryComponent extends SearchComponent { ReturnFields returnFields = new SolrReturnFields(req); rsp.setReturnFields(returnFields); int flags = 0; - if (returnFields.wantsScore()) { + if (returnFields.returnsScore()) { flags |= SolrIndexSearcher.GET_SCORES; } rb.setFieldFlags(flags); @@ -857,7 +857,7 @@ public class QueryComponent extends SearchComponent { if (fls != null && fls.length > 0 && (fls.length != 1 || !fls[0].isEmpty())) { // If the outer request contains actual FL's use them... sreq.params.set(CommonParams.FL, fls); - if (!fields.wantsField(keyFieldName)) { + if (!fields.returnsField(keyFieldName)) { additionalAdded = addFL(additionalFL, keyFieldName, additionalAdded); } } else { @@ -1440,7 +1440,7 @@ public class QueryComponent extends SearchComponent { // we already have the field sort values sreq.params.remove(ResponseBuilder.FIELD_SORT_VALUES); - if (!rb.rsp.getReturnFields().wantsField(uniqueField.getName())) { + if (!rb.rsp.getReturnFields().returnsField(uniqueField.getName())) { sreq.params.add(CommonParams.FL, uniqueField.getName()); } @@ -1464,7 +1464,7 @@ public class QueryComponent extends SearchComponent { if ((sreq.purpose & ShardRequest.PURPOSE_GET_FIELDS) != 0) { final String uniqueKey = rb.req.getSchema().getUniqueKeyField().getName(); String keyFieldName = uniqueKey; - boolean removeKeyField = !rb.rsp.getReturnFields().wantsField(keyFieldName); + boolean removeKeyField = !rb.rsp.getReturnFields().returnsField(keyFieldName); boolean returnRawScore = rb.rsp.getReturnFields().getExplicitlyRequestedFieldNames() != null && rb.rsp diff --git a/solr/core/src/java/org/apache/solr/handler/component/RealTimeGetComponent.java b/solr/core/src/java/org/apache/solr/handler/component/RealTimeGetComponent.java index d221132c835..c641c3ad50d 100644 --- a/solr/core/src/java/org/apache/solr/handler/component/RealTimeGetComponent.java +++ b/solr/core/src/java/org/apache/solr/handler/component/RealTimeGetComponent.java @@ -354,7 +354,7 @@ public class RealTimeGetComponent extends SearchComponent { reuseDvIters = new DocValuesIteratorCache(searcherInfo.getSearcher()); } docFetcher.decorateDocValueFields( - doc, docid, docFetcher.getNonStoredDVs(true), reuseDvIters); + doc, docid, docFetcher.getNonStoredDVsUsedAsStored(), reuseDvIters); if (null != transformer) { if (null == resultContext) { // either first pass, or we've re-opened searcher - either way now we setContext diff --git a/solr/core/src/java/org/apache/solr/handler/component/TermVectorComponent.java b/solr/core/src/java/org/apache/solr/handler/component/TermVectorComponent.java index 7252adc17dd..c108767b170 100644 --- a/solr/core/src/java/org/apache/solr/handler/component/TermVectorComponent.java +++ b/solr/core/src/java/org/apache/solr/handler/component/TermVectorComponent.java @@ -104,7 +104,7 @@ public class TermVectorComponent extends SearchComponent { // no tv.fl, parse the main fl ReturnFields rf = new SolrReturnFields(params.getParams(CommonParams.FL), rb.req); - if (rf.wantsAllFields()) { + if (rf.returnsAllFields()) { return null; } diff --git a/solr/core/src/java/org/apache/solr/handler/export/ExportWriter.java b/solr/core/src/java/org/apache/solr/handler/export/ExportWriter.java index 7872a61be1f..3278f2372c2 100644 --- a/solr/core/src/java/org/apache/solr/handler/export/ExportWriter.java +++ b/solr/core/src/java/org/apache/solr/handler/export/ExportWriter.java @@ -26,7 +26,6 @@ import java.io.PrintWriter; import java.lang.invoke.MethodHandles; import java.nio.charset.StandardCharsets; import java.util.ArrayList; -import java.util.HashSet; import java.util.LinkedHashMap; import java.util.LinkedHashSet; import java.util.List; @@ -295,7 +294,7 @@ public class ExportWriter implements SolrCore.RawWriter, Closeable { return; } - if (returnFields.wantsScore()) { + if (returnFields.returnsScore()) { writeException( (new IOException(new SyntaxError("Scoring is not currently supported with xsort."))), writer, @@ -518,7 +517,7 @@ public class ExportWriter implements SolrCore.RawWriter, Closeable { } transformer.transform(doc, sortDoc.docBase + sortDoc.docId, DocIterationInfo.NONE); for (Map.Entry<String, Object> entry : doc) { - if (returnFields.wantsField(entry.getKey())) { + if (returnFields.returnsField(entry.getKey())) { ew.put(entry.getKey(), entry.getValue()); } } @@ -541,24 +540,12 @@ public class ExportWriter implements SolrCore.RawWriter, Closeable { DocValuesIteratorCache dvIterCache = new DocValuesIteratorCache(req.getSearcher(), false); boolean includeStoredFields = req.getParams().getBool(INCLUDE_STORED_FIELDS_PARAM, false); - // Also fetch fields only a transformer reads, e.g. the f= of [geo]; wantsField excludes them - Set<String> fieldsToFetch = - solrReturnFields.getLuceneFieldNames() != null - ? solrReturnFields.getLuceneFieldNames() - : Set.of(); - // Non-exportable fields named here error; ones matched only by a glob are skipped. - // Includes fieldsToFetch, since requested names hold an alias, not its source field. - Set<String> explicitlyNamedFields = new HashSet<>(fieldsToFetch); - if (solrReturnFields.getRequestedFieldNames() != null) { - explicitlyNamedFields.addAll(solrReturnFields.getRequestedFieldNames()); - } - List<FieldWriter> writers = new ArrayList<>(); Set<SchemaField> docValueFields = new LinkedHashSet<>(); Map<String, SchemaField> storedFields = new LinkedHashMap<>(); for (String field : req.getSearcher().getFieldNames()) { - if (!solrReturnFields.wantsField(field) && !fieldsToFetch.contains(field)) { + if (!solrReturnFields.loadsField(field)) { continue; } SchemaField schemaField = req.getSchema().getField(field); @@ -571,7 +558,7 @@ public class ExportWriter implements SolrCore.RawWriter, Closeable { // Field is stored-only (no usable DocValues) if (includeStoredFields) { storedFields.put(field, schemaField); - } else if (explicitlyNamedFields.contains(field)) { + } else if (solrReturnFields.loadsFieldExplicitly(field)) { // Explicitly requested field without DocValues and includeStoredFields=false throw new IOException( schemaField @@ -579,7 +566,7 @@ public class ExportWriter implements SolrCore.RawWriter, Closeable { + "Try setting includeStoredFields=true to retrieve this field from stored values."); } // Else: glob matched stored-only field without includeStoredFields - silently skip - } else if (explicitlyNamedFields.contains(field)) { + } else if (solrReturnFields.loadsFieldExplicitly(field)) { // Explicitly requested field that has neither DocValues nor stored if (fieldType instanceof SortableTextField && !schemaField.useDocValuesAsStored()) { throw new IOException( diff --git a/solr/core/src/java/org/apache/solr/response/CSVResponseWriter.java b/solr/core/src/java/org/apache/solr/response/CSVResponseWriter.java index b630b697d75..540908f3484 100644 --- a/solr/core/src/java/org/apache/solr/response/CSVResponseWriter.java +++ b/solr/core/src/java/org/apache/solr/response/CSVResponseWriter.java @@ -275,7 +275,7 @@ class CSVWriter extends TabularResponseWriter { CSVSharedBufPrinter csvPrinterMV = new CSVSharedBufPrinter(mvWriter, mvStrategy); for (String field : fields) { - if (!returnFields.wantsField(field)) { + if (!returnFields.returnsField(field)) { continue; } if (field.equals("score")) { diff --git a/solr/core/src/java/org/apache/solr/response/DocsStreamer.java b/solr/core/src/java/org/apache/solr/response/DocsStreamer.java index 7b33cf5800c..2da496d94bf 100644 --- a/solr/core/src/java/org/apache/solr/response/DocsStreamer.java +++ b/solr/core/src/java/org/apache/solr/response/DocsStreamer.java @@ -155,23 +155,20 @@ public class DocsStreamer implements Iterator<SolrDocument> { assert null != schema; assert null != fields; - // can't just use fields.wantsField(String) - // because that doesn't include extra fields needed by transformers - final Set<String> fieldNamesNeeded = fields.getLuceneFieldNames(); - JavaBinResponseWriter.MaskCharSeqSolrDocument masked = null; final SolrDocument out = ResultContext.READASBYTES.get() == null ? new SolrDocument() : (masked = new JavaBinResponseWriter.MaskCharSeqSolrDocument()); - // NOTE: it would be tempting to try and optimize this to loop over fieldNamesNeeded when it's - // smaller then the IndexableField[] in the Document -- but that's actually *less* effecient - // since Document.getFields(String) does a full (internal) iteration over the full + // NOTE: it would be tempting to try and optimize this to loop over the needed fields when + // they're smaller than the IndexableField[] in the Document -- but that's actually *less* + // efficient since Document.getFields(String) does a full (internal) iteration over the full // IndexableField[]. see SOLR-11891 for (IndexableField f : doc.getFields()) { final String fname = f.name(); - if (null == fieldNamesNeeded || fieldNamesNeeded.contains(fname)) { + // not returnsField, which excludes extra fields needed by transformers + if (fields.loadsField(fname)) { // Make sure multivalued fields are represented as lists Object existing = masked == null ? out.get(fname) : masked.getRaw(fname); if (existing == null) { diff --git a/solr/core/src/java/org/apache/solr/response/GeoJSONResponseWriter.java b/solr/core/src/java/org/apache/solr/response/GeoJSONResponseWriter.java index b88eaf924f6..0266c6cad61 100644 --- a/solr/core/src/java/org/apache/solr/response/GeoJSONResponseWriter.java +++ b/solr/core/src/java/org/apache/solr/response/GeoJSONResponseWriter.java @@ -148,7 +148,7 @@ class GeoJSONWriter extends JSONWriter { boolean first = true; for (String fname : doc.getFieldNames()) { - if (fname.equals(geofield) || ((returnFields != null && !returnFields.wantsField(fname)))) { + if (fname.equals(geofield) || ((returnFields != null && !returnFields.returnsField(fname)))) { continue; } writeMapSeparator(); diff --git a/solr/core/src/java/org/apache/solr/response/JSONWriter.java b/solr/core/src/java/org/apache/solr/response/JSONWriter.java index e9eb78c9a36..a51028a2447 100644 --- a/solr/core/src/java/org/apache/solr/response/JSONWriter.java +++ b/solr/core/src/java/org/apache/solr/response/JSONWriter.java @@ -94,7 +94,7 @@ public class JSONWriter extends TextResponseWriter implements JsonTextWriter { boolean first = true; for (String fname : doc.getFieldNames()) { - if (returnFields != null && !returnFields.wantsField(fname)) { + if (returnFields != null && !returnFields.returnsField(fname)) { continue; } diff --git a/solr/core/src/java/org/apache/solr/response/JavaBinResponseWriter.java b/solr/core/src/java/org/apache/solr/response/JavaBinResponseWriter.java index a11dd693d88..8985f167be1 100644 --- a/solr/core/src/java/org/apache/solr/response/JavaBinResponseWriter.java +++ b/solr/core/src/java/org/apache/solr/response/JavaBinResponseWriter.java @@ -119,12 +119,12 @@ public class JavaBinResponseWriter implements QueryResponseWriter { @Override public boolean isWritable(String name) { - return returnFields.wantsField(name); + return returnFields.returnsField(name); } @Override public boolean wantsAllFields() { - return returnFields.wantsAllFields(); + return returnFields.returnsAllFields(); } protected void writeResultsBody(ResultContext res, JavaBinCodec codec) throws IOException { diff --git a/solr/core/src/java/org/apache/solr/response/ResultContext.java b/solr/core/src/java/org/apache/solr/response/ResultContext.java index ea07d076da5..a5dcfab6828 100644 --- a/solr/core/src/java/org/apache/solr/response/ResultContext.java +++ b/solr/core/src/java/org/apache/solr/response/ResultContext.java @@ -57,7 +57,7 @@ public abstract class ResultContext { public boolean wantsScores() { return getReturnFields() != null - && getReturnFields().wantsScore() + && getReturnFields().returnsScore() && getDocList() != null && getDocList().hasScores(); } diff --git a/solr/core/src/java/org/apache/solr/response/TabularResponseWriter.java b/solr/core/src/java/org/apache/solr/response/TabularResponseWriter.java index 133b53405f3..f3e91333a51 100644 --- a/solr/core/src/java/org/apache/solr/response/TabularResponseWriter.java +++ b/solr/core/src/java/org/apache/solr/response/TabularResponseWriter.java @@ -77,7 +77,7 @@ public abstract class TabularResponseWriter extends TextResponseWriter { fields.addAll(explicitReqFields); } - if (returnFields.wantsScore()) { + if (returnFields.returnsScore()) { fields.add("score"); } else { fields.remove("score"); diff --git a/solr/core/src/java/org/apache/solr/response/XMLWriter.java b/solr/core/src/java/org/apache/solr/response/XMLWriter.java index 708bc83e5d8..4d6b10ee026 100644 --- a/solr/core/src/java/org/apache/solr/response/XMLWriter.java +++ b/solr/core/src/java/org/apache/solr/response/XMLWriter.java @@ -179,7 +179,7 @@ public class XMLWriter extends TextResponseWriter { incLevel(); for (String fname : doc.getFieldNames()) { - if (returnFields != null && !returnFields.wantsField(fname)) { + if (returnFields != null && !returnFields.returnsField(fname)) { continue; } diff --git a/solr/core/src/java/org/apache/solr/response/transform/SubQueryAugmenterFactory.java b/solr/core/src/java/org/apache/solr/response/transform/SubQueryAugmenterFactory.java index 4a60804a522..628ddae02af 100644 --- a/solr/core/src/java/org/apache/solr/response/transform/SubQueryAugmenterFactory.java +++ b/solr/core/src/java/org/apache/solr/response/transform/SubQueryAugmenterFactory.java @@ -175,7 +175,7 @@ class SubQueryAugmenter extends DocTransformer { @Override public boolean wantsScores() { - return justWantAllFields.wantsScore(); + return justWantAllFields.returnsScore(); } @Override diff --git a/solr/core/src/java/org/apache/solr/search/ReturnFields.java b/solr/core/src/java/org/apache/solr/search/ReturnFields.java index e01db9f1f5c..05b05c41c80 100644 --- a/solr/core/src/java/org/apache/solr/search/ReturnFields.java +++ b/solr/core/src/java/org/apache/solr/search/ReturnFields.java @@ -32,7 +32,7 @@ public abstract class ReturnFields { * pass this to {@link SolrDocumentFetcher#doc(int, Set)}. * * <p>NOTE: In some situations, this method may return <code>null</code> even if {@link - * #wantsAllFields()} is <code>false</code>. For example: When glob expressions are used ({@link + * #returnsAllFields()} is <code>false</code>. For example: When glob expressions are used ({@link * #hasPatternMatching}), it is safer to request all field names then to attempt to resolve the * globs against all possible dynamic field names in the index. * @@ -44,10 +44,10 @@ public abstract class ReturnFields { * Set of field names with their exact names from the lucene index. * * @param ignoreWantsAll If true, it returns any additional specified field names, in spite of - * also wanting all fields. Example: when fl=*,field1, returns ["field1"]. If false, the - * method returns null when all fields are wanted. Example: when fl=*,field1, returns null. - * Note that this method returns null regardless of ignoreWantsAll if all fields are requested - * and no explicit field names are specified. + * also wanting all fields or globs. Example: when fl=*,field1, returns ["field1"]. If false, + * the method returns null when all fields are wanted or globs are used. Example: when + * fl=*,field1, returns null. Note that this method returns null regardless of ignoreWantsAll + * if all fields are requested and no explicit field names are specified. */ public abstract Set<String> getLuceneFieldNames(boolean ignoreWantsAll); @@ -98,17 +98,59 @@ public abstract class ReturnFields { } /** - * Returns <code>true</code> if the specified field should be returned <em>to the external - * client</em> -- either using its own name, or via an alias. This method returns <code>false - * </code> even if the specified name is needed as an "extra" field for use by transformers. + * Returns <code>true</code> if the value of the specified index field is needed, either to return + * or for use by transformers. This is <code>false</code> for a field that only shares its name + * with a pseudo field (e.g. an alias or a transformer), unless <code>*</code> was requested. */ - public abstract boolean wantsField(String name); + public abstract boolean loadsField(String field); + + /** + * Returns <code>true</code> if the specified index field was named explicitly (including as the + * source of an alias, or a field a transformer needs), rather than only matched by a glob or + * <code>*</code>. + */ + public abstract boolean loadsFieldExplicitly(String field); + + /** + * Returns <code>true</code> if the specified name should be returned <em>to the external + * client</em>: a field's own name, an alias, or the name of a transformer's output. + * + * <p>The value returned under a name is not necessarily the index field of that name. It is a + * label that may be sourced from the same field, another field, or a transformer computing it + * from anything. This returns <code>false</code> for a field that is only needed as an "extra" + * field for use by transformers, see {@link #loadsField}. + */ + public abstract boolean returnsField(String name); + + /** + * @deprecated use {@link #returnsField} + */ + @Deprecated + public boolean wantsField(String name) { + return returnsField(name); + } /** Returns <code>true</code> if all fields should be returned <em>to the external client</em>. */ - public abstract boolean wantsAllFields(); + public abstract boolean returnsAllFields(); + + /** + * @deprecated use {@link #returnsAllFields} + */ + @Deprecated + public boolean wantsAllFields() { + return returnsAllFields(); + } /** Returns <code>true</code> if the score should be returned. */ - public abstract boolean wantsScore(); + public abstract boolean returnsScore(); + + /** + * @deprecated use {@link #returnsScore} + */ + @Deprecated + public boolean wantsScore() { + return returnsScore(); + } /** Returns <code>true</code> if the fieldnames should be picked with a pattern */ public abstract boolean hasPatternMatching(); diff --git a/solr/core/src/java/org/apache/solr/search/SolrDocumentFetcher.java b/solr/core/src/java/org/apache/solr/search/SolrDocumentFetcher.java index 3366d5241ef..eafb64c1601 100644 --- a/solr/core/src/java/org/apache/solr/search/SolrDocumentFetcher.java +++ b/solr/core/src/java/org/apache/solr/search/SolrDocumentFetcher.java @@ -748,15 +748,17 @@ public class SolrDocumentFetcher { return allStored; } + /** Returns an unmodifiable set of all non-stored docValues field names. */ + public Set<String> getNonStoredDVs() { + return allNonStoredDVs; + } + /** - * Returns an unmodifiable set of non-stored docValues field names. - * - * @param onlyUseDocValuesAsStored If false, returns all non-stored docValues. If true, returns - * only those non-stored docValues which have the {@link SchemaField#useDocValuesAsStored()} - * flag true. + * Returns an unmodifiable set of the non-stored docValues field names that have the {@link + * SchemaField#useDocValuesAsStored()} flag true. */ - public Set<String> getNonStoredDVs(boolean onlyUseDocValuesAsStored) { - return onlyUseDocValuesAsStored ? nonStoredDVsUsedAsStored : allNonStoredDVs; + public Set<String> getNonStoredDVsUsedAsStored() { + return nonStoredDVsUsedAsStored; } /** @@ -827,11 +829,11 @@ public class SolrDocumentFetcher { private Set<String> calcStoredFieldsForReturn(ReturnFields returnFields) { final Set<String> storedFields = new HashSet<>(); Set<String> fnames = returnFields.getLuceneFieldNames(); - if (returnFields.wantsAllFields()) { + if (returnFields.returnsAllFields()) { return null; } else if (returnFields.hasPatternMatching()) { for (String s : getAllStored()) { - if (returnFields.wantsField(s)) { + if (returnFields.loadsField(s)) { storedFields.add(s); } } @@ -862,21 +864,23 @@ public class SolrDocumentFetcher { private Set<String> calcDocValueFieldsForReturn(ReturnFields returnFields) { // always return not null final Set<String> result = new HashSet<>(); - if (returnFields.wantsAllFields()) { - result.addAll(getNonStoredDVs(true)); + if (returnFields.returnsAllFields()) { + result.addAll(getNonStoredDVsUsedAsStored()); // check whether there are no additional fields Set<String> fieldNames = returnFields.getLuceneFieldNames(true); if (fieldNames != null) { // add all requested fields that may be useDocValuesAsStored=false for (String fl : fieldNames) { - if (getNonStoredDVs(false).contains(fl)) { + if (getNonStoredDVs().contains(fl)) { result.add(fl); } } } } else if (returnFields.hasPatternMatching()) { - for (String s : getNonStoredDVs(true)) { - if (returnFields.wantsField(s)) { + for (String s : getNonStoredDVs()) { + // a glob only matches useDocValuesAsStored fields, unlike naming the field explicitly + if (returnFields.loadsFieldExplicitly(s) + || (getNonStoredDVsUsedAsStored().contains(s) && returnFields.loadsField(s))) { result.add(s); } } @@ -887,7 +891,7 @@ public class SolrDocumentFetcher { // here we get all non-stored dv fields because even if a user has set // useDocValuesAsStored=false in schema, he may have requested a field // explicitly using the fl parameter - result.retainAll(getNonStoredDVs(false)); + result.retainAll(getNonStoredDVs()); } } return result; diff --git a/solr/core/src/java/org/apache/solr/search/SolrReturnFields.java b/solr/core/src/java/org/apache/solr/search/SolrReturnFields.java index bf850f4de8b..b069a1821ea 100644 --- a/solr/core/src/java/org/apache/solr/search/SolrReturnFields.java +++ b/solr/core/src/java/org/apache/solr/search/SolrReturnFields.java @@ -26,6 +26,7 @@ import java.util.HashSet; import java.util.LinkedHashSet; import java.util.List; import java.util.Map; +import java.util.SequencedSet; import java.util.Set; import java.util.function.Supplier; import org.apache.lucene.queries.function.ValueSource; @@ -55,19 +56,19 @@ public class SolrReturnFields extends ReturnFields { // The lucene field names to request from the SolrIndexSearcher // This *may* include fields that will not be in the final response - private final Set<String> fields = new HashSet<>(); + private final Set<String> loadFields = new HashSet<>(); // Field names that are OK to include in the response. // This will include pseudo fields, lucene fields, and matching globs - private Set<String> okFieldNames = new HashSet<>(); + private Set<String> returnFields = new HashSet<>(); // The list of explicitly requested fields // Order is important for CSVResponseWriter - private Set<String> reqFieldNames = null; + private SequencedSet<String> explicitReturnFields = null; protected DocTransformer transformer; - protected boolean _wantsScore = false; - protected boolean _wantsAllFields = false; + protected boolean _returnScore = false; + protected boolean _returnAllFields = false; protected Map<String, String> renameFields = Map.of(); private final Map<String, String> scoreDependentFields = new HashMap<>(); @@ -97,7 +98,7 @@ public class SolrReturnFields extends ReturnFields { private RetrieveFieldsOptimizer fetchOptimizer = null; public SolrReturnFields() { - _wantsAllFields = true; + _returnAllFields = true; } public SolrReturnFields(SolrQueryRequest req) { @@ -113,8 +114,8 @@ public class SolrReturnFields extends ReturnFields { // legacy thing to support fl=' ' => fl=*,score! // maybe time to drop support for this? // See ConvertedLegacyTest - _wantsScore = true; - _wantsAllFields = true; + _returnScore = true; + _returnAllFields = true; transformer = new ScoreAugmenter(SCORE); scoreDependentFields.put(SCORE, ""); } else { @@ -136,20 +137,20 @@ public class SolrReturnFields extends ReturnFields { */ public SolrReturnFields(Collection<String> plainFields, DocTransformer docTransformer) { if (plainFields != null) { - _wantsAllFields = false; + _returnAllFields = false; for (String field : plainFields) { assert field.indexOf('*') == -1 && !field.equals(SCORE); addField(field, null, null, false); } } else { - _wantsAllFields = true; + _returnAllFields = true; } if (docTransformer != null) { transformer = docTransformer; // doc transformer can request extra fields. String[] extraRequestFields = docTransformer.getExtraRequestFields(); if (extraRequestFields != null) { - Collections.addAll(fields, extraRequestFields); // do NOT call addField + Collections.addAll(loadFields, extraRequestFields); // do NOT call addField } } } @@ -167,10 +168,10 @@ public class SolrReturnFields extends ReturnFields { * logic behind deferring creation of "rename field" transformers). */ private void parseFieldList(String[] fl, SolrQueryRequest req) { - _wantsScore = false; - _wantsAllFields = false; - if (fl == null || fl.length == 0 || (fl.length == 1 && fl[0].length() == 0)) { - _wantsAllFields = true; + _returnScore = false; + _returnAllFields = false; + if (fl == null || fl.length == 0 || (fl.length == 1 && fl[0].isEmpty())) { + _returnAllFields = true; return; } @@ -181,28 +182,18 @@ public class SolrReturnFields extends ReturnFields { } Map<String, String> renamedNotCopied = new HashMap<>(); for (DeferredRenameEntry e : deferredRenameAugmenters) { - DocTransformer t = e.create(renamedNotCopied, reqFieldNames); + DocTransformer t = e.create(renamedNotCopied, explicitReturnFields); augmenters.addTransformer(t); - if (!_wantsAllFields) { + if (!_returnAllFields) { final String[] extraRequestFields = t.getExtraRequestFields(); if (extraRequestFields != null) { - for (String f : extraRequestFields) { - fields.add(f); - } + Collections.addAll(loadFields, extraRequestFields); } } } if (!renamedNotCopied.isEmpty()) { renameFields = renamedNotCopied; } - if (!_wantsAllFields && !globs.isEmpty()) { - // TODO??? need to fill up the fields with matching field names in the index - // and add them to okFieldNames? - // maybe just get all fields? - // this would disable field selection optimization... i think that is OK - fields.clear(); // this will get all fields, and use wantsField to limit - } - if (augmenters.size() == 1) { transformer = augmenters.getTransformer(0); } else if (augmenters.size() > 1) { @@ -309,7 +300,7 @@ public class SolrReturnFields extends ReturnFields { if (field != null && (Character.isWhitespace(ch) || ch == ',' || ch == 0)) { // "*" looks and acts like a glob, but we give it special treatment if ("*".equals(field)) { - _wantsAllFields = true; + _returnAllFields = true; } else { globs.add(field); } @@ -319,7 +310,7 @@ public class SolrReturnFields extends ReturnFields { // originalScore() becomes a true function ch = sp.ch(); if (Character.isWhitespace(ch) || ch == ',' || ch == 0) { - _wantsScore = true; + _returnScore = true; String disp = (key == null) ? ORIGINAL_SCORE : key; augmenters.addTransformer(new OriginalScoreAugmenter(disp)); @@ -353,7 +344,7 @@ public class SolrReturnFields extends ReturnFields { } TransformerFactory factory = req.getCore().getTransformerFactory(augmenterName); - if (factory instanceof TransformerFactory.FieldRenamer) { + if (factory instanceof TransformerFactory.FieldRenamer fieldRenamer) { // NOTE: `deferred` is a Deque because some TransformerFactories (e.g., // `GeoTransformerFactory`) can subtly modify the representation of the associated value // (i.e., it's not just a straight rename). This subverts the "update source field" @@ -363,9 +354,8 @@ public class SolrReturnFields extends ReturnFields { // `FieldRenamers` are added (here) to the front or back of the Deque, depending on the // return value of `mayModifyValue()`. final DeferredRenameEntry deferredEntry = - new DeferredRenameEntry( - disp, augmenterParams, req, (TransformerFactory.FieldRenamer) factory); - if (((TransformerFactory.FieldRenamer) factory).mayModifyValue()) { + new DeferredRenameEntry(disp, augmenterParams, req, fieldRenamer); + if (fieldRenamer.mayModifyValue()) { deferred.addLast(deferredEntry); } else { deferred.addFirst(deferredEntry); @@ -373,12 +363,10 @@ public class SolrReturnFields extends ReturnFields { } else if (factory != null) { DocTransformer t = factory.create(disp, augmenterParams, req); if (t != null) { - if (!_wantsAllFields) { + if (!_returnAllFields) { String[] extra = t.getExtraRequestFields(); if (extra != null) { - for (String f : extra) { - fields.add(f); // also request this field from IndexSearcher - } + Collections.addAll(loadFields, extra); } } augmenters.addTransformer(t); @@ -466,22 +454,11 @@ public class SolrReturnFields extends ReturnFields { } }; - private static final class DeferredRenameEntry { - private final String field; - private final SolrParams params; - private final SolrQueryRequest req; - private final TransformerFactory.FieldRenamer factory; - - private DeferredRenameEntry( - String field, - SolrParams params, - SolrQueryRequest req, - TransformerFactory.FieldRenamer factory) { - this.field = field; - this.params = params; - this.req = req; - this.factory = factory; - } + private record DeferredRenameEntry( + String field, + SolrParams params, + SolrQueryRequest req, + TransformerFactory.FieldRenamer factory) { private DocTransformer create(Map<String, String> renamedFields, Set<String> reqFieldNames) { return factory.create(field, params, req, renamedFields, reqFieldNames); @@ -490,27 +467,27 @@ public class SolrReturnFields extends ReturnFields { private void addField( String field, String key, DocTransformers augmenters, boolean isPseudoField) { - if (reqFieldNames == null) { - reqFieldNames = new LinkedHashSet<>(); + if (explicitReturnFields == null) { + explicitReturnFields = new LinkedHashSet<>(); } if (key == null) { - reqFieldNames.add(field); + explicitReturnFields.add(field); } else { - reqFieldNames.add(key); + explicitReturnFields.add(key); } if (!isPseudoField) { - // fields is returned by getLuceneFieldNames(), to be used to select which real fields - // to return, so pseudo-fields should not be added - fields.add(field); + // loadFields is returned by getLuceneFieldNames(), to be used to select which real fields + // to load, so pseudo-fields should not be added + loadFields.add(field); } - okFieldNames.add(field); - okFieldNames.add(key); + returnFields.add(field); + returnFields.add(key); // a valid field name if (SCORE.equals(field)) { - _wantsScore = true; + _returnScore = true; String disp = (key == null) ? field : key; augmenters.addTransformer(new ScoreAugmenter(disp)); @@ -525,24 +502,25 @@ public class SolrReturnFields extends ReturnFields { @Override public Set<String> getLuceneFieldNames(boolean ignoreWantsAll) { - if (ignoreWantsAll) return fields; - else return (_wantsAllFields || fields.isEmpty()) ? null : fields; + if (ignoreWantsAll) return loadFields; + // with globs, get all fields, and use loadsField to limit + else return (_returnAllFields || !globs.isEmpty() || loadFields.isEmpty()) ? null : loadFields; } @Override public Set<String> getRequestedFieldNames() { - if (_wantsAllFields || reqFieldNames == null || reqFieldNames.isEmpty()) { + if (_returnAllFields || explicitReturnFields == null || explicitReturnFields.isEmpty()) { return null; } - return reqFieldNames; + return explicitReturnFields; } @Override public Set<String> getExplicitlyRequestedFieldNames() { - if (reqFieldNames == null || reqFieldNames.isEmpty()) { + if (explicitReturnFields == null || explicitReturnFields.isEmpty()) { return null; } - return reqFieldNames; + return explicitReturnFields; } @Override @@ -551,13 +529,34 @@ public class SolrReturnFields extends ReturnFields { } @Override - public boolean wantsField(String name) { - if (_wantsAllFields || okFieldNames.contains(name)) { + public boolean loadsField(String field) { + if (_returnAllFields || loadFields.contains(field)) { + return true; + } + if (explicitReturnFields != null && explicitReturnFields.contains(field)) { + return false; // the name of a pseudo field + } + for (String s : globs) { + if (GlobPatternUtil.matches(s, field)) { + return true; + } + } + return false; + } + + @Override + public boolean loadsFieldExplicitly(String field) { + return loadFields.contains(field); + } + + @Override + public boolean returnsField(String name) { + if (_returnAllFields || returnFields.contains(name)) { return true; } for (String s : globs) { if (GlobPatternUtil.matches(s, name)) { - okFieldNames.add(name); // Don't calculate it again + returnFields.add(name); // Don't calculate it again return true; } } @@ -565,13 +564,13 @@ public class SolrReturnFields extends ReturnFields { } @Override - public boolean wantsAllFields() { - return _wantsAllFields; + public boolean returnsAllFields() { + return _returnAllFields; } @Override - public boolean wantsScore() { - return _wantsScore; + public boolean returnsScore() { + return _returnScore; } @Override @@ -586,22 +585,21 @@ public class SolrReturnFields extends ReturnFields { @Override public String toString() { - final StringBuilder sb = new StringBuilder("SolrReturnFields=("); - sb.append("globs="); - sb.append(globs); - sb.append(",fields="); - sb.append(fields); - sb.append(",okFieldNames="); - sb.append(okFieldNames); - sb.append(",reqFieldNames="); - sb.append(reqFieldNames); - sb.append(",transformer="); - sb.append(transformer); - sb.append(",wantsScore="); - sb.append(_wantsScore); - sb.append(",wantsAllFields="); - sb.append(_wantsAllFields); - sb.append(')'); - return sb.toString(); + return "SolrReturnFields=(" + + "globs=" + + globs + + ",loadFields=" + + loadFields + + ",returnFields=" + + returnFields + + ",explicitReturnFields=" + + explicitReturnFields + + ",returnScore=" + + _returnScore + + ",returnAllFields=" + + _returnAllFields + + ",transformer=" + + transformer + + ')'; } } diff --git a/solr/core/src/java/org/apache/solr/search/grouping/distributed/requestfactory/StoredFieldsShardRequestFactory.java b/solr/core/src/java/org/apache/solr/search/grouping/distributed/requestfactory/StoredFieldsShardRequestFactory.java index 14e12a02c89..b571030e20d 100644 --- a/solr/core/src/java/org/apache/solr/search/grouping/distributed/requestfactory/StoredFieldsShardRequestFactory.java +++ b/solr/core/src/java/org/apache/solr/search/grouping/distributed/requestfactory/StoredFieldsShardRequestFactory.java @@ -68,7 +68,7 @@ public class StoredFieldsShardRequestFactory implements ShardRequestFactory { sreq.params.remove(ResponseBuilder.FIELD_SORT_VALUES); // we need to ensure the uniqueField is included for collating docs with their return fields - if (!rb.rsp.getReturnFields().wantsField(uniqueField.getName())) { + if (!rb.rsp.getReturnFields().returnsField(uniqueField.getName())) { // the user didn't ask for it, so we have to... sreq.params.add(CommonParams.FL, uniqueField.getName()); } diff --git a/solr/core/src/test/org/apache/solr/handler/export/TestExportWriter.java b/solr/core/src/test/org/apache/solr/handler/export/TestExportWriter.java index 5be050c8063..77f48638be7 100644 --- a/solr/core/src/test/org/apache/solr/handler/export/TestExportWriter.java +++ b/solr/core/src/test/org/apache/solr/handler/export/TestExportWriter.java @@ -1348,6 +1348,27 @@ public class TestExportWriter extends SolrTestCaseJ4 { assertEquals(3, docs.size()); assertEquals(Map.of("id", "1", "x", 10, "v", "a,b"), docs.get(0)); + // a pseudo-field named like a non-exportable field doesn't fetch that field + docs = + (List<Map<String, Object>>) + queryJsonReturnDocs( + reqWithPath( + "/export", "q", "*:*", "fl", "id,str_s_stored:[value v=a]", "sort", "id asc")); + assertEquals(3, docs.size()); + assertEquals(Map.of("id", "1", "str_s_stored", "a"), docs.get(0)); + docs = + (List<Map<String, Object>>) + queryJsonReturnDocs( + reqWithPath( + "/export", + "q", + "*:*", + "fl", + "id,*_stored,str_s_stored:[value v=a]", + "sort", + "id asc")); + assertEquals(Map.of("id", "1", "str_s_stored", "a"), docs.get(0)); + // transformed fields are visible to streaming expressions docs = (List<Map<String, Object>>) @@ -1586,6 +1607,12 @@ public class TestExportWriter extends SolrTestCaseJ4 { "Expected error message to contain hint about includeStoredFields", resp.contains("includeStoredFields=true")); assertTrue("Expected error message to mention the field", resp.contains("str_s_stored")); + + // ... even alongside a glob + resp = + h.query( + reqWithPath("/export", "q", "*:*", "fl", "id,int*,str_s_stored", "sort", "intdv asc")); + assertTrue(resp, resp.contains("includeStoredFields=true")); } @Test diff --git a/solr/core/src/test/org/apache/solr/search/ReturnFieldsTest.java b/solr/core/src/test/org/apache/solr/search/ReturnFieldsTest.java index 856092c151c..26748e0e27a 100644 --- a/solr/core/src/test/org/apache/solr/search/ReturnFieldsTest.java +++ b/solr/core/src/test/org/apache/solr/search/ReturnFieldsTest.java @@ -111,136 +111,136 @@ public class ReturnFieldsTest extends SolrTestCaseJ4 { final ReturnFields rf1 = new SolrReturnFields(); final String rf1ToString = "SolrReturnFields=(globs=[]" - + ",fields=[]" - + ",okFieldNames=[]" - + ",reqFieldNames=null" - + ",transformer=null,wantsScore=false,wantsAllFields=true)"; + + ",loadFields=[]" + + ",returnFields=[]" + + ",explicitReturnFields=null" + + ",returnScore=false,returnAllFields=true,transformer=null)"; assertEquals(rf1ToString, rf1.toString()); final ReturnFields rf2 = new SolrReturnFields(req("fl", SolrReturnFields.SCORE)); final String rf2ToStringA = "SolrReturnFields=(globs=[]" - + ",fields=[" + + ",loadFields=[" + SolrReturnFields.SCORE + "]" - + ",okFieldNames=[null, " + + ",returnFields=[null, " + SolrReturnFields.SCORE + "]" - + ",reqFieldNames=[" + + ",explicitReturnFields=[" + SolrReturnFields.SCORE + "]" - + ",transformer=score,wantsScore=true,wantsAllFields=false)"; + + ",returnScore=true,returnAllFields=false,transformer=score)"; final String rf2ToStringB = "SolrReturnFields=(globs=[]" - + ",fields=[" + + ",loadFields=[" + SolrReturnFields.SCORE + "]" - + ",okFieldNames=[" + + ",returnFields=[" + SolrReturnFields.SCORE + ", null]" - + ",reqFieldNames=[" + + ",explicitReturnFields=[" + SolrReturnFields.SCORE + "]" - + ",transformer=score,wantsScore=true,wantsAllFields=false)"; + + ",returnScore=true,returnAllFields=false,transformer=score)"; assertTrue(rf2ToStringA.equals(rf2.toString()) || rf2ToStringB.equals(rf2.toString())); } @Test public void testSeparators() { ReturnFields rf = new SolrReturnFields(req("fl", "id name test subject score")); - assertTrue(rf.wantsScore()); - assertTrue(rf.wantsField("id")); - assertTrue(rf.wantsField("name")); - assertTrue(rf.wantsField("test")); - assertTrue(rf.wantsField("subject")); - assertTrue(rf.wantsField("score")); - assertFalse(rf.wantsAllFields()); - assertFalse(rf.wantsField("xxx")); + assertTrue(rf.returnsScore()); + assertTrue(rf.returnsField("id")); + assertTrue(rf.returnsField("name")); + assertTrue(rf.returnsField("test")); + assertTrue(rf.returnsField("subject")); + assertTrue(rf.returnsField("score")); + assertFalse(rf.returnsAllFields()); + assertFalse(rf.returnsField("xxx")); assertTrue(rf.getTransformer() instanceof ScoreAugmenter); rf = new SolrReturnFields(req("fl", "id,name,test,subject,score")); - assertTrue(rf.wantsScore()); - assertTrue(rf.wantsField("id")); - assertTrue(rf.wantsField("name")); - assertTrue(rf.wantsField("test")); - assertTrue(rf.wantsField("subject")); - assertTrue(rf.wantsField("score")); - assertFalse(rf.wantsAllFields()); - assertFalse(rf.wantsField("xxx")); + assertTrue(rf.returnsScore()); + assertTrue(rf.returnsField("id")); + assertTrue(rf.returnsField("name")); + assertTrue(rf.returnsField("test")); + assertTrue(rf.returnsField("subject")); + assertTrue(rf.returnsField("score")); + assertFalse(rf.returnsAllFields()); + assertFalse(rf.returnsField("xxx")); assertTrue(rf.getTransformer() instanceof ScoreAugmenter); rf = new SolrReturnFields(req("fl", "id,name test,subject score")); - assertTrue(rf.wantsScore()); - assertTrue(rf.wantsField("id")); - assertTrue(rf.wantsField("name")); - assertTrue(rf.wantsField("test")); - assertTrue(rf.wantsField("subject")); - assertTrue(rf.wantsField("score")); - assertFalse(rf.wantsAllFields()); - assertFalse(rf.wantsField("xxx")); + assertTrue(rf.returnsScore()); + assertTrue(rf.returnsField("id")); + assertTrue(rf.returnsField("name")); + assertTrue(rf.returnsField("test")); + assertTrue(rf.returnsField("subject")); + assertTrue(rf.returnsField("score")); + assertFalse(rf.returnsAllFields()); + assertFalse(rf.returnsField("xxx")); assertTrue(rf.getTransformer() instanceof ScoreAugmenter); rf = new SolrReturnFields(req("fl", "id, name test , subject,score")); - assertTrue(rf.wantsScore()); - assertTrue(rf.wantsField("id")); - assertTrue(rf.wantsField("name")); - assertTrue(rf.wantsField("test")); - assertTrue(rf.wantsField("subject")); - assertTrue(rf.wantsField("score")); - assertFalse(rf.wantsAllFields()); - assertFalse(rf.wantsField("xxx")); + assertTrue(rf.returnsScore()); + assertTrue(rf.returnsField("id")); + assertTrue(rf.returnsField("name")); + assertTrue(rf.returnsField("test")); + assertTrue(rf.returnsField("subject")); + assertTrue(rf.returnsField("score")); + assertFalse(rf.returnsAllFields()); + assertFalse(rf.returnsField("xxx")); assertTrue(rf.getTransformer() instanceof ScoreAugmenter); } @Test public void testWildcards() { ReturnFields rf = new SolrReturnFields(req("fl", "*")); - assertFalse(rf.wantsScore()); - assertTrue(rf.wantsField("xxx")); - assertTrue(rf.wantsAllFields()); + assertFalse(rf.returnsScore()); + assertTrue(rf.returnsField("xxx")); + assertTrue(rf.returnsAllFields()); assertNull(rf.getTransformer()); rf = new SolrReturnFields(req("fl", " * ")); - assertFalse(rf.wantsScore()); - assertTrue(rf.wantsField("xxx")); - assertTrue(rf.wantsAllFields()); + assertFalse(rf.returnsScore()); + assertTrue(rf.returnsField("xxx")); + assertTrue(rf.returnsAllFields()); assertNull(rf.getTransformer()); // Check that we want wildcards rf = new SolrReturnFields(req("fl", "id,aaa*,*bbb")); - assertTrue(rf.wantsField("id")); - assertTrue(rf.wantsField("aaaxxx")); - assertFalse(rf.wantsField("xxxaaa")); - assertTrue(rf.wantsField("xxxbbb")); - assertFalse(rf.wantsField("bbbxxx")); - assertFalse(rf.wantsField("aa")); - assertFalse(rf.wantsField("bb")); + assertTrue(rf.returnsField("id")); + assertTrue(rf.returnsField("aaaxxx")); + assertFalse(rf.returnsField("xxxaaa")); + assertTrue(rf.returnsField("xxxbbb")); + assertFalse(rf.returnsField("bbbxxx")); + assertFalse(rf.returnsField("aa")); + assertFalse(rf.returnsField("bb")); } @Test public void testManyParameters() { ReturnFields rf = new SolrReturnFields(req("fl", "id name", "fl", "test subject", "fl", "score")); - assertTrue(rf.wantsScore()); - assertTrue(rf.wantsField("id")); - assertTrue(rf.wantsField("name")); - assertTrue(rf.wantsField("test")); - assertTrue(rf.wantsField("subject")); - assertTrue(rf.wantsField("score")); - assertFalse(rf.wantsAllFields()); - assertFalse(rf.wantsField("xxx")); + assertTrue(rf.returnsScore()); + assertTrue(rf.returnsField("id")); + assertTrue(rf.returnsField("name")); + assertTrue(rf.returnsField("test")); + assertTrue(rf.returnsField("subject")); + assertTrue(rf.returnsField("score")); + assertFalse(rf.returnsAllFields()); + assertFalse(rf.returnsField("xxx")); assertTrue(rf.getTransformer() instanceof ScoreAugmenter); } @Test public void testFunctions() { ReturnFields rf = new SolrReturnFields(req("fl", "exists(text),id,sum(1,1)")); - assertFalse(rf.wantsScore()); - assertTrue(rf.wantsField("id")); - assertTrue(rf.wantsField("sum(1,1)")); - assertTrue(rf.wantsField("exists(text)")); - assertFalse(rf.wantsAllFields()); - assertFalse(rf.wantsField("xxx")); + assertFalse(rf.returnsScore()); + assertTrue(rf.returnsField("id")); + assertTrue(rf.returnsField("sum(1,1)")); + assertTrue(rf.returnsField("exists(text)")); + assertFalse(rf.returnsAllFields()); + assertFalse(rf.returnsField("xxx")); assertTrue(rf.getTransformer() instanceof DocTransformers); DocTransformers transformers = (DocTransformers) rf.getTransformer(); assertEquals("exists(text)", transformers.getTransformer(0).getName()); @@ -250,58 +250,58 @@ public class ReturnFieldsTest extends SolrTestCaseJ4 { @Test public void testTransformers() { ReturnFields rf = new SolrReturnFields(req("fl", "[explain]")); - assertFalse(rf.wantsScore()); - assertTrue(rf.wantsField("[explain]")); - assertFalse(rf.wantsField("id")); - assertFalse(rf.wantsAllFields()); + assertFalse(rf.returnsScore()); + assertTrue(rf.returnsField("[explain]")); + assertFalse(rf.returnsField("id")); + assertFalse(rf.returnsAllFields()); assertEquals("[explain]", rf.getTransformer().getName()); rf = new SolrReturnFields(req("fl", "[shard],id")); - assertFalse(rf.wantsScore()); - assertTrue(rf.wantsField("[shard]")); - assertTrue(rf.wantsField("id")); - assertFalse(rf.wantsField("xxx")); - assertFalse(rf.wantsAllFields()); + assertFalse(rf.returnsScore()); + assertTrue(rf.returnsField("[shard]")); + assertTrue(rf.returnsField("id")); + assertFalse(rf.returnsField("xxx")); + assertFalse(rf.returnsAllFields()); assertEquals("[shard]", rf.getTransformer().getName()); rf = new SolrReturnFields(req("fl", "[docid]")); - assertFalse(rf.wantsScore()); - assertTrue(rf.wantsField("[docid]")); - assertFalse(rf.wantsField("id")); - assertFalse(rf.wantsField("xxx")); - assertFalse(rf.wantsAllFields()); + assertFalse(rf.returnsScore()); + assertTrue(rf.returnsField("[docid]")); + assertFalse(rf.returnsField("id")); + assertFalse(rf.returnsField("xxx")); + assertFalse(rf.returnsAllFields()); assertEquals("[docid]", rf.getTransformer().getName()); rf = new SolrReturnFields(req("fl", "mydocid:[docid]")); - assertFalse(rf.wantsScore()); - assertTrue(rf.wantsField("mydocid")); - assertFalse(rf.wantsField("id")); - assertFalse(rf.wantsField("xxx")); - assertFalse(rf.wantsAllFields()); + assertFalse(rf.returnsScore()); + assertTrue(rf.returnsField("mydocid")); + assertFalse(rf.returnsField("id")); + assertFalse(rf.returnsField("xxx")); + assertFalse(rf.returnsAllFields()); assertEquals("mydocid", rf.getTransformer().getName()); rf = new SolrReturnFields(req("fl", "[docid][shard]")); - assertFalse(rf.wantsScore()); - assertTrue(rf.wantsField("[docid]")); - assertTrue(rf.wantsField("[shard]")); - assertFalse(rf.wantsField("xxx")); - assertFalse(rf.wantsAllFields()); + assertFalse(rf.returnsScore()); + assertTrue(rf.returnsField("[docid]")); + assertTrue(rf.returnsField("[shard]")); + assertFalse(rf.returnsField("xxx")); + assertFalse(rf.returnsAllFields()); assertTrue(rf.getTransformer() instanceof DocTransformers); assertEquals(2, ((DocTransformers) rf.getTransformer()).size()); rf = new SolrReturnFields(req("fl", "[xxxxx]")); - assertFalse(rf.wantsScore()); - assertTrue(rf.wantsField("[xxxxx]")); - assertFalse(rf.wantsField("id")); - assertFalse(rf.wantsAllFields()); + assertFalse(rf.returnsScore()); + assertTrue(rf.returnsField("[xxxxx]")); + assertFalse(rf.returnsField("id")); + assertFalse(rf.returnsAllFields()); assertNull(rf.getTransformer()); // Don't return 'store' just because it is required by the transformer rf = new SolrReturnFields(req("fl", "[geo f=store]")); - assertFalse(rf.wantsScore()); - assertTrue(rf.wantsField("[geo]")); - assertFalse(rf.wantsField("store")); - assertFalse(rf.wantsAllFields()); + assertFalse(rf.returnsScore()); + assertTrue(rf.returnsField("[geo]")); + assertFalse(rf.returnsField("store")); + assertFalse(rf.returnsAllFields()); assertNotNull(rf.getTransformer()); } @@ -309,85 +309,125 @@ public class ReturnFieldsTest extends SolrTestCaseJ4 { public void testAliases() { ReturnFields rf = new SolrReturnFields(req("fl", "newId:id newName:name newTest:test newSubject:subject")); - assertTrue(rf.wantsField("id")); - assertTrue(rf.wantsField("name")); - assertTrue(rf.wantsField("test")); - assertTrue(rf.wantsField("subject")); - assertTrue(rf.wantsField("newId")); - assertTrue(rf.wantsField("newName")); - assertTrue(rf.wantsField("newTest")); - assertTrue(rf.wantsField("newSubject")); - assertFalse(rf.wantsField("xxx")); - assertFalse(rf.wantsAllFields()); + assertTrue(rf.returnsField("id")); + assertTrue(rf.returnsField("name")); + assertTrue(rf.returnsField("test")); + assertTrue(rf.returnsField("subject")); + assertTrue(rf.returnsField("newId")); + assertTrue(rf.returnsField("newName")); + assertTrue(rf.returnsField("newTest")); + assertTrue(rf.returnsField("newSubject")); + assertFalse(rf.returnsField("xxx")); + assertFalse(rf.returnsAllFields()); rf = new SolrReturnFields( req("fl", "newId:id newName:name newTest:test newSubject:subject score")); - assertTrue(rf.wantsField("id")); - assertTrue(rf.wantsField("name")); - assertTrue(rf.wantsField("test")); - assertTrue(rf.wantsField("subject")); - assertTrue(rf.wantsField("newId")); - assertTrue(rf.wantsField("newName")); - assertTrue(rf.wantsField("newTest")); - assertTrue(rf.wantsField("newSubject")); - assertFalse(rf.wantsField("xxx")); - assertFalse(rf.wantsAllFields()); + assertTrue(rf.returnsField("id")); + assertTrue(rf.returnsField("name")); + assertTrue(rf.returnsField("test")); + assertTrue(rf.returnsField("subject")); + assertTrue(rf.returnsField("newId")); + assertTrue(rf.returnsField("newName")); + assertTrue(rf.returnsField("newTest")); + assertTrue(rf.returnsField("newSubject")); + assertFalse(rf.returnsField("xxx")); + assertFalse(rf.returnsAllFields()); assertTrue(rf.getTransformer() instanceof DocTransformers); assertEquals(5, ((DocTransformers) rf.getTransformer()).size()); // 4 rename and score } + @Test + public void testLoadsField() { + ReturnFields rf = new SolrReturnFields(req("fl", "id,newName:name,store:[geo f=store]")); + assertTrue(rf.loadsField("id")); + assertTrue(rf.loadsFieldExplicitly("id")); + assertTrue(rf.loadsField("name")); // alias source + assertTrue(rf.loadsFieldExplicitly("name")); + assertFalse(rf.loadsField("newName")); // alias + assertFalse(rf.loadsFieldExplicitly("newName")); + assertTrue(rf.loadsField("store")); // read by the transformer + assertTrue(rf.loadsFieldExplicitly("store")); + assertFalse(rf.loadsField("xxx")); + + // transformer named like a field it doesn't read + rf = new SolrReturnFields(req("fl", "id,subject:[docid]")); + assertFalse(rf.loadsField("subject")); + assertFalse(rf.loadsFieldExplicitly("subject")); + + // with globs + rf = new SolrReturnFields(req("fl", "id,aaa*,newName:name,aaa_s:[docid],[geo f=store]")); + assertTrue(rf.loadsField("id")); + assertTrue(rf.loadsFieldExplicitly("id")); + assertTrue(rf.loadsField("aaaxxx")); + assertFalse(rf.loadsFieldExplicitly("aaaxxx")); + assertTrue(rf.loadsField("name")); + assertTrue(rf.loadsFieldExplicitly("name")); + assertFalse(rf.loadsField("newName")); + assertFalse(rf.loadsField("aaa_s")); // pseudo field, despite the glob + assertTrue(rf.loadsField("store")); + assertTrue(rf.loadsFieldExplicitly("store")); + assertFalse(rf.loadsField("xxx")); + + // with * + rf = new SolrReturnFields(req("fl", "*,name,subject:[docid]")); + assertTrue(rf.loadsField("xxx")); + assertFalse(rf.loadsFieldExplicitly("xxx")); + assertTrue(rf.loadsField("subject")); + assertTrue(rf.loadsFieldExplicitly("name")); + } + // hyphens in field names are not supported in all contexts, but we wanted // the simplest case of fl=foo-bar to work @Test public void testHyphenInFieldName() { ReturnFields rf = new SolrReturnFields(req("fl", "id-test")); - assertFalse(rf.wantsScore()); - assertTrue(rf.wantsField("id-test")); - assertFalse(rf.wantsField("xxx")); - assertFalse(rf.wantsAllFields()); + assertFalse(rf.returnsScore()); + assertTrue(rf.returnsField("id-test")); + assertFalse(rf.returnsField("xxx")); + assertFalse(rf.returnsAllFields()); } @Test public void testTrailingDotInFieldName() { ReturnFields rf = new SolrReturnFields(req("fl", "id.test")); - assertFalse(rf.wantsScore()); - assertTrue(rf.wantsField("id.test")); - assertFalse(rf.wantsField("xxx")); - assertFalse(rf.wantsAllFields()); + assertFalse(rf.returnsScore()); + assertTrue(rf.returnsField("id.test")); + assertFalse(rf.returnsField("xxx")); + assertFalse(rf.returnsAllFields()); rf = new SolrReturnFields(req("fl", "test:id.test")); - assertFalse(rf.wantsScore()); - assertTrue(rf.wantsField("id.test")); - assertTrue(rf.wantsField("test")); - assertFalse(rf.wantsField("xxx")); - assertFalse(rf.wantsAllFields()); + assertFalse(rf.returnsScore()); + assertTrue(rf.returnsField("id.test")); + assertTrue(rf.returnsField("test")); + assertFalse(rf.returnsField("xxx")); + assertFalse(rf.returnsAllFields()); rf = new SolrReturnFields(req("fl", "test.id:id.test")); - assertFalse(rf.wantsScore()); - assertTrue(rf.wantsField("id.test")); - assertTrue(rf.wantsField("test.id")); - assertFalse(rf.wantsField("xxx")); - assertFalse(rf.wantsAllFields()); + assertFalse(rf.returnsScore()); + assertTrue(rf.returnsField("id.test")); + assertTrue(rf.returnsField("test.id")); + assertFalse(rf.returnsField("xxx")); + assertFalse(rf.returnsAllFields()); } @Test public void testTrailingDollarInFieldName() { ReturnFields rf = new SolrReturnFields(req("fl", "id$test")); - assertFalse(rf.wantsScore()); - assertTrue(rf.wantsField("id$test")); - assertFalse(rf.wantsField("xxx")); - assertFalse(rf.wantsAllFields()); + assertFalse(rf.returnsScore()); + assertTrue(rf.returnsField("id$test")); + assertFalse(rf.returnsField("xxx")); + assertFalse(rf.returnsAllFields()); } @Test public void testFunkyFieldNames() { ReturnFields rf = new SolrReturnFields(req("fl", "#foo_s", "fl", "id")); - assertFalse(rf.wantsScore()); - assertTrue(rf.wantsField("id")); - assertTrue(rf.wantsField("#foo_s")); - assertFalse(rf.wantsField("xxx")); - assertFalse(rf.wantsAllFields()); + assertFalse(rf.returnsScore()); + assertTrue(rf.returnsField("id")); + assertTrue(rf.returnsField("#foo_s")); + assertFalse(rf.returnsField("xxx")); + assertFalse(rf.returnsAllFields()); assertQ( req("q", "id:1", "fl", "#foo_s", "fl", "id"), @@ -465,7 +505,7 @@ public class ReturnFieldsTest extends SolrTestCaseJ4 { new SolrReturnFields(req()), new SolrReturnFields(req("fl", "*")), new SolrReturnFields(req("fl", "*,score")), - new SolrReturnFields(req("fl", "id,subword,uniq,foo_*,store_*")), + new SolrReturnFields(req("fl", "id,subword,uniq,foo_*,store*")), new SolrReturnFields(req("fl", allFieldNames + ",bogus1,bogus2,bogus3")))) { docOut = convertLuceneDocToSolrDoc(docIn, schema, rf); @@ -503,16 +543,16 @@ public class ReturnFieldsTest extends SolrTestCaseJ4 { final String fl = id + (r.nextBoolean() ? "" : ",") + foo_i; ReturnFields rf = new SolrReturnFields(req("fl", fl)); - assertFalse("score (" + fl + ")", rf.wantsScore()); + assertFalse("score (" + fl + ")", rf.returnsScore()); - assertTrue("id (" + fl + ")", rf.wantsField("id")); - assertTrue("foo_i (" + fl + ")", rf.wantsField("foo_i")); + assertTrue("id (" + fl + ")", rf.returnsField("id")); + assertTrue("foo_i (" + fl + ")", rf.returnsField("foo_i")); - assertEquals("aliasId (" + fl + ")", aliasId, rf.wantsField("aliasId")); - assertEquals("aliasFoo (" + fl + ")", aliasFoo, rf.wantsField("aliasFoo")); + assertEquals("aliasId (" + fl + ")", aliasId, rf.returnsField("aliasId")); + assertEquals("aliasFoo (" + fl + ")", aliasFoo, rf.returnsField("aliasFoo")); - assertFalse(rf.wantsField("xxx")); - assertFalse(rf.wantsAllFields()); + assertFalse(rf.returnsField("xxx")); + assertFalse(rf.returnsAllFields()); } } diff --git a/solr/core/src/test/org/apache/solr/search/TestPseudoReturnFields.java b/solr/core/src/test/org/apache/solr/search/TestPseudoReturnFields.java index 2fee5d8f858..28898246f10 100644 --- a/solr/core/src/test/org/apache/solr/search/TestPseudoReturnFields.java +++ b/solr/core/src/test/org/apache/solr/search/TestPseudoReturnFields.java @@ -485,6 +485,39 @@ public class TestPseudoReturnFields extends SolrTestCaseJ4 { } } + public void testGlobsAndExplicitNonStoredDocValues() { + // _version_ is useDocValuesAsStored=false, so only returned when named explicitly + assertQ( + req("q", "*:*", "rows", "1", "fl", "val_*,_version_"), + "//result[@numFound='5']", + "//result/doc/int[@name='val_i']", + "//result/doc/long[@name='_version_']", + "//result/doc[count(*)=2]"); + } + + public void testGlobsAndTransformerField() { + // the field read by the transformer is fetched, though the glob doesn't match it + assertQ( + req("q", "*:*", "rows", "1", "fl", "val_*,x:[json f=ssto]"), + "//result[@numFound='5']", + "//result/doc/int[@name='val_i']", + "//result/doc/str[@name='x'][.='X']", + "//result/doc[count(*)=2]"); + } + + public void testGlobsAndTransformerFieldRTG() { + // behavior shouldn't matter if we are committed or uncommitted + for (String id : Arrays.asList("42", "99")) { + assertQ( + id + ": fl=val_*,x:[json f=ssto]", + reqWithPath("/get", "id", id, "wt", "xml", "fl", "val_*,x:[json f=ssto]"), + "count(//doc)=1", + "//doc/int[@name='val_i'][.=1]", + "//doc/str[@name='x'][.='X']", + "//doc[count(*)=2]"); + } + } + public void testGlobsAndScore() { assertQ( "fl=val_*,score",
