Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 4.0-ALPHA
    • Fix Version/s: 3.6
    • Component/s: modules/spatial
    • Labels:
      None
    • Lucene Fields:
      New

      Description

      The spatial contrib is blighted by bugs. The latest series, found by Grant and discussed here shows that we need to re-think the cartesian tier implementation.

      Given the need to create a spatial module containing code taken from both lucene and Solr, it makes sense to deprecate the spatial contrib, and start from scratch in the new module.

      1. LUCENE-2599.patch
        24 kB
        Chris Male
      2. LUCENE-2599.patch
        21 kB
        Chris Male

        Issue Links

          Activity

          Chris Male created issue -
          Chris Male made changes -
          Field Original Value New Value
          Description The spatial contrib is blighted by bugs. The latest series, found by Grant and discussed [here|http://search.lucidimagination.com/search/document/c32e81783642df47/spatial_rethinking_cartesian_tiers_implementation] shows that we need to re-think the cartesian tier implementation.

          Given the need to create a spatial module containing code taken from both lucene and Solr, it makes sense to deprecate the spatial contrib, and start from scratch in the new module.

          Part of this process will be to address each of the remaining issues for the spatial contrib, applying the easy bug fixes we can, and closing those issues we do not intend to address.


          The spatial contrib is blighted by bugs. The latest series, found by Grant and discussed [here|http://search.lucidimagination.com/search/document/c32e81783642df47/spatial_rethinking_cartesian_tiers_implementation] shows that we need to re-think the cartesian tier implementation.

          Given the need to create a spatial module containing code taken from both lucene and Solr, it makes sense to deprecate the spatial contrib, and start from scratch in the new module.


          Hide
          Simon Willnauer added a comment -

          chris, wanna come up with a patch against trunk that deprecates things and I go and port it to 3.x - once we are done we can nuke it from trunk

          Show
          Simon Willnauer added a comment - chris, wanna come up with a patch against trunk that deprecates things and I go and port it to 3.x - once we are done we can nuke it from trunk
          Hide
          Chris Male added a comment -

          On it.

          Show
          Chris Male added a comment - On it.
          Hide
          Chris Male added a comment -

          Attached patch for deprecating the contrib. Each class is deprecated, and a message is left in the package.html.

          Show
          Chris Male added a comment - Attached patch for deprecating the contrib. Each class is deprecated, and a message is left in the package.html.
          Chris Male made changes -
          Attachment LUCENE-2599.patch [ 12465038 ]
          Hide
          Chris Male added a comment -

          Attaching a patch that includes the creation of a new spatial module. Definitely needs someone to look at the build.xml (Robert?) but I think it works.

          Show
          Chris Male added a comment - Attaching a patch that includes the creation of a new spatial module. Definitely needs someone to look at the build.xml (Robert?) but I think it works.
          Chris Male made changes -
          Attachment LUCENE-2599.patch [ 12465040 ]
          Hide
          Simon Willnauer added a comment -

          Chris, I think we should not add an empty modules sub though. I plan to commit the deprecation part to 3.x and nuke spatial from 4.0 entirely. Would you want to move forward and open an issue to fill the module with some solr spatial logic?

          Objections anybody?

          Show
          Simon Willnauer added a comment - Chris, I think we should not add an empty modules sub though. I plan to commit the deprecation part to 3.x and nuke spatial from 4.0 entirely. Would you want to move forward and open an issue to fill the module with some solr spatial logic? Objections anybody?
          Hide
          Chris Male added a comment -

          I just noticed that Solr depends upon some methods in DistanceUtils. We'll need to move that into the module before removing the contrib from 4x.

          Show
          Chris Male added a comment - I just noticed that Solr depends upon some methods in DistanceUtils. We'll need to move that into the module before removing the contrib from 4x.
          Mark Thomas made changes -
          Workflow jira [ 12517738 ] Default workflow, editable Closed status [ 12564008 ]
          Mark Thomas made changes -
          Workflow Default workflow, editable Closed status [ 12564008 ] jira [ 12585481 ]
          Hide
          David Smiley added a comment -

          I am in favor of nuking spatial contrib (FYI I'm the author of SOLR-2155). Solr 3.1 will be released soon so I suggest that committers here act now while there's time.

          Show
          David Smiley added a comment - I am in favor of nuking spatial contrib (FYI I'm the author of SOLR-2155 ). Solr 3.1 will be released soon so I suggest that committers here act now while there's time.
          Hide
          Chris Male added a comment -

          The nuking will occur in trunk so its unaffected by the Solr 3.1 release. However some of Solr's spatial code depends on the contrib so its too late to make the deprecations in 3.1.

          Show
          Chris Male added a comment - The nuking will occur in trunk so its unaffected by the Solr 3.1 release. However some of Solr's spatial code depends on the contrib so its too late to make the deprecations in 3.1.
          Hide
          Robert Muir added a comment -

          Please don't remove remove any lucene spatial capabilities until there is a suitable replacement.

          Its absurd to tell users they should have to use Solr to use spatial, this makes no sense.

          Show
          Robert Muir added a comment - Please don't remove remove any lucene spatial capabilities until there is a suitable replacement. Its absurd to tell users they should have to use Solr to use spatial, this makes no sense.
          Hide
          Chris Male added a comment -

          Thats a fair point and there is every intention to do that. But its also dangerous to continue to offer code with known (and probably many unknown) bugs.

          Show
          Chris Male added a comment - Thats a fair point and there is every intention to do that. But its also dangerous to continue to offer code with known (and probably many unknown) bugs.
          Hide
          Chris Male added a comment -

          I'd like to come to an agreement here and finish this off as to make a clear statement about spatial in Lucene 4.

          It seems we have a few options in front of us now:

          • Deprecate in 3x, nuke fully in trunk
          • Deprecate in 3x and in trunk, moving the code to the sandbox
          • Deprecate in 3x and in trunk and leave the code where it is
          • Do nothing

          Robert's concern that removing the contrib entirely without providing an alternative seems very reasonable. But as I said in February, I'm very wary of continuing to release code which has so many problems. As such, I favour deprecating in 3x and in trunk and sandboxing the code wit big warnings that it'll go when we have a better solution.

          Any opinions? Robert? David?

          Show
          Chris Male added a comment - I'd like to come to an agreement here and finish this off as to make a clear statement about spatial in Lucene 4. It seems we have a few options in front of us now: Deprecate in 3x, nuke fully in trunk Deprecate in 3x and in trunk, moving the code to the sandbox Deprecate in 3x and in trunk and leave the code where it is Do nothing Robert's concern that removing the contrib entirely without providing an alternative seems very reasonable. But as I said in February, I'm very wary of continuing to release code which has so many problems. As such, I favour deprecating in 3x and in trunk and sandboxing the code wit big warnings that it'll go when we have a better solution. Any opinions? Robert? David?
          Chris Male made changes -
          Assignee Chris Male [ cmale ]
          Hide
          David Smiley added a comment -

          Deprecate in 3x, absolutely. We know it's broken and we need to send a message to folks who don't know any better. I hope we can at least get consensus on this part.

          I don't see any point in this bad code moving forward into Lucene 4, whether there is a replacement or not.

          Show
          David Smiley added a comment - Deprecate in 3x, absolutely. We know it's broken and we need to send a message to folks who don't know any better. I hope we can at least get consensus on this part. I don't see any point in this bad code moving forward into Lucene 4, whether there is a replacement or not.
          Hide
          Ryan McKinley added a comment -

          Deprecate in 3x and in trunk, moving the code to the sandbox

          +1

          and then decide what to do one something better gets up-to-speed

          Show
          Ryan McKinley added a comment - Deprecate in 3x and in trunk, moving the code to the sandbox +1 and then decide what to do one something better gets up-to-speed
          David Smiley made changes -
          Link This issue is related to LUCENE-3795 [ LUCENE-3795 ]
          Hide
          David Smiley added a comment -

          This contrib module was eliminated from trunk as part of LUCENE-3795. So what to do about 3x?: Add a @Deprecated annotation to all classes in the contrib? And add a CHANGES.txt entry?

          Show
          David Smiley added a comment - This contrib module was eliminated from trunk as part of LUCENE-3795 . So what to do about 3x?: Add a @Deprecated annotation to all classes in the contrib? And add a CHANGES.txt entry?
          Hide
          Chris Male added a comment -

          +1

          Show
          Chris Male added a comment - +1
          Hide
          David Smiley added a comment -

          I'm about to commit a bunch of deprecations and the following edit to the overview.html javadoc:

          <h2>DEPRECATED</h2>
          <p>
            The Lucene 3.x spatial contrib module is deprecated as of Solr 3.6. In Solr 4 there is a new spatial
            module that should be used instead.
            <br/>
            For further information, see 
            <a href="https://issues.apache.org/jira/browse/LUCENE-2599">LUCENE-2599</a>.
            <br/>
            The old documentation follows below:
          </p>
          
          Show
          David Smiley added a comment - I'm about to commit a bunch of deprecations and the following edit to the overview.html javadoc: <h2>DEPRECATED</h2> <p> The Lucene 3.x spatial contrib module is deprecated as of Solr 3.6. In Solr 4 there is a new spatial module that should be used instead. <br/> For further information, see <a href= "https: //issues.apache.org/jira/browse/LUCENE-2599" >LUCENE-2599</a>. <br/> The old documentation follows below: </p>
          Hide
          Robert Muir added a comment -

          I think thats confusing. It should say Lucene 3.6, Lucene 4. Your current wording
          makes people think that they have to use Solr to do spatial queries, and thats not
          true, correct?

          Show
          Robert Muir added a comment - I think thats confusing. It should say Lucene 3.6, Lucene 4. Your current wording makes people think that they have to use Solr to do spatial queries, and thats not true, correct?
          Hide
          David Smiley added a comment -

          Marking as fixed. The javadoc & changes.txt were revision 1305595.

          Show
          David Smiley added a comment - Marking as fixed. The javadoc & changes.txt were revision 1305595.
          David Smiley made changes -
          Status Open [ 1 ] Resolved [ 5 ]
          Assignee Chris Male [ cmale ] David Smiley [ dsmiley ]
          Fix Version/s 3.6 [ 12319070 ]
          Resolution Fixed [ 1 ]
          Hide
          David Smiley added a comment -

          Woops Robert; you are right. I will fix this right away.

          Show
          David Smiley added a comment - Woops Robert; you are right. I will fix this right away.
          Uwe Schindler made changes -
          Status Resolved [ 5 ] Closed [ 6 ]

            People

            • Assignee:
              David Smiley
              Reporter:
              Chris Male
            • Votes:
              1 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development