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 format problem in composite of unmapped #50869

Merged
merged 2 commits into from
Jan 10, 2020

Conversation

nik9000
Copy link
Member

@nik9000 nik9000 commented Jan 10, 2020

When a composite aggregation is reduced using the results from an
index that has one of the fields unmapped we were throwing away the
formatter. This is mildly annoying, except in the case of IP addresses
which were coming out as non-utf-8-characters. And tripping assertions.

This carefully preserves the formatter from the working bucket.

Closes #50600

When a composite aggregation is reduced using the results from an
index that has one of the fields unmapped we were throwing away the
formatter. This is mildly annoying, except in the case of IP addresses
which were coming out as non-utf-8-characters. And tripping assertions.

This carefully preserves the formatter from the working bucket.

Closes elastic#50600
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-analytics-geo (:Analytics/Aggregations)

@Override
public String toString() {
return "ip";
}
Copy link
Member Author

Choose a reason for hiding this comment

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

I added these to help with debugging and figure they are probably fine to keep.

Copy link
Member

@not-napoleon not-napoleon left a comment

Choose a reason for hiding this comment

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

Nice clean fix, LGTM.

@@ -249,6 +251,30 @@ public void testReduceSame() throws IOException {
}
}

/**
* Check that reducing with an unmapped index produces useful formats.
Copy link
Member

Choose a reason for hiding this comment

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

Thank you for including a note of what we're testing, helps so much when debugging test failures!

Copy link
Contributor

@jimczi jimczi left a comment

Choose a reason for hiding this comment

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

LGTM too, thanks @nik9000 !

@nik9000 nik9000 merged commit 5a76fac into elastic:master Jan 10, 2020
@nik9000
Copy link
Member Author

nik9000 commented Jan 10, 2020

Thanks @not-napoleon and @jimczi !

nik9000 added a commit to nik9000/elasticsearch that referenced this pull request Jan 10, 2020
When a composite aggregation is reduced using the results from an
index that has one of the fields unmapped we were throwing away the
formatter. This is mildly annoying, except in the case of IP addresses
which were coming out as non-utf-8-characters. And tripping assertions.

This carefully preserves the formatter from the working bucket.

Closes elastic#50600
nik9000 added a commit that referenced this pull request Jan 10, 2020
When a composite aggregation is reduced using the results from an
index that has one of the fields unmapped we were throwing away the
formatter. This is mildly annoying, except in the case of IP addresses
which were coming out as non-utf-8-characters. And tripping assertions.

This carefully preserves the formatter from the working bucket.

Closes #50600
nik9000 added a commit to nik9000/elasticsearch that referenced this pull request Jan 17, 2020
Now that we've backported elastic#50869 we can update the skip config for its
test.
nik9000 added a commit that referenced this pull request Jan 17, 2020
Now that we've backported #50869 we can update the skip config for its
test.
SivagurunathanV pushed a commit to SivagurunathanV/elasticsearch that referenced this pull request Jan 23, 2020
When a composite aggregation is reduced using the results from an
index that has one of the fields unmapped we were throwing away the
formatter. This is mildly annoying, except in the case of IP addresses
which were coming out as non-utf-8-characters. And tripping assertions.

This carefully preserves the formatter from the working bucket.

Closes elastic#50600
SivagurunathanV pushed a commit to SivagurunathanV/elasticsearch that referenced this pull request Jan 23, 2020
Now that we've backported elastic#50869 we can update the skip config for its
test.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

composite and terms aggregations behave differently on ip fields
5 participants