From b88669c60f2944d8916b05b4058e0e3069ef5e50 Mon Sep 17 00:00:00 2001 From: Steven Rowe Date: Tue, 25 Feb 2014 01:03:07 +0000 Subject: [PATCH] SOLR-5423: CSV output doesn't include function field git-svn-id: https://svn.apache.org/repos/asf/lucene/dev/trunk@1571505 13f79535-47bb-0310-9956-ffa450edef68 --- solr/CHANGES.txt | 3 ++ .../transform/ValueSourceAugmenter.java | 2 +- .../apache/solr/search/SolrReturnFields.java | 26 +++++++------- .../solr/response/TestCSVResponseWriter.java | 36 ++++++++++++++++++- .../apache/solr/search/ReturnFieldsTest.java | 17 +++++++-- 5 files changed, 66 insertions(+), 18 deletions(-) diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index ab3babafb6f..8653dfc32ec 100644 --- a/solr/CHANGES.txt +++ b/solr/CHANGES.txt @@ -89,6 +89,9 @@ Bug Fixes * SOLR-5647: The lib paths in example-schemaless will now load correctly. (Paul Westin via Shawn Heisey) +* SOLR-5423: CSV output doesn't include function field + (Arun Kumar, hossman, Steve Rowe) + Optimizations ---------------------- * SOLR-1880: Distributed Search skips GET_FIELDS stage if EXECUTE_QUERY diff --git a/solr/core/src/java/org/apache/solr/response/transform/ValueSourceAugmenter.java b/solr/core/src/java/org/apache/solr/response/transform/ValueSourceAugmenter.java index 71d8a354d54..ed594a7f70d 100644 --- a/solr/core/src/java/org/apache/solr/response/transform/ValueSourceAugmenter.java +++ b/solr/core/src/java/org/apache/solr/response/transform/ValueSourceAugmenter.java @@ -56,7 +56,7 @@ public class ValueSourceAugmenter extends DocTransformer @Override public String getName() { - return "function("+name+")"; + return name; } @Override 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 20a06786379..03289bac8e7 100644 --- a/solr/core/src/java/org/apache/solr/search/SolrReturnFields.java +++ b/solr/core/src/java/org/apache/solr/search/SolrReturnFields.java @@ -198,7 +198,7 @@ public class SolrReturnFields extends ReturnFields { start = sp.pos; } else { if (Character.isWhitespace(ch) || ch == ',' || ch==0) { - addField( field, key, augmenters, req ); + addField(field, key, augmenters, false); continue; } // an invalid field name... reset the position pointer to retry @@ -213,7 +213,7 @@ public class SolrReturnFields extends ReturnFields { ch = sp.ch(); if (field != null && (Character.isWhitespace(ch) || ch == ',' || ch==0)) { rename.add(field, key); - addField( field, key, augmenters, req ); + addField(field, key, augmenters, false); continue; } // an invalid field name... reset the position pointer to retry @@ -266,7 +266,7 @@ public class SolrReturnFields extends ReturnFields { else { // unknown transformer? } - addField(field, disp, augmenters, req); + addField(field, disp, augmenters, true); continue; } @@ -307,6 +307,7 @@ public class SolrReturnFields extends ReturnFields { assert parser.getLocalParams() != null; sp.pos = start + parser.localParamsEnd; } + funcStr = sp.val.substring(start, sp.pos); if (q instanceof FunctionQuery) { @@ -320,18 +321,12 @@ public class SolrReturnFields extends ReturnFields { if (localParams != null) { key = localParams.get("key"); } - if (key == null) { - // use the function name itself as the field name - key = sp.val.substring(start, sp.pos); - } } - if (key==null) { key = funcStr; } - okFieldNames.add( key ); - okFieldNames.add( funcStr ); + addField(funcStr, key, augmenters, true); augmenters.addTransformer( new ValueSourceAugmenter( key, parser, vs ) ); } catch (SyntaxError e) { @@ -341,7 +336,7 @@ public class SolrReturnFields extends ReturnFields { if (req.getSchema().getFieldOrNull(field) != null) { // OK, it was an oddly named field - fields.add(field); + addField(field, key, augmenters, false); if( key != null ) { rename.add(field, key); } @@ -358,7 +353,7 @@ public class SolrReturnFields extends ReturnFields { } } - private void addField(String field, String key, DocTransformers augmenters, SolrQueryRequest req) + private void addField(String field, String key, DocTransformers augmenters, boolean isPseudoField) { if(reqFieldNames==null) { reqFieldNames = new LinkedHashSet(); @@ -371,7 +366,12 @@ public class SolrReturnFields extends ReturnFields { reqFieldNames.add(key); } - fields.add(field); // need to put in the map to maintain order for things like CSVResponseWriter + 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); + } + okFieldNames.add( field ); okFieldNames.add( key ); // a valid field name 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 8a6b85a77e9..ecfc9bd462b 100644 --- a/solr/core/src/test/org/apache/solr/response/TestCSVResponseWriter.java +++ b/solr/core/src/test/org/apache/solr/response/TestCSVResponseWriter.java @@ -22,7 +22,6 @@ import org.apache.solr.common.SolrDocument; import org.apache.solr.common.SolrDocumentList; import org.apache.solr.common.util.DateUtil; import org.apache.solr.request.SolrQueryRequest; -import org.apache.solr.search.ReturnFields; import org.apache.solr.search.SolrReturnFields; import org.junit.*; @@ -192,6 +191,24 @@ public class TestCSVResponseWriter extends SolrTestCaseJ4 { "2,,\n", buf.toString()); + // Test function queries + rsp.setReturnFields( new SolrReturnFields("sum(1,1),id,exists(foo_i),div(9,1),foo_f", req) ); + buf = new StringWriter(); + w.write(buf, req, rsp); + assertEquals("\"sum(1,1)\",id,exists(foo_i),\"div(9,1)\",foo_f\n" + + "\"\",1,,,1.414\n" + + "\"\",2,,,\n", + buf.toString()); + + // Test transformers + rsp.setReturnFields( new SolrReturnFields("mydocid:[docid],[explain]", req) ); + buf = new StringWriter(); + w.write(buf, req, rsp); + assertEquals("mydocid,[explain]\n" + + "\"\",\n" + + "\"\",\n", + buf.toString()); + req.close(); } @@ -207,6 +224,23 @@ public class TestCSVResponseWriter extends SolrTestCaseJ4 { assertEquals(2, lines.length); assertEquals("XXX,YYY,FOO", lines[0] ); assertEquals("1,0,hi", lines[1] ); + + //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("XXX,YYY,exists(shouldbeunstored)", funcLines[0] ); + assertEquals("1,true,false", funcLines[1] ); + assertEquals("3,false,true", funcLines[3] ); + + + //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("exists(shouldbeunstored),XXX", singleFuncLines[0] ); + assertEquals("false,1", singleFuncLines[1] ); + assertEquals("true,3", singleFuncLines[3] ); } 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 311816f107d..994ce905cf8 100644 --- a/solr/core/src/test/org/apache/solr/search/ReturnFieldsTest.java +++ b/solr/core/src/test/org/apache/solr/search/ReturnFieldsTest.java @@ -181,25 +181,31 @@ public class ReturnFieldsTest extends SolrTestCaseJ4 { @Test public void testFunctions() { - ReturnFields rf = new SolrReturnFields( req("fl", "id sum(1,1)") ); + 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" ) ); - assertTrue( rf.getTransformer() instanceof ValueSourceAugmenter); - assertEquals("sum(1,1)", ((ValueSourceAugmenter) rf.getTransformer()).name); + assertTrue( rf.getTransformer() instanceof DocTransformers); + DocTransformers transformers = (DocTransformers)rf.getTransformer(); + assertEquals("exists(text)", transformers.getTransformer(0).getName()); + assertEquals("sum(1,1)", transformers.getTransformer(1).getName()); } @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()); 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()); @@ -207,6 +213,7 @@ public class ReturnFieldsTest extends SolrTestCaseJ4 { rf = new SolrReturnFields( req("fl", "[docid]") ); assertFalse( rf.wantsScore() ); + assertTrue(rf.wantsField("[docid]")); assertFalse( rf.wantsField( "id" ) ); assertFalse(rf.wantsField("xxx")); assertFalse(rf.wantsAllFields()); @@ -214,6 +221,7 @@ public class ReturnFieldsTest extends SolrTestCaseJ4 { 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()); @@ -221,6 +229,8 @@ public class ReturnFieldsTest extends SolrTestCaseJ4 { 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()); assertTrue( rf.getTransformer() instanceof DocTransformers); @@ -228,6 +238,7 @@ public class ReturnFieldsTest extends SolrTestCaseJ4 { rf = new SolrReturnFields( req("fl", "[xxxxx]") ); assertFalse( rf.wantsScore() ); + assertTrue(rf.wantsField("[xxxxx]")); assertFalse( rf.wantsField( "id" ) ); assertFalse(rf.wantsAllFields()); assertNull(rf.getTransformer());