Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 5.0, 6.0
    • Component/s: modules/spatial
    • Labels:
      None
    • Lucene Fields:
      New

      Description

      This is a refactor of the SpatialPrefixTree spatial API, in preparation for more SPT implementations on the near horizon. These are fairly internal APIs; SpatialExample.java didn't have to change, nor the Solr adapters, and I doubt ES would have to either.

      API changes:

      • SpatialPrefixTree & Cell had a fairly significant make-over. The existing implementations for Geohash & Quad have been made to subclass LegacyPrefixTree & LegacyCell shim's, and otherwise had very few changes (performance should be the same). Cell is now an interface.
      • New CellIterator which is an Iterator<Cell>. Includes 3 implementations.
      • PrefixTreeStrategy.simplifyIndexedCells was renamed to pruneLeafyBranches and moved to RPT and made toggle'able with a setter. It's going to be removed in the future but for the time being it remains a useful optimization.
      • RPT's pointsOnly & multiOverlappingIndexedShapes options now have setters.

      Future:

      • The AbstractVisitingPrefixTreeFilter (used by RPT's Intersects, Within, Disjoint) really should be refactored to use the new CellIterator API as it will reduce the amount of code and should make the code easier to follow since it would be based on a well-knon design-pattern (an iterator).

      I wish I had done this as a series of commits on a GitHub branch; ah well.

        Issue Links

          Activity

          Hide
          David Smiley added a comment -

          I'll commit this in a day or so.

          Show
          David Smiley added a comment - I'll commit this in a day or so.
          Hide
          Ryan McKinley added a comment -

          +1 for code cleanup

          Show
          Ryan McKinley added a comment - +1 for code cleanup
          Hide
          ASF subversion and git services added a comment -

          Commit 1588404 from David Smiley in branch 'dev/trunk'
          [ https://svn.apache.org/r1588404 ]

          LUCENE-5608: Refactor SpatialPrefixTree/Cell API. Introduced CellIterator.

          Show
          ASF subversion and git services added a comment - Commit 1588404 from David Smiley in branch 'dev/trunk' [ https://svn.apache.org/r1588404 ] LUCENE-5608 : Refactor SpatialPrefixTree/Cell API. Introduced CellIterator.
          Hide
          David Smiley added a comment - - edited

          Not back-porting to 4x until it gets used, which might cause more changes.

          Show
          David Smiley added a comment - - edited Not back-porting to 4x until it gets used, which might cause more changes.
          Hide
          ASF subversion and git services added a comment -

          Commit 1588457 from David Smiley in branch 'dev/trunk'
          [ https://svn.apache.org/r1588457 ]

          LUCENE-5608: fix visibility of CellTokenStream from RPT/PT Strategy because of javadoc constraints on package visibility

          Show
          ASF subversion and git services added a comment - Commit 1588457 from David Smiley in branch 'dev/trunk' [ https://svn.apache.org/r1588457 ] LUCENE-5608 : fix visibility of CellTokenStream from RPT/PT Strategy because of javadoc constraints on package visibility
          Hide
          ASF subversion and git services added a comment -

          Commit 1593468 from David Smiley in branch 'dev/trunk'
          [ https://svn.apache.org/r1593468 ]

          LUCENE-5608: Move readCell() back to SPT from Cell.

          Show
          ASF subversion and git services added a comment - Commit 1593468 from David Smiley in branch 'dev/trunk' [ https://svn.apache.org/r1593468 ] LUCENE-5608 : Move readCell() back to SPT from Cell.
          Hide
          ASF subversion and git services added a comment -

          Commit 1594391 from David Smiley in branch 'dev/trunk'
          [ https://svn.apache.org/r1594391 ]

          LUCENE-5608 fix assertions given that tokens are re-used

          Show
          ASF subversion and git services added a comment - Commit 1594391 from David Smiley in branch 'dev/trunk' [ https://svn.apache.org/r1594391 ] LUCENE-5608 fix assertions given that tokens are re-used
          Hide
          ASF subversion and git services added a comment -

          Commit 1594394 from David Smiley in branch 'dev/trunk'
          [ https://svn.apache.org/r1594394 ]

          LUCENE-5608 better/more comments

          Show
          ASF subversion and git services added a comment - Commit 1594394 from David Smiley in branch 'dev/trunk' [ https://svn.apache.org/r1594394 ] LUCENE-5608 better/more comments
          Hide
          David Smiley added a comment -

          One thing about the API I don't love is that to write an efficient Cell impl, you have to lazy-decode from bytes. This is because the code using the Cell creates Cells sometimes only to check leaf status and/or the level, or maybe to check if it's within (underneath, a suffix of) another cell. Alternatively, there could be some simple methods on SPT like readCellIsBoolean() and readCellLevel(), and have Cell.isPrefixOf() take a BytesRef instead of another Cell. Ok, a couple simple new methods, but then there is the conundrum of the parameters to abstract methods in AbstractVisitingPrefixTreeFilter like visitLeaf() that take a Cell, which would maybe would be modified to take, what, a BytesRef; and maybe the length and leaf boolean? That doesn't seem right. So lazy-decode it is.

          Show
          David Smiley added a comment - One thing about the API I don't love is that to write an efficient Cell impl, you have to lazy-decode from bytes. This is because the code using the Cell creates Cells sometimes only to check leaf status and/or the level, or maybe to check if it's within (underneath, a suffix of) another cell. Alternatively, there could be some simple methods on SPT like readCellIsBoolean() and readCellLevel(), and have Cell.isPrefixOf() take a BytesRef instead of another Cell. Ok, a couple simple new methods, but then there is the conundrum of the parameters to abstract methods in AbstractVisitingPrefixTreeFilter like visitLeaf() that take a Cell, which would maybe would be modified to take, what, a BytesRef; and maybe the length and leaf boolean? That doesn't seem right. So lazy-decode it is.
          Hide
          ASF subversion and git services added a comment -

          Commit 1603992 from David Smiley in branch 'dev/trunk'
          [ https://svn.apache.org/r1603992 ]

          LUCENE-5608: small test improvements.
          Rename SpatialOpRecursivePrefixTreeTest to RandomSpatialOpFuzzyPrefixTreeTest.

          Show
          ASF subversion and git services added a comment - Commit 1603992 from David Smiley in branch 'dev/trunk' [ https://svn.apache.org/r1603992 ] LUCENE-5608 : small test improvements. Rename SpatialOpRecursivePrefixTreeTest to RandomSpatialOpFuzzyPrefixTreeTest.
          Hide
          ASF subversion and git services added a comment -

          Commit 1603994 from David Smiley in branch 'dev/branches/branch_4x'
          [ https://svn.apache.org/r1603994 ]

          LUCENE-5608: small test improvements. Rename SpatialOpRecursivePrefixTreeTest to RandomSpatialOpFuzzyPrefixTreeTest.

          Show
          ASF subversion and git services added a comment - Commit 1603994 from David Smiley in branch 'dev/branches/branch_4x' [ https://svn.apache.org/r1603994 ] LUCENE-5608 : small test improvements. Rename SpatialOpRecursivePrefixTreeTest to RandomSpatialOpFuzzyPrefixTreeTest.
          Hide
          ASF subversion and git services added a comment -

          Commit 1608933 from David Smiley in branch 'dev/branches/branch_4x'
          [ https://svn.apache.org/r1608933 ]

          LUCENE-5608: sync StrategyTestCase from trunk

          Show
          ASF subversion and git services added a comment - Commit 1608933 from David Smiley in branch 'dev/branches/branch_4x' [ https://svn.apache.org/r1608933 ] LUCENE-5608 : sync StrategyTestCase from trunk
          Hide
          Anshum Gupta added a comment -

          Bulk close after 5.0 release.

          Show
          Anshum Gupta added a comment - Bulk close after 5.0 release.

            People

            • Assignee:
              David Smiley
              Reporter:
              David Smiley
            • Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development