From d7b878e90c6ce185d799f1fa554e8c3770793f80 Mon Sep 17 00:00:00 2001 From: Mikhail Khludnev Date: Sun, 25 Nov 2018 11:26:39 +0300 Subject: [PATCH] SOLR-12546: Let csv response writer to handle docValues fields by default. --- solr/CHANGES.txt | 3 +- .../solr/response/CSVResponseWriter.java | 9 ++-- .../solr/collection1/conf/schema12.xml | 4 ++ .../solr/response/TestCSVResponseWriter.java | 49 +++++++++++++++++-- 4 files changed, 56 insertions(+), 9 deletions(-) diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index 13009a5408e..9e63d6690ee 100644 --- a/solr/CHANGES.txt +++ b/solr/CHANGES.txt @@ -130,8 +130,9 @@ Other Changes Bug Fixes ---------------------- +* SOLR-12546: CVSResponseWriter omits useDocValuesAsStored=true field when fl=* + (Munendra S N via Mikhail Khludnev) -(No Changes) Improvements ---------------------- 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 e894c77a839..5ebec77ed16 100644 --- a/solr/core/src/java/org/apache/solr/response/CSVResponseWriter.java +++ b/solr/core/src/java/org/apache/solr/response/CSVResponseWriter.java @@ -247,7 +247,7 @@ class CSVWriter extends TextResponseWriter { Collection fields = returnFields.getRequestedFieldNames(); Object responseObj = rsp.getResponse(); - boolean returnOnlyStored = false; + boolean returnStoredOrDocValStored = false; if (fields==null||returnFields.hasPatternMatching()) { if (responseObj instanceof SolrDocumentList) { // get the list of fields from the SolrDocumentList @@ -271,7 +271,7 @@ class CSVWriter extends TextResponseWriter { } else { fields.remove("score"); } - returnOnlyStored = true; + returnStoredOrDocValStored = true; } CSVSharedBufPrinter csvPrinterMV = new CSVSharedBufPrinter(mvWriter, mvStrategy); @@ -293,8 +293,9 @@ class CSVWriter extends TextResponseWriter { sf = new SchemaField(field, ft); } - // Return only stored fields, unless an explicit field list is specified - if (returnOnlyStored && sf != null && !sf.stored()) { + // Return stored fields or useDocValuesAsStored=true fields, + // unless an explicit field list is specified + if (returnStoredOrDocValStored && !sf.stored() && !(sf.hasDocValues() && sf.useDocValuesAsStored())) { continue; } diff --git a/solr/core/src/test-files/solr/collection1/conf/schema12.xml b/solr/core/src/test-files/solr/collection1/conf/schema12.xml index e4c3ad2e2ca..6f33b41d549 100644 --- a/solr/core/src/test-files/solr/collection1/conf/schema12.xml +++ b/solr/core/src/test-files/solr/collection1/conf/schema12.xml @@ -715,6 +715,10 @@ + + + + diff --git a/solr/core/src/test/org/apache/solr/response/TestCSVResponseWriter.java b/solr/core/src/test/org/apache/solr/response/TestCSVResponseWriter.java index d10ea71fbdb..979279cc091 100644 --- a/solr/core/src/test/org/apache/solr/response/TestCSVResponseWriter.java +++ b/solr/core/src/test/org/apache/solr/response/TestCSVResponseWriter.java @@ -20,6 +20,8 @@ import java.io.StringWriter; import java.time.Instant; import java.util.Arrays; import java.util.Date; +import java.util.List; +import java.util.stream.Collectors; import org.apache.solr.SolrTestCaseJ4; import org.apache.solr.common.SolrDocument; @@ -43,6 +45,7 @@ public class TestCSVResponseWriter extends SolrTestCaseJ4 { assertU(adoc("id","3", "shouldbeunstored","foo")); assertU(adoc("id","4", "amount_c", "1.50,EUR")); assertU(adoc("id","5", "store", "12.434,-134.1")); + assertU(adoc("id","6", "pubyear_ii", "123", "store_iis", "12", "price_ff", "1.3")); assertU(commit()); } @@ -111,8 +114,9 @@ public class TestCSVResponseWriter extends SolrTestCaseJ4 { , h.query(req("q","id:[1 TO 2]", "wt","csv", "csv.header","false", "fl","id,v_ss,foo_s"))); // test SOLR-2970 not returning non-stored fields by default. Compare sorted list - assertEquals(sortHeader("amount_c,store,v_ss,foo_b,v2_ss,foo_f,foo_i,foo_d,foo_s,foo_dt,id,foo_l\n") - , sortHeader(h.query(req("q","id:3", "wt","csv", "csv.header","true", "fl","*", "rows","0")))); + assertEquals(sortHeader("amount_c,store,v_ss,foo_b,v2_ss,foo_f,foo_i,foo_d,foo_s,foo_dt,id,foo_l," + + "pubyear_ii,store_iis\n"), + sortHeader(h.query(req("q","id:3", "wt","csv", "csv.header","true", "fl","*", "rows","0")))); // now test SolrDocumentList @@ -229,7 +233,7 @@ public class TestCSVResponseWriter extends SolrTestCaseJ4 { //assertions specific to multiple pseudofields functions like abs, div, exists, etc.. (SOLR-5423) String funcText = h.query(req("q","*", "wt","csv", "csv.header","true", "fl","XXX:id,YYY:exists(foo_i),exists(shouldbeunstored)")); String[] funcLines = funcText.split("\n"); - assertEquals(6, funcLines.length); + assertEquals(7, funcLines.length); assertEquals("XXX,YYY,exists(shouldbeunstored)", funcLines[0] ); assertEquals("1,true,false", funcLines[1] ); assertEquals("3,false,true", funcLines[3] ); @@ -238,11 +242,48 @@ public class TestCSVResponseWriter extends SolrTestCaseJ4 { //assertions specific to single function without alias (SOLR-5423) String singleFuncText = h.query(req("q","*", "wt","csv", "csv.header","true", "fl","exists(shouldbeunstored),XXX:id")); String[] singleFuncLines = singleFuncText.split("\n"); - assertEquals(6, singleFuncLines.length); + assertEquals(7, singleFuncLines.length); assertEquals("exists(shouldbeunstored),XXX", singleFuncLines[0] ); assertEquals("false,1", singleFuncLines[1] ); assertEquals("true,3", singleFuncLines[3] ); } + + @Test + public void testForDVEnabledFields() throws Exception { + // for dv enabled and useDocValueAsStored=true + // returns pubyear_i, store_iis but not price_ff + String singleFuncText = h.query(req("q","id:6", "wt","csv", "csv.header","true")); + String sortedHeader = sortHeader("amount_c,store,v_ss,foo_b,v2_ss,foo_f,foo_i,foo_d,foo_s,foo_dt,id,foo_l," + + "pubyear_ii,store_iis"); + String[] singleFuncLines = singleFuncText.split("\n"); + assertEquals(2, singleFuncLines.length); + assertEquals(sortedHeader, sortHeader(singleFuncLines[0])); + List actualVal = Arrays.stream(singleFuncLines[1].trim().split(",")) + .filter(val -> !val.trim().isEmpty() && !val.trim().equals("\"\"")) + .collect(Collectors.toList()); + assertEquals(3, actualVal.size()); + assertTrue(actualVal.containsAll(Arrays.asList("6", "123", "12"))); + + // explicit fl=* + singleFuncText = h.query(req("q","id:6", "wt","csv", "csv.header","true", "fl", "*")); + sortedHeader = sortHeader("amount_c,store,v_ss,foo_b,v2_ss,foo_f,foo_i,foo_d,foo_s,foo_dt,id,foo_l," + + "pubyear_ii,store_iis"); + singleFuncLines = singleFuncText.split("\n"); + assertEquals(2, singleFuncLines.length); + assertEquals(sortedHeader, sortHeader(singleFuncLines[0])); + actualVal = Arrays.stream(singleFuncLines[1].trim().split(",")) + .filter(val -> !val.trim().isEmpty() && !val.trim().equals("\"\"")) + .collect(Collectors.toList()); + assertEquals(3, actualVal.size()); + assertTrue(actualVal.containsAll(Arrays.asList("6", "123", "12"))); + + // explicit price_ff + singleFuncText = h.query(req("q","id:6", "wt","csv", "csv.header","true", "fl", "price_ff")); + singleFuncLines = singleFuncText.split("\n"); + assertEquals(2, singleFuncLines.length); + assertEquals("price_ff", singleFuncLines[0]); + assertEquals("1.3", singleFuncLines[1]); + } /*