You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@lucene.apache.org by "Jason Gerlowski (JIRA)" <ji...@apache.org> on 2019/04/08 02:16:00 UTC

[jira] [Comment Edited] (SOLR-13318) JsonFacetingResponse classes should record provide access to count fields as longs

    [ https://issues.apache.org/jira/browse/SOLR-13318?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16812026#comment-16812026 ] 

Jason Gerlowski edited comment on SOLR-13318 at 4/8/19 2:15 AM:
----------------------------------------------------------------

Thanks for the patch Munendra.  I agree with the comments you left in the patch - we'll have to change the types of our instance vars where you pointed them out, as well as some method return types.

You're also right that changing those return types would run afoul of our back-compat policy.  We've got a few options:
1. Only fix this in master, and requires users wait until 9.0 for the fix.
2. Keep the int-returning methods and leave their signatures as-is, while introducing near-duplicate methods that return the correct types.  Deprecate the int-returning versions with references to these new methods.
3. Make a special request on the dev-list to exempt this change from our back-compat policy.  I think there's an "ok" argument for doing that here...our backcompat policy is intended to help us serve our users.  Slavishly maintaining backcompat and keeping the buggy versions of these methods around meets the letter of that law, but totally fails to meet the spirit.  We're not doing our users any favors by keeping these methods around.

So to answer your question ("Should I create different patches"), I'm not entirely decided.  I might appeal to the dev-list to see if anyone cares in this case.  But most likely I'll pursue option 2.  In that case, I'll build off of your patch and introduce the duplicate methods and deprecation warnings.  I'll try to have something ready on this sometime this week.


was (Author: gerlowskija):
Thanks for the patch Munendra.  I agree with the comments you left in the patch - we'll have to change the types of our instance vars where you pointed them out, as well as some method return types.

You're also right that changing those return types would run afoul of our back-compat policy.  We've got a few options:
1. Only fix this in master, and requires users wait until 9.0 for the fix.
2. Keep the int-returning methods and leave their signatures as-is, while introducing near-duplicate methods that return the correct types.  Deprecate the int-returning versions with references to these new methods.
3. Make a special request on the dev-list to exempt this change from our back-compat policy.  I think there's an "ok" argument for doing that here...our backcompat policy is intended to help us serve our users.  Slavishly maintaining backcompat and keeping the buggy versions of these methods around meets the letter of that law, but totally fails to meet the spirit.  We're not doing our users any favors by keeping these methods around.

So to answer your question ("Should I create different patches"), I'm not entirely decided.  I might appeal to the dev-list to see if anyone cares in this case.  But most likely I'll pursue option 2.  In that case, I'll build off of your patch and introduce the duplicate methods and deprecation warnings.  I'll try to something ready on this sometime this week.

> JsonFacetingResponse classes should record  provide access to count fields as longs
> -----------------------------------------------------------------------------------
>
>                 Key: SOLR-13318
>                 URL: https://issues.apache.org/jira/browse/SOLR-13318
>             Project: Solr
>          Issue Type: Bug
>      Security Level: Public(Default Security Level. Issues are Public) 
>          Components: SolrJ
>    Affects Versions: 7.7.1
>            Reporter: Jason Gerlowski
>            Assignee: Jason Gerlowski
>            Priority: Minor
>         Attachments: SOLR-13318.patch
>
>
> JsonFacetingResponse and its series of dependent classes hold a variety of count fields for bucket counts and various optional properties ({{allBuckets}}, {{numBuckets}}, etc.).  Currently, some of the code that parses these values out of the originating NamedList either stores or casts the values as ints.  When doc counts are low this works fine.  But when the doc counts become larger and stray into "long" territory, SolrJ is liable to blow up with ClassCastExceptions.
> A user on the list reported on of these with the partial stack trace:
> {code}
> Caused by: java.lang.ClassCastException: java.lang.Long cannot be cast to java.lang.Integer
>       at org.apache.solr.client.solrj.response.json.NestableJsonFacet.<init>(NestableJsonFacet.java:52)
>       at org.apache.solr.client.solrj.response.QueryResponse.extractJsonFacetingInfo(QueryResponse.java:200)
>       at org.apache.solr.client.solrj.response.QueryResponse.getJsonFacetingResponse(QueryResponse.java:571)
> {code}
> We should fix this so that these classes can be used without incident for any doc counts.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org