[
https://issues.apache.org/jira/browse/LUCENE-4167?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13402783#comment-13402783
]
Chris Male commented on LUCENE-4167:
------------------------------------
{quote}
The stategy shouldn't care about the bbox concept, I agree. I think the bbox
capability should be decoupled from SpatialOperation. It's not a simple matter
of the client calling queryShape.getBoundingBox() since the expression of the
query shape from client to server is a string. So instead of
"BBoxIntersects(Circle(3,5 d=10))" I propose supporting
"INTERSECTS(BBOX(Circle(3,5 d=10)))". The actual set of operations I want to
support are [E]CQL spatial predicates:
http://docs.geoserver.org/latest/en/user/filter/ecql_reference.html#spatial-predicate
but that perhaps deserves its own issue.
{quote}
I think we need to be cautious here about exposing too much complexity in the
Strategys. Query language requirements shouldn't be passed on down to
Strategy. Instead, the Strategys should have a very controlled list of spatial
operations they support and how they are connected to the query parser should
be the parser's responsibility. Requiring direct users of the Strategys to use
queryShape.getBoundingBox() seems like a good way to mitigate complexity in the
Strategys themselves and we can then do whatever we like in any parsers to make
our query languages work.
{quote}
Sorry, but I disagree with your point of view. The Strategy is supposed to be a
single facade to the implementation details of how a query will work, including
the various possible spatial predicates (i.e. spatial operations) that is
supports. If one Java class file shows that it becomes too complicated and it
would be better separated because implementing different predicates are just so
fundamentally different, then then the operations could be decomposed to
separate source files but it would be behind the facade of the Strategy.
{quote}
Okay fair enough. I think we can come to a compromise. My goal here is to
make it clear to the user what operations our Strategys support at compile
time, not through some undocumented runtime check. That seems a recipe for
disaster. Imagine someone who uses one of the Prefix Strategys and then tries
to do a Disjoint operation. At runtime they get an error and then after some
reading through source code they discover they actually need to use TwoDoubles
which requires a re-index.
Instead what I recommend is that we rename makeQuery to makeIntersectsQuery.
Then all implementations of that method will only construct a Query for the
intersects operation. We can then add makeXXXQuery methods to the Strategy
interface as we add support to all the implementations. If a Strategy impl
supports a particular operation that the rest don't, then that can just be a
method on that specific Strategy and not added to the Strategy interface.
Consequently TwoDoubles will get a makeDisjointQuery method. This way we have
more readable code, better compile time checking and less confused users.
How we map this into any Client / Server interaction or a query language should
be the responsibility of those classes, not the Strategys.
I'm going to create a patch to this effect.
> Remove the use of SpatialOperation
> ----------------------------------
>
> Key: LUCENE-4167
> URL: https://issues.apache.org/jira/browse/LUCENE-4167
> Project: Lucene - Java
> Issue Type: Bug
> Components: modules/spatial
> Reporter: Chris Male
>
> Looking at the code in TwoDoublesStrategy I noticed
> SpatialOperations.BBoxWithin vs isWithin which confused me. Looking over the
> other Strategys I see that really only isWithin and Intersects is supported.
> Only TwoDoublesStrategy supports IsDisjointTo. The remainder of
> SpatialOperations are not supported.
> I don't think we should use SpatialOperation as this stage since it is not
> clear what Operations are supported by what Strategys, many Operations are
> not supported, and the code for handling the Operations is usually the same.
> We can spin off the code for TwoDoublesStrategy's IsDisjointTo support into a
> different Strategy.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
