epugh commented on code in PR #4053:
URL: https://github.com/apache/solr/pull/4053#discussion_r2699220549
##########
solr/core/src/java/org/apache/solr/handler/export/ExportWriter.java:
##########
@@ -484,43 +494,75 @@ void writeDoc(
LeafReaderContext context = leaves.get(ord);
int fieldIndex = 0;
for (FieldWriter fieldWriter : writers) {
- if (fieldWriter.write(sortDoc, context, ew, fieldIndex)) {
- ++fieldIndex;
- }
+ fieldIndex += fieldWriter.write(sortDoc, context, ew, fieldIndex);
}
}
public List<FieldWriter> getFieldWriters(String[] fields, SolrQueryRequest
req)
throws IOException {
DocValuesIteratorCache dvIterCache = new
DocValuesIteratorCache(req.getSearcher(), false);
-
SolrReturnFields solrReturnFields = new SolrReturnFields(fields, req);
+ boolean includeStoredFields =
req.getParams().getBool(INCLUDE_STORED_FIELDS_PARAM, false);
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)) {
continue;
}
SchemaField schemaField = req.getSchema().getField(field);
- if (!schemaField.hasDocValues()) {
- throw new IOException(schemaField + " must have DocValues to use this
feature.");
Review Comment:
Love this! I've been burned so many times on wanting to do this and
discovering missing DocValues.
##########
solr/solr-ref-guide/modules/query-guide/pages/exporting-result-sets.adoc:
##########
@@ -44,6 +46,11 @@ Filter queries are also supported.
An optional parameter `batchSize` determines the size of the internal buffers
for partial results.
The default value is `30000` but users may want to specify smaller values to
limit the memory use (at the cost of degraded performance) or higher values to
improve export performance (the relationship is not linear and larger values
don't bring proportionally larger performance increases).
+An optional parameter `includeStoredFields` (default `false`) enables
exporting fields that only have stored values (no docValues).
Review Comment:
some day we'll have a way of thinking about should this be "true" ;-).
Challenge of open source is we struggle to know what hsould be a default yes
even when it has knock on impacts until a lot of time passes. ;-)
##########
solr/core/src/test/org/apache/solr/handler/export/TestExportWriter.java:
##########
@@ -1476,4 +1474,261 @@ private void addField(SolrInputDocument doc, String
type, String value, boolean
doc.addField("number_" + type + (mv ? "s" : "") + "_ni_t", value);
doc.addField("number_" + type + (mv ? "s" : "") + "_ni_p", value);
}
+
+ @Test
+ public void testIncludeStoredFieldsExplicitRequest() throws Exception {
+ // Test that stored-only fields are returned when includeStoredFields=true
+ clearIndex();
Review Comment:
maybe not speciifc to your goal, but should `clearIndex()` be in a before
method/setup step? If it's on every unit test?
##########
solr/core/src/java/org/apache/solr/handler/export/DoubleFieldWriter.java:
##########
@@ -34,7 +34,7 @@ public DoubleFieldWriter(
}
@Override
- public boolean write(
+ public int write(
Review Comment:
I went down thorugh the code, why the boolean -> int? Looks like we only
ever return 0 or 1?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
