You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@lucene.apache.org by "Chris Male (JIRA)" <ji...@apache.org> on 2012/06/27 13:28:42 UTC
[jira] [Created] (LUCENE-4167) Remove the use of SpatialOperation
Chris Male created LUCENE-4167:
----------------------------------
Summary: 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: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[jira] [Commented] (LUCENE-4167) Remove the use of SpatialOperation
Posted by "David Smiley (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/LUCENE-4167?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13403181#comment-13403181 ]
David Smiley commented on LUCENE-4167:
--------------------------------------
bq. We don't need SpatialArgs at the moment. It feels like a hack to allow us to add whatever arguments we want without having to change method signature and so we can have some defaults.
The command pattern isn't a hack, I find it rather appropriate in this case, and I remember thinking it was a brilliant move by Ryan to have used it when I first saw it in Strategy for the first time. The distance precision is kind of a "hint" that is optional. On the other hand, the shape & operation are fundamental. I agree putting the operation into the method name does indeed make it clear which operations are supported, which is good for clarity and compile-time safety. But the compile-time safety is only realized if used directly by client code, not when the client is remote and passes a query string that is parsed and evaluated against a Strategy. This is what happens in Solr, in some of our testing, and I expect similarly for potential users like ElasticSearch when that eventually happens.
Curious, how do you propose compile-time safety be added to see what shape(s) a Strategy indexes? And what about multi-value compile-time checks?
bq. Now I think about it, what exactly are the purposes of makeQuery and makeValueSource()? I don't think it's clear.
I'll copy-paste the javadocs for you here:
{code:java}
/**
* Make a query which has a score based on the distance from the data to the query shape.
* The default implementation constructs a {@link FilteredQuery} based on
* {@link #makeFilter(com.spatial4j.core.query.SpatialArgs, SpatialFieldInfo)} and
* {@link #makeValueSource(com.spatial4j.core.query.SpatialArgs, SpatialFieldInfo)}.
*/
public Query makeQuery(SpatialArgs args, T fieldInfo) {
{code}
{code:java}
/**
* The value source yields a number that is proportional to the distance between the query shape and indexed data.
* @param args
* @param fieldInfo
*/
public abstract ValueSource makeValueSource(SpatialArgs args, T fieldInfo);
{code}
You rightly point out that the javadocs for makeValueSource is a bit vague. I thought it would be best to leave some wiggle room for different ways of calculating the distance instead of being precise. Do you think we should spell it out exactly and thus leave no room for alternatives? In terms of its relationship with makeQuery(), I think it makes sense to effectively fix the Strategy specification to essentially be what makeQuery()'s default impl does now (now as of last night anyway). So yes, the filter & query's matching documents should always be the same. Adding documentation to this affect would be good; one has to infer that at the moment based on the existing docs.
Here's an idea: what if makeQuery() goes away? Why is it needed when it can be constructed by a client quite simply like it is now:
{code:java}
public Query makeQuery(SpatialArgs args, T fieldInfo) {
Filter filter = makeFilter(args, fieldInfo);
ValueSource vs = makeValueSource(args, fieldInfo);
return new FilteredQuery(new FunctionQuery(vs), filter);
}
{code}
If it goes away, my acceptance level of putting the operation name into the method would be higher since you'd only have makeXXXXFilter without a Query variant.
{code}
> 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
> Attachments: LUCENE-4167.patch
>
>
> 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: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[jira] [Commented] (LUCENE-4167) Remove the use of SpatialOperation
Posted by "Chris Male (JIRA)" <ji...@apache.org>.
[ 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: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[jira] [Commented] (LUCENE-4167) Remove the use of SpatialOperation
Posted by "Chris Male (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/LUCENE-4167?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13402830#comment-13402830 ]
Chris Male commented on LUCENE-4167:
------------------------------------
{quote}
This is of course just one aspect of a Strategy's limitations, consider wether or not the Strategy supports multi-value data or wether it supports indexing non-point shapes. Surely that is quite relevant to a potential client. It seems very doubtful to me that the compile-time type checks could be added for everything
{quote}
Quite right and we can tackle these issues on a case by case basis. Having a check like supportsMultiValued() on Strategys seems like a good idea. That way the user can consult this method before indexing.
{quote}
And even with spatial operations – there are a lot of them to support, and wouldn't it be twice as many for both makeXXXQuery & makeXXXFilter? I don't know where you would draw the line. At least the current interface is fairly simple, and there is always Javadocs.
{quote}
We don't have any useful Javadocs on this issue so I'm not going to rely on that. I don't see any issue with having a makeXXXQuery/Filter for each operation. Strategys are essentially factories so I think the ability to see at compile time what the factory can create is vitally important. If we get to 20 operations I'll start to worry.
> 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
> Attachments: LUCENE-4167.patch
>
>
> 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: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[jira] [Commented] (LUCENE-4167) Remove the use of SpatialOperation
Posted by "David Smiley (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/LUCENE-4167?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13402827#comment-13402827 ]
David Smiley commented on LUCENE-4167:
--------------------------------------
I agree that something could/should be done to improve the awareness of exactly which operations a Strategy supports. This is of course just one aspect of a Strategy's limitations, consider wether or not the Strategy supports multi-value data or wether it supports indexing non-point shapes. Surely *that* is quite relevant to a potential client. It seems very doubtful to me that the compile-time type checks could be added for everything. And even with spatial operations -- there are a lot of them to support, and wouldn't it be twice as many for both makeXXXQuery & makeXXXFilter? I don't know where you would draw the line. At least the current interface is fairly simple, and there is always Javadocs.
That said, I look forward to seeing any patches you may having demonstrating what you have in mind. Maybe I just won't get it until I see it.
bq. How we map this into any Client / Server interaction or a query language should be the responsibility of those classes, not the Strategies.
True.
> 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
> Attachments: LUCENE-4167.patch
>
>
> 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: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[jira] [Commented] (LUCENE-4167) Remove the use of SpatialOperation
Posted by "Chris Male (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/LUCENE-4167?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13402870#comment-13402870 ]
Chris Male commented on LUCENE-4167:
------------------------------------
I don't see how it makes it harder to support more than one operation. If anything, it is making operations a 1st class citizen.
> 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
> Attachments: LUCENE-4167.patch
>
>
> 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: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[jira] [Commented] (LUCENE-4167) Remove the use of SpatialOperation
Posted by "Chris Male (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/LUCENE-4167?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13403055#comment-13403055 ]
Chris Male commented on LUCENE-4167:
------------------------------------
{quote}
makeFilter & makeQuery receives a SpatialArgs object which has already been parsed; it's not a string anymore. (The "Command" design pattern, by the way). And he's also proposing that SpatialArgs is gone, at least from these methods, and instead you get a Shape + distancePrecision pair of args.
{quote}
I gather you don't like that idea, it reads that way at least.
We don't need SpatialArgs at the moment. It feels like a hack to allow us to add whatever arguments we want without having to change method signature and so we can have some defaults. The Strategy API should be explicit so users know exactly what they need to provide (distance precision is used in both of the PrefixTreeStrategys, it is important) and we should consider any changes to the API very carefully. This API shouldnt have any mystery or surprises, it should be extremely clear to what user what to expect. I cannot stress this enough.
> 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
> Attachments: LUCENE-4167.patch
>
>
> 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: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[jira] [Commented] (LUCENE-4167) Remove the use of SpatialOperation
Posted by "Chris Male (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/LUCENE-4167?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13403642#comment-13403642 ]
Chris Male commented on LUCENE-4167:
------------------------------------
I don't see consensus being reached on how the Strategy API should look so I'm going to leave the patch. Go ahead and do your moves.
> 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
> Attachments: LUCENE-4167.patch
>
>
> 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: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[jira] [Commented] (LUCENE-4167) Remove the use of SpatialOperation
Posted by "Ryan McKinley (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/LUCENE-4167?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13402883#comment-13402883 ]
Ryan McKinley commented on LUCENE-4167:
---------------------------------------
This is essentially a java API vs 'query string' question. As is we essentially pass the query string all the way to the SpatialStrategy before it checks the operation; Chris proposes that the operation should be parsed *before* it gets to the strategy and have real java functions for each operation.
I support:
* removing the make* functions from SpatialStrategy
* for each possible strategy, we would create an interface like:
{code:java}
interface IntersectsSpatialQueryBuilder {
public Query makeIntersectsQuery(SpatialArgs args);
public Filter makeIntersectsFilter(SpatialArgs args);
}
interface IsWithinSpatialQueryBuilder {
public Query makeIsWithinQuery(SpatialArgs args);
public Filter makeIsWithinFilter(SpatialArgs args);
}
{code}
and then concrete SpatialStrategy implementations would implement everything they can do. This may be just Intersects, or it may be a list of 10 things it can do.
The advantage to this approach would be:
* clear java API and good place in javadocs to give the inevitable caveats for the operation implementation details
* testing can check for instances of 'IsWithinSpatialQueryBuilder'
The end user query string can still look like "INTERSECTS( xxxx )" but this would let us take that parsing out of the indexing class.
- - - -
I'm going to go ahead and push the BBox strategy into trunk because it is a strategy with a bunch of operations, and i want to make sure general operations remain a top level concept.
> 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
> Attachments: LUCENE-4167.patch
>
>
> 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: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[jira] [Commented] (LUCENE-4167) Remove the use of SpatialOperation
Posted by "Chris Male (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/LUCENE-4167?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13402865#comment-13402865 ]
Chris Male commented on LUCENE-4167:
------------------------------------
A follow up to this patch is to break out the operation specific makeXXXQuery/Filter into their own interfaces so the concrete Strategys can declare what operations they support. We would strip SpatialStrategy back to just having createFields and then each operation specific interface (say IntersectsStrategy) would extend it. If a concrete Strategy didnt care for any of the standard operations, then it could just implement SpatialStrategy and do what it likes. However those we have currently could implement IntersectsStrategy, and TwoDoubles could implement DisjointStrategy.
I'll update the patch.
> 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
> Attachments: LUCENE-4167.patch
>
>
> 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: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[jira] [Commented] (LUCENE-4167) Remove the use of SpatialOperation
Posted by "David Smiley (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/LUCENE-4167?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13402531#comment-13402531 ]
David Smiley commented on LUCENE-4167:
--------------------------------------
bq. I don't see the need to differentiate BBoxIntersects and Intersects. If the user wants to find those Documents related to the bounding box of a Shape, then they can call shape.getBoundingBox() and pass that into the Strategy. The Strategys shouldn't have to worry about the Shape (although TwoDoubles does but that needs to be re-thought separately). The Strategys should just take the Shape given and roll with it. Is that what you're suggesting?
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.
> 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: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[jira] [Comment Edited] (LUCENE-4167) Remove the use of
SpatialOperation
Posted by "Chris Male (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/LUCENE-4167?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13403059#comment-13403059 ]
Chris Male edited comment on LUCENE-4167 at 6/28/12 12:44 PM:
--------------------------------------------------------------
{quote}
Somewhat related to this is the Filter/Query distinction. Minutes ago I had the realization that makeQuery() could have a fixed implementation based on makeFilter() with makeValueSource() (for the distance). It's already committed so you can see. Assuming you also think it makes a good default implementation, this suggests any variety of makeXXXXQuery would call the same method, which seems less than ideal but not terrible.
{quote}
Now I think about it, what exactly are the purposes of makeQuery and makeValueSource()? I don't think it's clear. Does makeQuery() have to score the same Documents as makeFilter() identifies? We don't verify that they do. What does the score for makeQuery() mean? Is it always the distance between the centre of two Shapes? I can think of a IsDisJoint scenario where the two Shapes are disjoint but they have the same central point. Consequently is the score related to the operation? What is makeValueSource() all about then? does it return the same score for the same Document as makeQuery() just doesn't do any filtering as well?
Whatever the answer is to all these questions, we really need to document them.
was (Author: cmale):
{quote}
Somewhat related to this is the Filter/Query distinction. Minutes ago I had the realization that makeQuery() could have a fixed implementation based on makeFilter() with makeValueSource() (for the distance). It's already committed so you can see. Assuming you also think it makes a good default implementation, this suggests any variety of makeXXXXQuery would call the same method, which seems less than ideal but not terrible.
{quote}
Now I think about it, what exactly are the purposes of makeQuery and makeValueSource()? I don't think it's clear. Does makeQuery() have to score the same Documents as makeFilter() identifies? We don't verify that they do. What does the score for makeQuery() mean? Is it always the distance between the centre of two Shapes? I can think of a IsDisJoint scenario where the two Shapes are disjoint but they have the same central point. Consequently is the score related to the operation? What is makeValueSource() all about then? does it return the same score for the same Document as makeQuery() just doesn't do any filtering as well?
> 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
> Attachments: LUCENE-4167.patch
>
>
> 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: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[jira] [Commented] (LUCENE-4167) Remove the use of SpatialOperation
Posted by "Chris Male (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/LUCENE-4167?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13402286#comment-13402286 ]
Chris Male commented on LUCENE-4167:
------------------------------------
{quote}
Intersects – equivalent to IsWithin when index data is points
BBoxWIntersects – again, equivalent to BBoxIsWithin when the indexed data is points.
{quote}
I don't see the need to differentiate BBoxIntersects and Intersects. If the user wants to find those Documents related to the bounding box of a Shape, then they can call shape.getBoundingBox() and pass that into the Strategy. The Strategys shouldn't have to worry about the Shape (although TwoDoubles does but that needs to be re-thought separately). The Strategys should just take the Shape given and roll with it. Is that what you're suggesting?
{quote}
My concern with your suggestion to remove SpatialOperation is that I do think it will return. I know I want to work on an IsWithin when indexed data is shapes with area. And it is serving the purpose of SpatialArgsParser parsing out the operation you want to do, which I don't think should go away (i.e. the query string shouldn't assume an intersect, it should include "Intersects(...)" Perhaps the unsupported operations could be commented out?
{quote}
I can see the need for different behaviour for different Shape relationships to. But I think we should perhaps do that using method specialization. We already have the PrefixTreeStrategy abstraction, so you could write a WithinRecursivePrefixTreeStrategy which specialized makeQuery differently. That way it is clear to the user what the Strategy does, we won't need the runtime checks and we won't have Strategys like TwoDoubles which has methods for each of the different behaviours in the same class.
So I think we can remove the need for SpatialOperation now and support the idea differently in the future.
(As a side note, this actually makes me think we should decouple the indexing code of Strategys from the querying code).
{quote}
Separately, I think com.spatial4j.core.query.* belongs in Lucene spatial. It's not used by any of the rest of Spatial4j, yet it's tightly related to the concept of querying which is Lucene spatial's business, and is not the business of Spatial4j.
{quote}
+1. As a short term solution I think we just replicate the code that we need in Lucene now and then drop it from Spatial4J in the next release.
> 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: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[jira] [Commented] (LUCENE-4167) Remove the use of SpatialOperation
Posted by "David Smiley (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/LUCENE-4167?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13402591#comment-13402591 ]
David Smiley commented on LUCENE-4167:
--------------------------------------
bq. I can see the need for different behaviour for different Shape relationships to. But I think we should perhaps do that using method specialization. We already have the PrefixTreeStrategy abstraction, so you could write a WithinRecursivePrefixTreeStrategy which specialized makeQuery differently. That way it is clear to the user what the Strategy does, we won't need the runtime checks and we won't have Strategys like TwoDoubles which has methods for each of the different behaviours in the same class.
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. I don't believe that TwoDoublesStrategy demonstrates complexity of a class trying to do too many things. I absolutely think TwoDoublesStrategy could be coded to be more clear. If it is as buggy/untested as I think it is and nobody wants to fix it (I don't), personally I think this strategy can go away.
> 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: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[jira] [Updated] (LUCENE-4167) Remove the use of SpatialOperation
Posted by "David Smiley (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/LUCENE-4167?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
David Smiley updated LUCENE-4167:
---------------------------------
Attachment: LUCENE-4167_migrate_com_spatial4j_core_query_to_Lucene_spatial.patch
> 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
> Attachments: LUCENE-4167.patch, LUCENE-4167_migrate_com_spatial4j_core_query_to_Lucene_spatial.patch, LUCENE-4167_migrate_com_spatial4j_core_query_to_Lucene_spatial.patch
>
>
> 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: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[jira] [Resolved] (LUCENE-4167) Remove the use of SpatialOperation
Posted by "David Smiley (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/LUCENE-4167?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
David Smiley resolved LUCENE-4167.
----------------------------------
Resolution: Won't Fix
> Remove the use of SpatialOperation
> ----------------------------------
>
> Key: LUCENE-4167
> URL: https://issues.apache.org/jira/browse/LUCENE-4167
> Project: Lucene - Core
> Issue Type: Bug
> Components: modules/spatial
> Reporter: Chris Male
> Attachments: LUCENE-4167_migrate_com_spatial4j_core_query_to_Lucene_spatial.patch, LUCENE-4167_migrate_com_spatial4j_core_query_to_Lucene_spatial.patch, LUCENE-4167.patch
>
>
> 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
For more information on JIRA, see: http://www.atlassian.com/software/jira
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[jira] [Updated] (LUCENE-4167) Remove the use of SpatialOperation
Posted by "David Smiley (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/LUCENE-4167?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
David Smiley updated LUCENE-4167:
---------------------------------
Attachment: LUCENE-4167_migrate_com_spatial4j_core_query_to_Lucene_spatial.patch
Attached is the patch copying com.spatial4j.core.query into Lucene spatial, and updating code to reference this instead. I also moved Spatial4j's UnsupportedSpatialOperation which is tightly related, into the same destination.
I'll commit shortly to both branches.
> 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
> Attachments: LUCENE-4167.patch, LUCENE-4167_migrate_com_spatial4j_core_query_to_Lucene_spatial.patch
>
>
> 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: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[jira] [Commented] (LUCENE-4167) Remove the use of SpatialOperation
Posted by "David Smiley (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/LUCENE-4167?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13402867#comment-13402867 ]
David Smiley commented on LUCENE-4167:
--------------------------------------
I looked at the patch. {{shrug}} Some things are marginally simpler but it seems like a step back for supporting anything other than one spatial operation. Please hold off for a few days before committing without more consensus; I'd like to hear Ryan's point of view on this issue as it's a big deal.
> 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
> Attachments: LUCENE-4167.patch
>
>
> 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: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[jira] [Commented] (LUCENE-4167) Remove the use of SpatialOperation
Posted by "David Smiley (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/LUCENE-4167?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13402888#comment-13402888 ]
David Smiley commented on LUCENE-4167:
--------------------------------------
Thanks for your input Ryan.
bq. Chris proposes that the operation should be parsed before it gets to the strategy and have real java functions for each operation.
makeFilter & makeQuery receives a SpatialArgs object which has already been parsed; it's not a string anymore. (The "Command" design pattern, by the way). And he's also proposing that SpatialArgs is gone, at least from these methods, and instead you get a Shape + distancePrecision pair of args.
Somewhat related to this is the Filter/Query distinction. Minutes ago I had the realization that makeQuery() could have a fixed implementation based on makeFilter() with makeValueSource() (for the distance). It's already committed so you can see. Assuming you also think it makes a good default implementation, this suggests any variety of makeXXXXQuery would call the same method, which seems less than ideal but not terrible.
> 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
> Attachments: LUCENE-4167.patch
>
>
> 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: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[jira] [Updated] (LUCENE-4167) Remove the use of SpatialOperation
Posted by "Chris Male (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/LUCENE-4167?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Chris Male updated LUCENE-4167:
-------------------------------
Attachment: LUCENE-4167.patch
First shot at this.
I completely removed SpatialArgs from the Strategy interface. We don't have so many parameters that we can't force them to be defined.
Changed makeQuery/makeFilter to makeIntersectsQuery/makeIntersectsFilter respectively.
I want to address the method javadocs before committing this.
> 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
> Attachments: LUCENE-4167.patch
>
>
> 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: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[jira] [Commented] (LUCENE-4167) Remove the use of SpatialOperation
Posted by "David Smiley (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/LUCENE-4167?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13402274#comment-13402274 ]
David Smiley commented on LUCENE-4167:
--------------------------------------
I agree with your complaint. The only two supported operations are:
* Intersects -- equivalent to IsWithin when index data is points
* BBoxWIntersects -- again, equivalent to BBoxIsWithin when the indexed data is points.
The distinction of "overlaps" with "intersects" seems dubious.
The bbox handling is universally handled in SpatialArgs.getShape() which checks the operation and returns the wrapping rectangle. So effectively the strategies need not even bother with the whole SpatialOperation concept, at least not at the moment.
My concern with your suggestion to remove SpatialOperation is that I do think it will return. I know I want to work on an IsWithin when indexed data is shapes with area. And it is serving the purpose of SpatialArgsParser parsing out the operation you want to do, which I don't think should go away (i.e. the query string shouldn't assume an intersect, it should include "Intersects(...)" Perhaps the unsupported operations could be commented out?
Separately, I think com.spatial4j.core.query.* belongs in Lucene spatial. It's not used by any of the rest of Spatial4j, yet it's tightly related to the concept of querying which is Lucene spatial's business, and is not the business of Spatial4j.
> 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: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[jira] [Commented] (LUCENE-4167) Remove the use of SpatialOperation
Posted by "David Smiley (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/LUCENE-4167?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13403444#comment-13403444 ]
David Smiley commented on LUCENE-4167:
--------------------------------------
Shall I move com.spatial4j.core.query.* into Lucene spatial now or will that create too many conflicts with your patch, Chris?
> 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
> Attachments: LUCENE-4167.patch
>
>
> 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: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[jira] [Commented] (LUCENE-4167) Remove the use of SpatialOperation
Posted by "Chris Male (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/LUCENE-4167?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13403056#comment-13403056 ]
Chris Male commented on LUCENE-4167:
------------------------------------
{quote}
This is essentially a java API vs 'query string' question. As is we essentially pass the query string all the way to the SpatialStrategy before it checks the operation; Chris proposes that the operation should be parsed before it gets to the strategy and have real java functions for each operation.
{quote}
This is exactly what I'm suggesting. The Strategys shouldn't be doing decision making, they are Factorys. The logic for parsing queries, validating, choosing Strategys, constructing Shapes and identifying operations, shouldn't be their concern at all.
> 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
> Attachments: LUCENE-4167.patch
>
>
> 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: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[jira] [Commented] (LUCENE-4167) Remove the use of SpatialOperation
Posted by "Chris Male (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/LUCENE-4167?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13403059#comment-13403059 ]
Chris Male commented on LUCENE-4167:
------------------------------------
{quote}
Somewhat related to this is the Filter/Query distinction. Minutes ago I had the realization that makeQuery() could have a fixed implementation based on makeFilter() with makeValueSource() (for the distance). It's already committed so you can see. Assuming you also think it makes a good default implementation, this suggests any variety of makeXXXXQuery would call the same method, which seems less than ideal but not terrible.
{quote}
Now I think about it, what exactly are the purposes of makeQuery and makeValueSource()? I don't think it's clear. Does makeQuery() have to score the same Documents as makeFilter() identifies? We don't verify that they do. What does the score for makeQuery() mean? Is it always the distance between the centre of two Shapes? I can think of a IsDisJoint scenario where the two Shapes are disjoint but they have the same central point. Consequently is the score related to the operation? What is makeValueSource() all about then? does it return the same score for the same Document as makeQuery() just doesn't do any filtering as well?
> 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
> Attachments: LUCENE-4167.patch
>
>
> 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: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org