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

Add new ip_range field type #24433

Closed
wants to merge 1 commit into from

Conversation

nknize
Copy link
Contributor

@nknize nknize commented May 2, 2017

This commit adds support for indexing and searching the new ip_range field type. Both IPv4 and IPv6 formats are supported. Tests and docs are updated.

@nknize nknize added :Search/Mapping Index mappings, including merging and defining field types v6.0.0-alpha1 v5.5.0 labels May 2, 2017
@nknize nknize requested a review from nik9000 May 2, 2017 15:33
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.

I left some minor comments but the change looks good to me overall.

}
@Override
public InetAddress parseFrom(RangeFieldType fieldType, XContentParser parser, boolean coerce, boolean included)
throws IOException {
Copy link
Contributor

Choose a reason for hiding this comment

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

can you indent so that the throws is not indented at the same level as the content of the block? (similar issues on the below lines)

@Override
public InetAddress parse(Object value, boolean coerce) {
try {
return value instanceof InetAddress ? (InetAddress) value : InetAddress.getByName((String) value);
Copy link
Contributor

Choose a reason for hiding this comment

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

can you use InetAddresses.forString intead, which guarantees it won't do a dns lookup

if (type.equals("date_range")) {
strVal = "1477872000000";
} else if (type.equals("ip_range")) {
strVal = "/192.168.1.7:/2001:db8:0:0:0:0:0:0";
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess we get this by doing a toString on an InetAddress. Could you make sure we use InetAddresses.toAddrString instead?

@nknize
Copy link
Contributor Author

nknize commented May 3, 2017

Thanks for the excellent feedback @jpountz. I made the changes and will go ahead and merge tomorrow unless you have any other concerns.

This commit adds support for indexing and searching a new ip_range field type. Both IPv4 and IPv6 formats are supported. Tests are updated and docs are added.
@nknize
Copy link
Contributor Author

nknize commented May 5, 2017

merged in 0c4eb0a

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>feature :Search/Mapping Index mappings, including merging and defining field types v5.5.0 v6.0.0-alpha2
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants