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

Aggregations: Makes ValueFormat and ValueFormatter never null #11943

Merged
merged 1 commit into from Jul 1, 2015

Conversation

@colings86
Copy link
Member

commented Jun 30, 2015

This allows a lot of null checks to be removed where we were always falling back to the ValueFormat.RAW anyway. Now the format is set to ValueFormat.RAW when no alternative is suitable.

Closes #10594

@jpountz
jpountz reviewed Jun 30, 2015
View changes
...rc/main/java/org/elasticsearch/search/aggregations/bucket/histogram/HistogramAggregator.java Outdated
@@ -60,7 +60,8 @@

public HistogramAggregator(String name, AggregatorFactories factories, Rounding rounding, InternalOrder order,
boolean keyed, long minDocCount, @Nullable ExtendedBounds extendedBounds,
@Nullable ValuesSource.Numeric valuesSource, @Nullable ValueFormatter formatter,
@Nullable ValuesSource.Numeric valuesSource,
ValueFormatter formatter,

This comment has been minimized.

Copy link
@jpountz

jpountz Jun 30, 2015

Contributor

indentation?

@jpountz
jpountz reviewed Jun 30, 2015
View changes
.../src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/InternalHistogram.java Outdated
@@ -260,7 +259,7 @@ public B createBucket(InternalAggregations aggregations, B prototype) {
}

public B createBucket(Object key, long docCount, InternalAggregations aggregations, boolean keyed,
@Nullable ValueFormatter formatter) {
ValueFormatter formatter) {

This comment has been minimized.

Copy link
@jpountz

jpountz Jun 30, 2015

Contributor

indentation?

@jpountz

This comment has been minimized.

Copy link
Contributor

commented Jun 30, 2015

Wonderful! LGTM

This allows a lot of null checks to be removed where we were always falling back to the ValueFormat.RAW anyway. Now the format is set to ValueFormat.RAW when no alternative is suitable.

Closes #10594
@colings86 colings86 force-pushed the colings86:enhancement/nonNullValueFormat branch to d73ce34 Jul 1, 2015
@colings86 colings86 merged commit d73ce34 into elastic:master Jul 1, 2015
1 check passed
1 check passed
CLA Commit author has signed the CLA
Details
@kevinkluge kevinkluge removed the review label Jul 1, 2015
@colings86 colings86 deleted the colings86:enhancement/nonNullValueFormat branch Jul 1, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.