From dff3579da3e9b27c4478343286aeaa6a6a0da83b Mon Sep 17 00:00:00 2001 From: Steven Rowe Date: Tue, 3 Dec 2013 15:22:59 +0000 Subject: [PATCH] SOLR-5354: applying hoss's patch to fix function edge case in distributed sort git-svn-id: https://svn.apache.org/repos/asf/lucene/dev/trunk@1547430 13f79535-47bb-0310-9956-ffa450edef68 --- .../handler/component/QueryComponent.java | 63 +++++++++++++------ .../component/QueryElevationComponent.java | 47 ++++++++++---- .../solr/search/LuceneQParserPlugin.java | 6 +- .../java/org/apache/solr/search/QParser.java | 10 +-- .../org/apache/solr/search/QueryParsing.java | 52 ++++++++++----- .../java/org/apache/solr/search/SortSpec.java | 53 ++++++++++++++-- .../org/apache/solr/util/SolrPluginUtils.java | 4 +- .../collection1/conf/schema-custom-field.xml | 3 + ...stributedQueryComponentCustomSortTest.java | 8 ++- .../apache/solr/search/QueryParsingTest.java | 49 ++++++++++++++- .../test/org/apache/solr/search/TestSort.java | 11 +++- 11 files changed, 239 insertions(+), 67 deletions(-) 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 ca072e18dec..55ff8ad913f 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 @@ -190,7 +190,7 @@ public class QueryComponent extends SearchComponent // groupSort defaults to sort String groupSortStr = params.get(GroupParams.GROUP_SORT); //TODO: move weighting of sort - Sort sortWithinGroup = groupSortStr == null ? groupSort : searcher.weightSort(QueryParsing.parseSort(groupSortStr, req)); + Sort sortWithinGroup = groupSortStr == null ? groupSort : searcher.weightSort(QueryParsing.parseSortSpec(groupSortStr, req).getSort()); if (sortWithinGroup == null) { sortWithinGroup = Sort.RELEVANCE; } @@ -454,8 +454,6 @@ public class QueryComponent extends SearchComponent // take the documents given and re-derive the sort values. boolean fsv = req.getParams().getBool(ResponseBuilder.FIELD_SORT_VALUES,false); if(fsv){ - Sort sort = searcher.weightSort(rb.getSortSpec().getSort()); - SortField[] sortFields = sort==null ? new SortField[]{SortField.FIELD_SCORE} : sort.getSort(); NamedList sortVals = new NamedList(); // order is important for the sort fields IndexReaderContext topReaderContext = searcher.getTopReaderContext(); List leaves = topReaderContext.leaves(); @@ -477,18 +475,22 @@ public class QueryComponent extends SearchComponent } Arrays.sort(sortedIds); + SortSpec sortSpec = rb.getSortSpec(); + Sort sort = searcher.weightSort(sortSpec.getSort()); + SortField[] sortFields = sort==null ? new SortField[]{SortField.FIELD_SCORE} : sort.getSort(); + List schemaFields = sortSpec.getSchemaFields(); + + for (int fld = 0; fld < schemaFields.size(); fld++) { + SchemaField schemaField = schemaFields.get(fld); + FieldType ft = null == schemaField? null : schemaField.getType(); + SortField sortField = sortFields[fld]; - for (SortField sortField: sortFields) { SortField.Type type = sortField.getType(); + // :TODO: would be simpler to always serialize every position of SortField[] if (type==SortField.Type.SCORE || type==SortField.Type.DOC) continue; FieldComparator comparator = null; - - String fieldname = sortField.getField(); - FieldType ft = fieldname==null ? null : searcher.getSchema().getFieldTypeNoEx(fieldname); - Object[] vals = new Object[nDocs]; - int lastIdx = -1; int idx = 0; @@ -518,7 +520,7 @@ public class QueryComponent extends SearchComponent vals[position] = val; } - sortVals.add(fieldname, vals); + sortVals.add(sortField.getField(), vals); } rsp.add("sort_values", sortVals); @@ -865,7 +867,7 @@ public class QueryComponent extends SearchComponent } } - shardDoc.sortFieldValues = unmarshalSortValues(sortFieldValues, schema); + shardDoc.sortFieldValues = unmarshalSortValues(ss, sortFieldValues, schema); queue.insertWithOverflow(shardDoc); } // end for-each-doc-in-response @@ -907,22 +909,43 @@ public class QueryComponent extends SearchComponent } } - private NamedList unmarshalSortValues(NamedList sortFieldValues, IndexSchema schema) { + private NamedList unmarshalSortValues(SortSpec sortSpec, + NamedList sortFieldValues, + IndexSchema schema) { NamedList unmarshalledSortValsPerField = new NamedList(); - for (int fieldNum = 0 ; fieldNum < sortFieldValues.size() ; ++fieldNum) { - String fieldName = sortFieldValues.getName(fieldNum); - SchemaField field = schema.getFieldOrNull(fieldName); - List sortVals = (List)sortFieldValues.getVal(fieldNum); - if (null == field) { - unmarshalledSortValsPerField.add(fieldName, sortVals); + + if (0 == sortFieldValues.size()) return unmarshalledSortValsPerField; + + List schemaFields = sortSpec.getSchemaFields(); + SortField[] sortFields = sortSpec.getSort().getSort(); + + int marshalledFieldNum = 0; + for (int sortFieldNum = 0; sortFieldNum < sortFields.length; sortFieldNum++) { + final SortField sortField = sortFields[sortFieldNum]; + final SortField.Type type = sortField.getType(); + + // :TODO: would be simpler to always serialize every position of SortField[] + if (type==SortField.Type.SCORE || type==SortField.Type.DOC) continue; + + final String sortFieldName = sortField.getField(); + final String valueFieldName = sortFieldValues.getName(marshalledFieldNum); + assert sortFieldName.equals(valueFieldName) + : "sortFieldValues name key does not match expected SortField.getField"; + + List sortVals = (List)sortFieldValues.getVal(marshalledFieldNum); + + final SchemaField schemaField = schemaFields.get(sortFieldNum); + if (null == schemaField) { + unmarshalledSortValsPerField.add(sortField.getField(), sortVals); } else { - FieldType fieldType = field.getType(); + FieldType fieldType = schemaField.getType(); List unmarshalledSortVals = new ArrayList(); for (Object sortVal : sortVals) { unmarshalledSortVals.add(fieldType.unmarshalSortValue(sortVal)); } - unmarshalledSortValsPerField.add(fieldName, unmarshalledSortVals); + unmarshalledSortValsPerField.add(sortField.getField(), unmarshalledSortVals); } + marshalledFieldNum++; } return unmarshalledSortValsPerField; } diff --git a/solr/core/src/java/org/apache/solr/handler/component/QueryElevationComponent.java b/solr/core/src/java/org/apache/solr/handler/component/QueryElevationComponent.java index b0caa898c28..15ca4e695fa 100644 --- a/solr/core/src/java/org/apache/solr/handler/component/QueryElevationComponent.java +++ b/solr/core/src/java/org/apache/solr/handler/component/QueryElevationComponent.java @@ -419,16 +419,16 @@ public class QueryElevationComponent extends SearchComponent implements SolrCore // insert documents in their proper place SortSpec sortSpec = rb.getSortSpec(); if (sortSpec.getSort() == null) { - sortSpec.setSort(new Sort(new SortField[]{ - new SortField("_elevate_", comparator, true), - new SortField(null, SortField.Type.SCORE, false) - })); + sortSpec.setSortAndFields(new Sort(new SortField[]{ + new SortField("_elevate_", comparator, true), + new SortField(null, SortField.Type.SCORE, false) + }), + Arrays.asList(new SchemaField[2])); } else { // Check if the sort is based on score - SortField[] current = sortSpec.getSort().getSort(); - Sort modified = this.modifySort(current, force, comparator); - if(modified != null) { - sortSpec.setSort(modified); + SortSpec modSortSpec = this.modifySortSpec(sortSpec, force, comparator); + if (null != modSortSpec) { + rb.setSortSpec(modSortSpec); } } @@ -470,22 +470,43 @@ public class QueryElevationComponent extends SearchComponent implements SolrCore } private Sort modifySort(SortField[] current, boolean force, ElevationComparatorSource comparator) { + SortSpec tmp = new SortSpec(new Sort(current), Arrays.asList(new SchemaField[current.length])); + tmp = modifySortSpec(tmp, force, comparator); + return null == tmp ? null : tmp.getSort(); + } + + private SortSpec modifySortSpec(SortSpec current, boolean force, ElevationComparatorSource comparator) { boolean modify = false; - ArrayList sorts = new ArrayList(current.length + 1); + SortField[] currentSorts = current.getSort().getSort(); + List currentFields = current.getSchemaFields(); + + ArrayList sorts = new ArrayList(currentSorts.length + 1); + List fields = new ArrayList(currentFields.size() + 1); + // Perhaps force it to always sort by score - if (force && current[0].getType() != SortField.Type.SCORE) { + if (force && currentSorts[0].getType() != SortField.Type.SCORE) { sorts.add(new SortField("_elevate_", comparator, true)); + fields.add(null); modify = true; } - for (SortField sf : current) { + for (int i = 0; i < currentSorts.length; i++) { + SortField sf = currentSorts[i]; if (sf.getType() == SortField.Type.SCORE) { sorts.add(new SortField("_elevate_", comparator, !sf.getReverse())); + fields.add(null); modify = true; } sorts.add(sf); + fields.add(currentFields.get(i)); } - - return modify ? new Sort(sorts.toArray(new SortField[sorts.size()])) : null; + if (modify) { + SortSpec newSpec = new SortSpec(new Sort(sorts.toArray(new SortField[sorts.size()])), + fields); + newSpec.setOffset(current.getOffset()); + newSpec.setCount(current.getCount()); + return newSpec; + } + return null; } @Override diff --git a/solr/core/src/java/org/apache/solr/search/LuceneQParserPlugin.java b/solr/core/src/java/org/apache/solr/search/LuceneQParserPlugin.java index 00c4424b2f1..8ea19187328 100644 --- a/solr/core/src/java/org/apache/solr/search/LuceneQParserPlugin.java +++ b/solr/core/src/java/org/apache/solr/search/LuceneQParserPlugin.java @@ -86,9 +86,9 @@ class OldLuceneQParser extends LuceneQParser { public SortSpec getSort(boolean useGlobal) throws SyntaxError { SortSpec sort = super.getSort(useGlobal); if (sortStr != null && sortStr.length()>0 && sort.getSort()==null) { - Sort oldSort = QueryParsing.parseSort(sortStr, getReq()); - if( oldSort != null ) { - sort.sort = oldSort; + SortSpec oldSort = QueryParsing.parseSortSpec(sortStr, getReq()); + if( oldSort.getSort() != null ) { + sort.setSortAndFields(oldSort.getSort(), oldSort.getSchemaFields()); } } return sort; diff --git a/solr/core/src/java/org/apache/solr/search/QParser.java b/solr/core/src/java/org/apache/solr/search/QParser.java index 16db2d36b5f..e39d424bd9b 100644 --- a/solr/core/src/java/org/apache/solr/search/QParser.java +++ b/solr/core/src/java/org/apache/solr/search/QParser.java @@ -276,11 +276,11 @@ public abstract class QParser { int start = startS != null ? Integer.parseInt(startS) : 0; int rows = rowsS != null ? Integer.parseInt(rowsS) : 10; - Sort sort = null; - if( sortStr != null ) { - sort = QueryParsing.parseSort(sortStr, req); - } - return new SortSpec( sort, start, rows ); + SortSpec sort = QueryParsing.parseSortSpec(sortStr, req); + + sort.setOffset(start); + sort.setCount(rows); + return sort; } public String[] getDefaultHighlightFields() { diff --git a/solr/core/src/java/org/apache/solr/search/QueryParsing.java b/solr/core/src/java/org/apache/solr/search/QueryParsing.java index 1cdb4c4e3f7..75d5b527d06 100644 --- a/solr/core/src/java/org/apache/solr/search/QueryParsing.java +++ b/solr/core/src/java/org/apache/solr/search/QueryParsing.java @@ -43,6 +43,7 @@ import org.apache.solr.schema.FieldType; import org.apache.solr.schema.IndexSchema; import org.apache.solr.schema.SchemaField; import java.io.IOException; +import java.util.Collections; import java.util.ArrayList; import java.util.HashMap; import java.util.List; @@ -219,16 +220,24 @@ public class QueryParsing { return new MapSolrParams(localParams); } + /** + * Returns the Sort object represented by the string, or null if default sort + * by score descending should be used. + * @see #parseSortSpec + * @deprecated use {@link #parseSortSpec} + */ + @Deprecated + public static Sort parseSort(String sortSpec, SolrQueryRequest req) { + return parseSortSpec(sortSpec, req).getSort(); + } /** - * Returns null if the sortSpec is the standard sort desc. - *

*

* The form of the sort specification string currently parsed is: *

* 
    * SortSpec ::= SingleSort [, SingleSort]*
-   * SingleSort ::= <fieldname> SortDirection
+   * SingleSort ::= <fieldname|function> SortDirection
    * SortDirection ::= top | desc | bottom | asc
    *
* Examples: @@ -239,10 +248,15 @@ public class QueryParsing { * height desc,weight desc #sort by height descending, and use weight descending to break any ties * height desc,weight asc #sort by height descending, using weight ascending as a tiebreaker * + * @return a SortSpec object populated with the appropriate Sort (which may be null if + * default score sort is used) and SchemaFields (where applicable) using + * hardcoded default count & offset values. */ - public static Sort parseSort(String sortSpec, SolrQueryRequest req) { - if (sortSpec == null || sortSpec.length() == 0) return null; - List lst = new ArrayList(4); + public static SortSpec parseSortSpec(String sortSpec, SolrQueryRequest req) { + if (sortSpec == null || sortSpec.length() == 0) return newEmptySortSpec(); + + List sorts = new ArrayList(4); + List fields = new ArrayList(4); try { @@ -299,10 +313,11 @@ public class QueryParsing { if (null != top) { // we have a Query and a valid direction if (q instanceof FunctionQuery) { - lst.add(((FunctionQuery)q).getValueSource().getSortField(top)); + sorts.add(((FunctionQuery)q).getValueSource().getSortField(top)); } else { - lst.add((new QueryValueSource(q, 0.0f)).getSortField(top)); + sorts.add((new QueryValueSource(q, 0.0f)).getSortField(top)); } + fields.add(null); continue; } } catch (Exception e) { @@ -327,12 +342,14 @@ public class QueryParsing { if (SCORE.equals(field)) { if (top) { - lst.add(SortField.FIELD_SCORE); + sorts.add(SortField.FIELD_SCORE); } else { - lst.add(new SortField(null, SortField.Type.SCORE, true)); + sorts.add(new SortField(null, SortField.Type.SCORE, true)); } + fields.add(null); } else if (DOCID.equals(field)) { - lst.add(new SortField(null, SortField.Type.DOC, top)); + sorts.add(new SortField(null, SortField.Type.DOC, top)); + fields.add(null); } else { // try to find the field SchemaField sf = req.getSchema().getFieldOrNull(field); @@ -348,7 +365,8 @@ public class QueryParsing { (SolrException.ErrorCode.BAD_REQUEST, "sort param field can't be found: " + field); } - lst.add(sf.getSortField(top)); + sorts.add(sf.getSortField(top)); + fields.add(sf); } } @@ -358,13 +376,17 @@ public class QueryParsing { // normalize a sort on score desc to null - if (lst.size()==1 && lst.get(0) == SortField.FIELD_SCORE) { - return null; + if (sorts.size()==1 && sorts.get(0) == SortField.FIELD_SCORE) { + return newEmptySortSpec(); } - return new Sort(lst.toArray(new SortField[lst.size()])); + Sort s = new Sort(sorts.toArray(new SortField[sorts.size()])); + return new SortSpec(s, fields); } + private static SortSpec newEmptySortSpec() { + return new SortSpec(null, Collections.emptyList()); + } /////////////////////////// diff --git a/solr/core/src/java/org/apache/solr/search/SortSpec.java b/solr/core/src/java/org/apache/solr/search/SortSpec.java index 4573bf691bd..6655aa67a79 100644 --- a/solr/core/src/java/org/apache/solr/search/SortSpec.java +++ b/solr/core/src/java/org/apache/solr/search/SortSpec.java @@ -20,29 +20,63 @@ package org.apache.solr.search; import org.apache.lucene.search.Sort; import org.apache.lucene.search.SortField; +import org.apache.solr.schema.SchemaField; + +import java.util.Arrays; +import java.util.List; +import java.util.ArrayList; +import java.util.Collections; /*** * SortSpec encapsulates a Lucene Sort and a count of the number of documents * to return. */ public class SortSpec { - Sort sort; - int num; - int offset; + private Sort sort; + private List fields; + private int num = 10; + private int offset = 0; + public SortSpec(Sort sort, List fields) { + setSortAndFields(sort, fields); + } + public SortSpec(Sort sort, SchemaField[] fields) { + setSortAndFields(sort, Arrays.asList(fields)); + } + + /** @deprecated Specify both Sort and SchemaField[] when constructing */ + @Deprecated public SortSpec(Sort sort, int num) { this(sort,0,num); } + /** @deprecated Specify both Sort and SchemaField[] when constructing */ + @Deprecated public SortSpec(Sort sort, int offset, int num) { - this.sort=sort; + setSort(sort); this.offset=offset; this.num=num; } + /** @deprecated use {@link #setSortAndFields} */ + @Deprecated public void setSort( Sort s ) { sort = s; + fields = Collections.unmodifiableList(Arrays.asList(new SchemaField[s.getSort().length])); + } + + /** + * the specified SchemaFields must correspond one to one with the Sort's SortFields, + * using null where appropriate. + */ + public void setSortAndFields( Sort s, List fields ) + { + + assert null == s || s.getSort().length == fields.size() + : "SortFields and SchemaFields do not have equal lengths"; + this.sort = s; + this.fields = Collections.unmodifiableList(fields); } public static boolean includesScore(Sort sort) { @@ -63,10 +97,20 @@ public class SortSpec */ public Sort getSort() { return sort; } + /** + * Gets the Solr SchemaFields that correspond to each of the SortFields used + * in this sort. The array may contain null if a SortField doesn't correspond directly + * to a SchemaField (ie: score, lucene docid, custom function sorting, etc...) + * + * @return an immutable list, may be empty if getSort is null + */ + public List getSchemaFields() { return fields; } + /** * Offset into the list of results. */ public int getOffset() { return offset; } + public void setOffset(int offset) { this.offset = offset; } /** * Gets the number of documents to return after sorting. @@ -74,6 +118,7 @@ public class SortSpec * @return number of docs to return, or -1 for no cut off (just sort) */ public int getCount() { return num; } + public void setCount(int count) { this.num = count; } @Override public String toString() { diff --git a/solr/core/src/java/org/apache/solr/util/SolrPluginUtils.java b/solr/core/src/java/org/apache/solr/util/SolrPluginUtils.java index 8e9e405f404..af7a9633a76 100644 --- a/solr/core/src/java/org/apache/solr/util/SolrPluginUtils.java +++ b/solr/core/src/java/org/apache/solr/util/SolrPluginUtils.java @@ -430,7 +430,7 @@ public class SolrPluginUtils { // we can use the Lucene sort ability. Sort sort = null; if (commands.size() >= 2) { - sort = QueryParsing.parseSort(commands.get(1), req); + sort = QueryParsing.parseSortSpec(commands.get(1), req).getSort(); } DocList results = req.getSearcher().getDocList(query,(DocSet)null, sort, start, limit); @@ -825,7 +825,7 @@ public class SolrPluginUtils { SolrException sortE = null; Sort ss = null; try { - ss = QueryParsing.parseSort(sort, req); + ss = QueryParsing.parseSortSpec(sort, req).getSort(); } catch (SolrException e) { sortE = e; } diff --git a/solr/core/src/test-files/solr/collection1/conf/schema-custom-field.xml b/solr/core/src/test-files/solr/collection1/conf/schema-custom-field.xml index de206bce0bb..602527541c9 100644 --- a/solr/core/src/test-files/solr/collection1/conf/schema-custom-field.xml +++ b/solr/core/src/test-files/solr/collection1/conf/schema-custom-field.xml @@ -36,6 +36,9 @@ + + + text id diff --git a/solr/core/src/test/org/apache/solr/handler/component/DistributedQueryComponentCustomSortTest.java b/solr/core/src/test/org/apache/solr/handler/component/DistributedQueryComponentCustomSortTest.java index 8a4b9735e0b..b19b486ea7a 100644 --- a/solr/core/src/test/org/apache/solr/handler/component/DistributedQueryComponentCustomSortTest.java +++ b/solr/core/src/test/org/apache/solr/handler/component/DistributedQueryComponentCustomSortTest.java @@ -47,7 +47,9 @@ public class DistributedQueryComponentCustomSortTest extends BaseDistributedSear public void doTest() throws Exception { del("*:*"); - index(id, "1", "text", "a", "payload", ByteBuffer.wrap(new byte[] { 0x12, 0x62, 0x15 })); // 2 + index(id, "1", "text", "a", "payload", ByteBuffer.wrap(new byte[] { 0x12, 0x62, 0x15 }), // 2 + // quick check to prove "*" dynamicField hasn't been broken by somebody mucking with schema + "asdfasdf_field_should_match_catchall_dynamic_field_adsfasdf", "value"); index(id, "2", "text", "b", "payload", ByteBuffer.wrap(new byte[] { 0x25, 0x21, 0x16 })); // 5 index(id, "3", "text", "a", "payload", ByteBuffer.wrap(new byte[] { 0x35, 0x32, 0x58 })); // 8 index(id, "4", "text", "b", "payload", ByteBuffer.wrap(new byte[] { 0x25, 0x21, 0x15 })); // 4 @@ -90,6 +92,10 @@ public class DistributedQueryComponentCustomSortTest extends BaseDistributedSear rsp = query("q", "text:d", "fl", "id", "sort", "payload desc", "rows", "20"); assertFieldValues(rsp.getResults(), id, 11, 13, 12); + // sanity check function sorting + rsp = query("q", "id:[1 TO 10]", "fl", "id", "rows", "20", + "sort", "abs(sub(5,id)) asc, id desc"); + assertFieldValues(rsp.getResults(), id, 5 , 6,4 , 7,3 , 8,2 , 9,1 , 10 ); // Add two more docs with same payload as in doc #4 index(id, "14", "text", "b", "payload", ByteBuffer.wrap(new byte[] { 0x25, 0x21, 0x15 })); diff --git a/solr/core/src/test/org/apache/solr/search/QueryParsingTest.java b/solr/core/src/test/org/apache/solr/search/QueryParsingTest.java index ae9fe340092..482c02e79ea 100644 --- a/solr/core/src/test/org/apache/solr/search/QueryParsingTest.java +++ b/solr/core/src/test/org/apache/solr/search/QueryParsingTest.java @@ -20,11 +20,15 @@ import org.apache.lucene.search.Query; import org.apache.lucene.search.Sort; import org.apache.lucene.search.SortField; import org.apache.solr.SolrTestCaseJ4; +import org.apache.solr.schema.SchemaField; +import org.apache.solr.search.SortSpec; import org.apache.solr.common.SolrException; import org.apache.solr.request.SolrQueryRequest; import org.junit.BeforeClass; import org.junit.Test; +import java.util.List; + /** * * @@ -72,22 +76,48 @@ public class QueryParsingTest extends SolrTestCaseJ4 { @Test public void testSort() throws Exception { Sort sort; + SortSpec spec; SolrQueryRequest req = req(); sort = QueryParsing.parseSort("score desc", req); assertNull("sort", sort);//only 1 thing in the list, no Sort specified + spec = QueryParsing.parseSortSpec("score desc", req); + assertNotNull("spec", spec); + assertNull(spec.getSort()); + assertNotNull(spec.getSchemaFields()); + assertEquals(0, spec.getSchemaFields().size()); + // SOLR-4458 - using different case variations of asc and desc sort = QueryParsing.parseSort("score aSc", req); SortField[] flds = sort.getSort(); assertEquals(flds[0].getType(), SortField.Type.SCORE); assertTrue(flds[0].getReverse()); + spec = QueryParsing.parseSortSpec("score aSc", req); + flds = spec.getSort().getSort(); + assertEquals(1, flds.length); + assertEquals(flds[0].getType(), SortField.Type.SCORE); + assertTrue(flds[0].getReverse()); + assertEquals(1, spec.getSchemaFields().size()); + assertNull(spec.getSchemaFields().get(0)); + sort = QueryParsing.parseSort("weight dEsC", req); flds = sort.getSort(); assertEquals(flds[0].getType(), SortField.Type.FLOAT); assertEquals(flds[0].getField(), "weight"); assertEquals(flds[0].getReverse(), true); + + spec = QueryParsing.parseSortSpec("weight dEsC", req); + flds = spec.getSort().getSort(); + assertEquals(1, flds.length); + assertEquals(flds[0].getType(), SortField.Type.FLOAT); + assertEquals(flds[0].getField(), "weight"); + assertEquals(flds[0].getReverse(), true); + assertEquals(1, spec.getSchemaFields().size()); + assertNotNull(spec.getSchemaFields().get(0)); + assertEquals("weight", spec.getSchemaFields().get(0).getName()); + sort = QueryParsing.parseSort("weight desc,bday ASC", req); flds = sort.getSort(); assertEquals(flds[0].getType(), SortField.Type.FLOAT); @@ -147,20 +177,29 @@ public class QueryParsingTest extends SolrTestCaseJ4 { //Not thrilled about the fragility of string matching here, but... //the value sources get wrapped, so the out field is different than the input assertEquals(flds[0].getField(), "pow(float(weight),const(2.0))"); + + spec = QueryParsing.parseSortSpec("pow(weight, 2.0) desc, weight desc, bday asc", req); + flds = spec.getSort().getSort(); + List schemaFlds = spec.getSchemaFields(); + assertEquals(3, flds.length); + assertEquals(3, schemaFlds.size()); - sort = QueryParsing.parseSort("pow(weight, 2.0) desc, weight desc, bday asc", req); - flds = sort.getSort(); assertEquals(flds[0].getType(), SortField.Type.REWRITEABLE); - //Not thrilled about the fragility of string matching here, but... //the value sources get wrapped, so the out field is different than the input assertEquals(flds[0].getField(), "pow(float(weight),const(2.0))"); + assertNull(schemaFlds.get(0)); assertEquals(flds[1].getType(), SortField.Type.FLOAT); assertEquals(flds[1].getField(), "weight"); + assertNotNull(schemaFlds.get(1)); + assertEquals("weight", schemaFlds.get(1).getName()); + assertEquals(flds[2].getField(), "bday"); assertEquals(flds[2].getType(), SortField.Type.LONG); + assertNotNull(schemaFlds.get(2)); + assertEquals("bday", schemaFlds.get(2).getName()); //handles trailing commas sort = QueryParsing.parseSort("weight desc,", req); @@ -178,6 +217,10 @@ public class QueryParsingTest extends SolrTestCaseJ4 { sort = QueryParsing.parseSort("", req); assertNull(sort); + spec = QueryParsing.parseSortSpec("", req); + assertNotNull(spec); + assertNull(spec.getSort()); + req.close(); } diff --git a/solr/core/src/test/org/apache/solr/search/TestSort.java b/solr/core/src/test/org/apache/solr/search/TestSort.java index 1f391c5e762..5be8848807e 100644 --- a/solr/core/src/test/org/apache/solr/search/TestSort.java +++ b/solr/core/src/test/org/apache/solr/search/TestSort.java @@ -36,6 +36,7 @@ import org.apache.lucene.util.Bits; import org.apache.lucene.util.OpenBitSet; import org.apache.lucene.util._TestUtil; import org.apache.solr.SolrTestCaseJ4; +import org.apache.solr.schema.SchemaField; import org.apache.solr.request.SolrQueryRequest; import org.junit.BeforeClass; @@ -114,13 +115,18 @@ public class TestSort extends SolrTestCaseJ4 { } input.deleteCharAt(input.length()-1); SortField[] sorts = null; + List fields = null; try { - sorts = QueryParsing.parseSort(input.toString(), req).getSort(); + SortSpec spec = QueryParsing.parseSortSpec(input.toString(), req); + sorts = spec.getSort().getSort(); + fields = spec.getSchemaFields(); } catch (RuntimeException e) { throw new RuntimeException("Failed to parse sort: " + input, e); } assertEquals("parsed sorts had unexpected size", names.length, sorts.length); + assertEquals("parsed sort schema fields had unexpected size", + names.length, fields.size()); for (int j = 0; j < names.length; j++) { assertEquals("sorts["+j+"] had unexpected reverse: " + input, reverse[j], sorts[j].getReverse()); @@ -144,6 +150,9 @@ public class TestSort extends SolrTestCaseJ4 { assertEquals("sorts["+j+"] ("+type.toString()+ ") had unexpected field in: " + input, names[j], sorts[j].getField()); + assertEquals("fields["+j+"] ("+type.toString()+ + ") had unexpected name in: " + input, + names[j], fields.get(j).getName()); } } }