SOLR-17022: Support for glob patterns for fields in Export handler, Stream handler and with SelectStream streaming expression #1996
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -27,9 +27,13 @@ | |
| 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.List; | ||
| import java.util.Map; | ||
| import java.util.Set; | ||
| import java.util.TreeSet; | ||
| import org.apache.lucene.index.FieldInfo; | ||
| import org.apache.lucene.index.LeafReader; | ||
| import org.apache.lucene.index.LeafReaderContext; | ||
| import org.apache.lucene.index.SortedDocValues; | ||
|
|
@@ -53,6 +57,7 @@ | |
| import org.apache.solr.common.params.CommonParams; | ||
| import org.apache.solr.common.params.SolrParams; | ||
| import org.apache.solr.common.params.StreamParams; | ||
| import org.apache.solr.common.util.GlobPatternUtil; | ||
| import org.apache.solr.common.util.JavaBinCodec; | ||
| import org.apache.solr.core.SolrCore; | ||
| import org.apache.solr.metrics.SolrMetricsContext; | ||
|
|
@@ -487,19 +492,14 @@ void writeDoc( | |
|
|
||
| public FieldWriter[] getFieldWriters(String[] fields, SolrIndexSearcher searcher) | ||
| throws IOException { | ||
| DocValuesIteratorCache dvIterCache = new DocValuesIteratorCache(searcher, false); | ||
|
|
||
| List<SchemaField> expandedFields = expandFieldList(fields, searcher); | ||
|
|
||
| FieldWriter[] writers = new FieldWriter[expandedFields.size()]; | ||
| for (int i = 0; i < expandedFields.size(); i++) { | ||
| SchemaField schemaField = expandedFields.get(i); | ||
| String field = schemaField.getName(); | ||
| if (!schemaField.hasDocValues()) { | ||
| throw new IOException(schemaField + " must have DocValues to use this feature."); | ||
| } | ||
|
|
@@ -844,4 +844,61 @@ public String getMessage() { | |
| return "Early Client Disconnect"; | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Creates a complete field list using the provided field list by expanding any glob patterns into | ||
| * field names | ||
| * | ||
| * @param fields the original set of fields provided | ||
| * @param searcher an index searcher to access schema info | ||
| * @return a complete list of fields included any fields matching glob patterns | ||
| * @throws IOException if a provided field does not exist or cannot be retrieved from the schema | ||
| * info | ||
| */ | ||
| private List<SchemaField> expandFieldList(String[] fields, SolrIndexSearcher searcher) | ||
| throws IOException { | ||
| List<SchemaField> expandedFields = new ArrayList<>(fields.length); | ||
| Set<String> fieldsProcessed = new HashSet<>(); | ||
| for (String field : fields) { | ||
| try { | ||
| if (field.contains("*")) { | ||
| getGlobFields(field, searcher, fieldsProcessed, expandedFields); | ||
| } else { | ||
| if (fieldsProcessed.add(field)) { | ||
| expandedFields.add(searcher.getSchema().getField(field)); | ||
| } | ||
| } | ||
| } catch (Exception e) { | ||
| throw new IOException(e); | ||
| } | ||
| } | ||
|
|
||
| return expandedFields; | ||
| } | ||
|
|
||
| /** | ||
| * Create a list of schema fields that match a given glob pattern | ||
| * | ||
| * @param fieldPattern the glob pattern to match | ||
| * @param searcher an index search to access schema info | ||
| * @param fieldsProcessed the set of field names already processed to avoid duplicating | ||
| * @param expandedFields the list of fields to add expanded field names into | ||
| */ | ||
| private void getGlobFields( | ||
| String fieldPattern, | ||
| SolrIndexSearcher searcher, | ||
| Set<String> fieldsProcessed, | ||
| List<SchemaField> expandedFields) { | ||
| for (FieldInfo fi : searcher.getFieldInfos()) { | ||
| if (GlobPatternUtil.matches(fieldPattern, fi.getName())) { | ||
| SchemaField schemaField = searcher.getSchema().getField(fi.getName()); | ||
| if (fieldsProcessed.add(fi.getName()) | ||
|
There was a problem hiding this comment. This line might add to fieldsProcessed, yet exclude the field because doesn't "hasDocValues". This looks suspicious to me. |
||
| && schemaField.hasDocValues() | ||
| && (!(schemaField.getType() instanceof SortableTextField) | ||
|
There was a problem hiding this comment. Wanted to highlight this because @magibney and I were discussing this previously. The way this is implemented for the Export handler here actually deviates from how glob patterns work for the Select handler. The Select handler will not match any fields where For the purposes of Export I did not follow that pattern and instead will match any fields that have docvalues enabled with the exception of SortableTextField which require My reasoning here is that there is a performance hit for getting non-stored, docvalues enabled fields in the Select handler that doesn't seem to be to be as impactful in the Export handler. Open to other opinions on this topic, should we:
There was a problem hiding this comment. I vote for consistency if possible, since I think of both /select and /export as the same, just one is faster than the other ;-). If that isn't possible, then maybe if the ref guide makes it really clear "This is why you use /select and ramifications" and "THis is why you use /export and it's ramifications", then that might cover these differences. I can see the bug reports if we aren't clear ;-). There was a problem hiding this comment. As @justinrsweeney foreshadowed, I am in favor of parity in how this is handled between select and export. The purpose of udvas as I understand it is not simply to mitigate performance issues in select (and in fact, performance should be comparable between select and export as of #1938), but also as a config option to allow the configuration of docValues for a field without requiring that those values be returned when a field matches a glob pattern. There are plenty of use cases where, e.g., a derivative string field is configured solely for the purpose of sort, faceting, [or other purpose that would make even less sense to return on export]. It should be possible to configure a field that's intended for one of the many other purposes docValues serve, without forcing it to be returned when the field name matches a glob pattern. Note also, if both I think what I'm missing is: how does this help usability? Are there cases where one would want docValues to be used for field value retrieval, but could not simply set that field to I'm curious to hear others' thoughts on this as well! There was a problem hiding this comment. I updated this PR to mimic the same behavior as the select handler, where |
||
| || schemaField.useDocValuesAsStored())) { | ||
| expandedFields.add(schemaField); | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
| /* | ||
| * Licensed to the Apache Software Foundation (ASF) under one or more | ||
| * contributor license agreements. See the NOTICE file distributed with | ||
| * this work for additional information regarding copyright ownership. | ||
| * The ASF licenses this file to You under the Apache License, Version 2.0 | ||
| * (the "License"); you may not use this file except in compliance with | ||
| * the License. You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, software | ||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| */ | ||
| package org.apache.solr.common.util; | ||
justinrsweeney marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| import java.nio.file.FileSystems; | ||
| import java.nio.file.Paths; | ||
|
|
||
| public class GlobPatternUtil { | ||
dsmiley marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| public static boolean matches(String pattern, String input) { | ||
| return FileSystems.getDefault().getPathMatcher("glob:" + pattern).matches(Paths.get(input)); | ||
|
There was a problem hiding this comment. This is bizarre just for some field wildcard support. If there is a reason we use FileSystems then a comment is necessary. There was a problem hiding this comment. I took a look at how it's implemented. If only we could call ZipUtils.toRegexPattern but the class is package protected. It's a shame to recompile the glob on each call to matches! |
||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,33 @@ | ||
| /* | ||
| * Licensed to the Apache Software Foundation (ASF) under one or more | ||
| * contributor license agreements. See the NOTICE file distributed with | ||
| * this work for additional information regarding copyright ownership. | ||
| * The ASF licenses this file to You under the Apache License, Version 2.0 | ||
| * (the "License"); you may not use this file except in compliance with | ||
| * the License. You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, software | ||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| */ | ||
| package org.apache.solr.common.util; | ||
justinrsweeney marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| import org.apache.solr.SolrTestCase; | ||
|
|
||
| public class TestGlobPatternUtil extends SolrTestCase { | ||
|
|
||
| public void testMatches() { | ||
| assertTrue(GlobPatternUtil.matches("*_str", "user_str")); | ||
| assertFalse(GlobPatternUtil.matches("*_str", "str_user")); | ||
| assertTrue(GlobPatternUtil.matches("str_*", "str_user")); | ||
| assertFalse(GlobPatternUtil.matches("str_*", "user_str")); | ||
| assertTrue(GlobPatternUtil.matches("str?", "str1")); | ||
| assertFalse(GlobPatternUtil.matches("str?", "str_user")); | ||
| assertTrue(GlobPatternUtil.matches("user_*_str", "user_type_str")); | ||
| assertFalse(GlobPatternUtil.matches("user_*_str", "user_str")); | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There can be many fieldInfo, and you're looping over a "matches" call that is going to internally build a regex each time. Maybe you should first do the hasDocValues etc. checks so we do this matches check last?