Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix NPE with scaled floats stats when field is not indexed #23528

Merged
merged 3 commits into from
Mar 15, 2017

Conversation

rjernst
Copy link
Member

@rjernst rjernst commented Mar 10, 2017

This fixes an NPE in finding scaled float stats. The type of min/max
methods on the wrapped long stats returns a boxed type, but in the case
this is null, the unbox done for the FieldStats.Double ctor primitive
types will cause the NPE. These methods would have null for min/max when
the field exists, but does not actually have points values.

fixes #23487

This fixes an NPE in finding scaled float stats. The type of min/max
methods on the wrapped long stats returns a boxed type, but in the case
this is null, the unbox done for the FieldStats.Double ctor primitive
types will cause the NPE. These methods would have null for min/max when
the field exists, but does not actually have points values.

fixes elastic#23487
Copy link
Member

@jasontedor jasontedor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left a question.

@@ -268,8 +268,9 @@ public Query rangeQuery(Object lowerTerm, Object upperTerm, boolean includeLower
return new FieldStats.Double(stats.getMaxDoc(), stats.getDocCount(),
stats.getSumDocFreq(), stats.getSumTotalTermFreq(),
stats.isSearchable(), stats.isAggregatable(),
stats.getMinValue() == null ? null : stats.getMinValue() / scalingFactor,
stats.getMaxValue() == null ? null : stats.getMaxValue() / scalingFactor);
stats.getMinValue() == null ? 0.0 : stats.getMinValue() / scalingFactor,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe Double.NaN?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

with regular numerics, we seem to be setting min/max to null in that case. Maybe we should look at the docFreq stat of the fieldstats and call the constructor that does not take min/max values when it is 0?

@rjernst
Copy link
Member Author

rjernst commented Mar 11, 2017

@jasontedor @jpountz I pushed a change to use the appropriate ctor based on whether the wrapped stats have min/max.

Copy link
Contributor

@jpountz jpountz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@rjernst rjernst merged commit d808e75 into elastic:master Mar 15, 2017
@rjernst rjernst deleted the scaled_float_no_points branch March 15, 2017 22:14
rjernst added a commit that referenced this pull request Mar 15, 2017
…23528)

This fixes an NPE in finding scaled float stats. The type of min/max
methods on the wrapped long stats returns a boxed type, but in the case
this is null, the unbox done for the FieldStats.Double ctor primitive
types will cause the NPE. These methods would have null for min/max when
the field exists, but does not actually have points values.

fixes #23487
rjernst added a commit that referenced this pull request Mar 15, 2017
…23528)

This fixes an NPE in finding scaled float stats. The type of min/max
methods on the wrapped long stats returns a boxed type, but in the case
this is null, the unbox done for the FieldStats.Double ctor primitive
types will cause the NPE. These methods would have null for min/max when
the field exists, but does not actually have points values.

fixes #23487
jasontedor added a commit to jasontedor/elasticsearch that referenced this pull request Mar 17, 2017
* master:
  Clear the interrupt flag before joining
  Migrate to max line length of 100
  Docs: Corrected path to elasticsearch-plugin (elastic#23622)
  Docs: Add comma to reverse nested agg snippet
  Fix third-party audit task for Gradle 3.4 (elastic#23612)
  Adapter action future should restore interrupts
  Update scripting.asciidoc
  Unmark reindex as experimental
  CompletionSuggestionContext#toQuery() should also consider text if prefix/regex missing (elastic#23451)
  Docs: Specify that byte units use powers of 1024 (elastic#23574)
  Remove Settings.settingsBuilder (elastic#23575)
  Change params._source to params['_source'] in example.
  Fix example in documentation for Painless using _source. (elastic#21322)
  Remove extra line from license header
  Fix num docs to be positive in bucket deferring collector test
  Mapping: Fix NPE with scaled floats stats when field is not indexed (elastic#23528)
jasontedor added a commit to jasontedor/elasticsearch that referenced this pull request Mar 17, 2017
* single-node-discovery:
  Clear the interrupt flag before joining
  Migrate to max line length of 100
  Docs: Corrected path to elasticsearch-plugin (elastic#23622)
  Docs: Add comma to reverse nested agg snippet
  Fix third-party audit task for Gradle 3.4 (elastic#23612)
  Adapter action future should restore interrupts
  Update scripting.asciidoc
  Unmark reindex as experimental
  CompletionSuggestionContext#toQuery() should also consider text if prefix/regex missing (elastic#23451)
  Docs: Specify that byte units use powers of 1024 (elastic#23574)
  Remove Settings.settingsBuilder (elastic#23575)
  Change params._source to params['_source'] in example.
  Fix example in documentation for Painless using _source. (elastic#21322)
  Remove extra line from license header
  Fix num docs to be positive in bucket deferring collector test
  Mapping: Fix NPE with scaled floats stats when field is not indexed (elastic#23528)
@clintongormley clintongormley changed the title Mapping: Fix NPE with scaled floats stats when field is not indexed Fix NPE with scaled floats stats when field is not indexed Apr 20, 2017
@clintongormley clintongormley added :Search Foundations/Mapping Index mappings, including merging and defining field types >bug labels Apr 20, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>bug :Search Foundations/Mapping Index mappings, including merging and defining field types v5.3.1 v5.4.0 v6.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

NPE when field index turned off with scaled_float type
4 participants