[
https://issues.apache.org/jira/browse/SOLR-2798?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15097125#comment-15097125
]
Hoss Man commented on SOLR-2798:
--------------------------------
Demian: I haven't tested your code out, but at first glance it seems great to
me - if that's really all the changes that were needed to fix all existing
usages of the parse method it's much more straight forward then I thought and
i'm seriously embarrassed this bug has been arround so long
bq. 2.) Speaking of those new tests, I'm not sure if they are in the most
appropriate place, or if we even want to keep the map-oriented ones in the long
term since those methods should be deprecated.
Considering how simple/straight forward this is, it's probably fair game to be
backported to the 5x branch, which means having those tests & deprecated
methods in place is a good idea -- that way we can leave the deprecated method
(with the tests you add) in the 5x branch, and only delete it from trunk (so if
someone has a custom plugin it will still compile in 5x w/o any changes)
bq. 3.) Speaking of deprecated methods, I didn't do anything to flag them – not
sure what the convention is in this project.
standard java deprecations, here's an existing example pulled at random from
the code...
{code}
/**
* @param aliasName the alias name
* @deprecated use {@link #setAliasName(String)} instead
*/
@Deprecated
public void setCollectionName(String aliasName) {
this.aliasName = aliasName;
}
{code}
bq. Not sure if there are more targeted tests that need to be written to truly
confirm this.
The one thing I would really like to see is some additional higher level tests
showing that these underlying changes are having the desired/expected impact on
the user facing issues.
For example: the summary report of this has a dismax example with two "bq"
localparams. With your patch, we should be able to add a test to
QueryEqualityTest showing that query is parsed equivalently to a request with 2
top level "bq" params. I can't remember if i made that example up or if it
came from your original email, but if you have a different example of how this
bug is impacting you at a higher level, let's add some explicit tests of that
as well. w/o higher level tests like this, it won't necessarily be obvious to
anyone reviewing/refactoring the code in the future why those tests exist --
they show *what* output comes from that input, but not *why* that output is
important.
Make sense?
> Local Param parsing does not support multivalued params
> -------------------------------------------------------
>
> Key: SOLR-2798
> URL: https://issues.apache.org/jira/browse/SOLR-2798
> Project: Solr
> Issue Type: Bug
> Reporter: Hoss Man
> Assignee: Anshum Gupta
>
> As noted by Demian on the solr-user mailing list, Local Param parsing seems
> to use a "last one wins" approach when parsing multivalued params.
> In this example, the value of "111" is completely ignored:
> {code}
> http://localhost:8983/solr/select?debug=query&q={!dismax%20bq=111%20bq=222}foo
> {code}
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
