Details

    • Type: New Feature
    • Status: Resolved
    • Priority: Major
    • Resolution: Implemented
    • Affects Version/s: None
    • Fix Version/s: None
    • Component/s: None
    • Labels:
      None
    • Lucene Fields:
      New

      Description

      This issue adda a new InetAddressRangeField for indexing and querying InetAddress ranges.

      1. LUCENE-7738.patch
        15 kB
        Nicholas Knize
      2. LUCENE-7738.patch
        15 kB
        Nicholas Knize

        Activity

        Hide
        nknize Nicholas Knize added a comment -

        First patch adds InetAddressRangeField along with support for INTERSECTS, WITHIN, CONTAINS, and CROSSES queries.

        Patch also includes random testing for both IPv4 and IPv6.

        Show
        nknize Nicholas Knize added a comment - First patch adds InetAddressRangeField along with support for INTERSECTS , WITHIN , CONTAINS , and CROSSES queries. Patch also includes random testing for both IPv4 and IPv6.
        Hide
        jpountz Adrien Grand added a comment -

        +1 Could you just create ipv4 addresses in the test from a byte[4] rather than from a String? (like you do for ipv6 addresses)

        Show
        jpountz Adrien Grand added a comment - +1 Could you just create ipv4 addresses in the test from a byte [4] rather than from a String? (like you do for ipv6 addresses)
        Hide
        nknize Nicholas Knize added a comment -

        Yes! I cleaned that up and merged those separate ipv4 and ipv6 methods.

        Show
        nknize Nicholas Knize added a comment - Yes! I cleaned that up and merged those separate ipv4 and ipv6 methods.
        Hide
        dsmiley David Smiley added a comment -

        This is very nice and especially well documented; nice job Nick!

        What do you think about putting this in Lucene's "misc" module? IP range search seems fairly fringe – useful to some folks surely. And InetAddressPoint (currently in sandbox) should probably be co-located with InetAddressRangeField; no?

        Show
        dsmiley David Smiley added a comment - This is very nice and especially well documented; nice job Nick! What do you think about putting this in Lucene's "misc" module? IP range search seems fairly fringe – useful to some folks surely. And InetAddressPoint (currently in sandbox) should probably be co-located with InetAddressRangeField; no?
        Hide
        nknize Nicholas Knize added a comment -

        Thanks David Smiley!

        I have no problem putting both InetAddressRangeField and InetAddressPoint in the misc module. Seems like a sound justification to me.

        I also think I'm going to refactor all range field classes to remove Field from the class name. It seems more natural (to me) to have Point and Range counterparts (e.g., DoublePoint, DoubleRange, InetAddressPoint, InetAddressRange)? (and less typing is always a bonus) What do you think? (or does anyone care? - I know naming is often an exercise in bikeshedding)

        If no one objects I can open a separate issue to "Graduate Range Fields from sandbox" that will refactor the range field class names and move them to the appropriate module? What does everyone think?

        Show
        nknize Nicholas Knize added a comment - Thanks David Smiley ! I have no problem putting both InetAddressRangeField and InetAddressPoint in the misc module. Seems like a sound justification to me. I also think I'm going to refactor all range field classes to remove Field from the class name. It seems more natural (to me) to have Point and Range counterparts (e.g., DoublePoint , DoubleRange , InetAddressPoint , InetAddressRange )? (and less typing is always a bonus) What do you think? (or does anyone care? - I know naming is often an exercise in bikeshedding) If no one objects I can open a separate issue to "Graduate Range Fields from sandbox" that will refactor the range field class names and move them to the appropriate module? What does everyone think?
        Hide
        dsmiley David Smiley added a comment -

        RE naming; some consistency is good. So if the non-range variant doesn't have 'Field' then don't put it on the range variant. But vice-versa is also true – consistency being most important. I care little wether both should have (or not have) a "Field" suffix. There seems to be a lot of variation across Lucene, unfortunately.

        I can open a separate issue to "Graduate Range Fields from sandbox" that will refactor the range field class names and move them to the appropriate module?

        +1

        Show
        dsmiley David Smiley added a comment - RE naming; some consistency is good. So if the non-range variant doesn't have 'Field' then don't put it on the range variant. But vice-versa is also true – consistency being most important. I care little wether both should have (or not have) a "Field" suffix. There seems to be a lot of variation across Lucene, unfortunately. I can open a separate issue to "Graduate Range Fields from sandbox" that will refactor the range field class names and move them to the appropriate module? +1
        Hide
        jira-bot ASF subversion and git services added a comment -

        Commit 1745b0338e822db43f292f7ad495789b21c6634a in lucene-solr's branch refs/heads/master from Nicholas Knize
        [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=1745b03 ]

        LUCENE-7738: Add new InetAddressRangeField for indexing and querying InetAddress ranges.

        Show
        jira-bot ASF subversion and git services added a comment - Commit 1745b0338e822db43f292f7ad495789b21c6634a in lucene-solr's branch refs/heads/master from Nicholas Knize [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=1745b03 ] LUCENE-7738 : Add new InetAddressRangeField for indexing and querying InetAddress ranges.
        Hide
        jpountz Adrien Grand added a comment -

        +1 to the patch
        +1 to removing the "Field" suffix from range fields
        +1 on graduating those fields from sandbox, I was initially thinking core would be a good fit (LUCENE-7314) but misc works for me too

        Show
        jpountz Adrien Grand added a comment - +1 to the patch +1 to removing the "Field" suffix from range fields +1 on graduating those fields from sandbox, I was initially thinking core would be a good fit ( LUCENE-7314 ) but misc works for me too
        Hide
        jira-bot ASF subversion and git services added a comment -

        Commit 13f020f5899118bfba41b845deb728c1c131ff46 in lucene-solr's branch refs/heads/branch_6x from Nicholas Knize
        [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=13f020f ]

        LUCENE-7738: Add new InetAddressRangeField for indexing and querying InetAddress ranges.

        Show
        jira-bot ASF subversion and git services added a comment - Commit 13f020f5899118bfba41b845deb728c1c131ff46 in lucene-solr's branch refs/heads/branch_6x from Nicholas Knize [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=13f020f ] LUCENE-7738 : Add new InetAddressRangeField for indexing and querying InetAddress ranges.
        Hide
        nknize Nicholas Knize added a comment -

        I opened LUCENE-7740 to refactor class names and move InetAddressRange from sandbox to misc. All other *Range fields will be refactored to core.

        Show
        nknize Nicholas Knize added a comment - I opened LUCENE-7740 to refactor class names and move InetAddressRange from sandbox to misc . All other *Range fields will be refactored to core.
        Hide
        jpountz Adrien Grand added a comment - - edited

        Actually can you remove the e.printStackTrace() call in the test and rethrow the exception instead? (potentially wrapped in a RuntimeException if it makes things easier)

        Show
        jpountz Adrien Grand added a comment - - edited Actually can you remove the e.printStackTrace() call in the test and rethrow the exception instead? (potentially wrapped in a RuntimeException if it makes things easier)
        Hide
        nknize Nicholas Knize added a comment -

        +1

        Show
        nknize Nicholas Knize added a comment - +1
        Hide
        erickerickson Erick Erickson added a comment -

        This has been committed, can it be closed?

        Show
        erickerickson Erick Erickson added a comment - This has been committed, can it be closed?
        Hide
        jira-bot ASF subversion and git services added a comment -

        Commit ef8126e5eab7aec9c8775c2e08bd6c2bb1ef690f in lucene-solr's branch refs/heads/master from Nicholas Knize
        [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=ef8126e ]

        LUCENE-7738: Fix min/max verification bug in InetAddressRange to correctly compare IPv4 and IPv6. Update tests.

        Show
        jira-bot ASF subversion and git services added a comment - Commit ef8126e5eab7aec9c8775c2e08bd6c2bb1ef690f in lucene-solr's branch refs/heads/master from Nicholas Knize [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=ef8126e ] LUCENE-7738 : Fix min/max verification bug in InetAddressRange to correctly compare IPv4 and IPv6. Update tests.
        Hide
        jira-bot ASF subversion and git services added a comment -

        Commit 6eb00543bc969d375b43ed20bedcbdb60bc8f22a in lucene-solr's branch refs/heads/branch_6x from Nicholas Knize
        [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=6eb0054 ]

        LUCENE-7738: Fix min/max verification bug in InetAddressRange to correctly compare IPv4 and IPv6. Update tests.

        Show
        jira-bot ASF subversion and git services added a comment - Commit 6eb00543bc969d375b43ed20bedcbdb60bc8f22a in lucene-solr's branch refs/heads/branch_6x from Nicholas Knize [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=6eb0054 ] LUCENE-7738 : Fix min/max verification bug in InetAddressRange to correctly compare IPv4 and IPv6. Update tests.
        Hide
        jira-bot ASF subversion and git services added a comment -

        Commit 2bdb3c7d1e738bbcfffa45bf47639446b0734f0f in lucene-solr's branch refs/heads/branch_6_5 from Nicholas Knize
        [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=2bdb3c7 ]

        LUCENE-7738: Fix min/max verification bug in InetAddressRange to correctly compare IPv4 and IPv6. Update tests.

        Show
        jira-bot ASF subversion and git services added a comment - Commit 2bdb3c7d1e738bbcfffa45bf47639446b0734f0f in lucene-solr's branch refs/heads/branch_6_5 from Nicholas Knize [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=2bdb3c7 ] LUCENE-7738 : Fix min/max verification bug in InetAddressRange to correctly compare IPv4 and IPv6. Update tests.

          People

          • Assignee:
            Unassigned
            Reporter:
            nknize Nicholas Knize
          • Votes:
            0 Vote for this issue
            Watchers:
            5 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development