munendrasn commented on a change in pull request #1371:
URL: https://github.com/apache/lucene-solr/pull/1371#discussion_r491312573
##########
File path:
solr/core/src/java/org/apache/solr/search/CollapsingQParserPlugin.java
##########
@@ -188,6 +206,11 @@ public int hashCode() {
return 17 * (31 + selectorText.hashCode()) * (31 + type.hashCode());
}
+ @Override
+ public String toString(){
+ return "groupHeadSelectorText=" +this.selectorText + ",
groupHeadSelectorType=" +this.type;
Review comment:
would it be better to include the class name too here?
```suggestion
return "GroupHeadSelector[selectorText=" +this.selectorText + ",
type=" +this.type + "]";
```
##########
File path:
solr/core/src/java/org/apache/solr/search/CollapsingQParserPlugin.java
##########
@@ -128,6 +128,28 @@ field collapsing (with ngroups) when the number of
distinct groups
public static final String HINT_TOP_FC = "top_fc";
public static final String HINT_MULTI_DOCVALUES = "multi_docvalues";
+ public enum NullPolicy {
+ IGNORE("ignore", 0),
Review comment:
I agree with @madrob about removing the unused declarations but as
access the public for these variables. I think we should deprecate it in this
PR(for 8x) and remove it in different PR
wdyt?
##########
File path:
solr/core/src/java/org/apache/solr/search/CollapsingQParserPlugin.java
##########
@@ -279,7 +299,12 @@ public int getCost() {
}
public String toString(String s) {
- return s;
+ return "{!collapse field=" + this.collapseField +
+ ", nullPolicy=" + this.nullPolicy.getName() + ", " +
+ this.groupHeadSelector.toString() +
+ (hint == null ? "": ", hint=" + this.hint) +
+ ", size=" + this.size
+ + "}";
Review comment:
The above suggestion might lead cause this in debug response
```
CollapsingPostFilter([CollapsingPostFilter field=id, nullPolicy=expand,
GroupHeadSelector[selectorText=_version_ asc, type=SORT], hint=top_fc,
size=5000])
```
This is caused due to how Query is converted to String by `QueryParsing`
Class but I think that should be handled separately as issue exists for
different query type
https://github.com/apache/lucene-solr/blob/ab3f1f0d1d0fcdbe58958197b11ba3837795a24a/solr/core/src/java/org/apache/solr/search/QueryParsing.java#L336
##########
File path:
solr/core/src/java/org/apache/solr/search/CollapsingQParserPlugin.java
##########
@@ -279,7 +299,12 @@ public int getCost() {
}
public String toString(String s) {
- return s;
+ return "{!collapse field=" + this.collapseField +
+ ", nullPolicy=" + this.nullPolicy.getName() + ", " +
+ this.groupHeadSelector.toString() +
+ (hint == null ? "": ", hint=" + this.hint) +
+ ", size=" + this.size
+ + "}";
Review comment:
`{!collapse` specifies the query parser, I think we shouldn't include it
in toString()
```suggestion
return "CollapsingPostFilter[field=" + this.collapseField +
", nullPolicy=" + this.nullPolicy.getName() + ", " +
this.groupHeadSelector +
(hint == null ? "": ", hint=" + this.hint) +
", size=" + this.size
+ "]";
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
