Issue Details (XML | Word | Printable)

Key: NUCSPATIAL-10
Type: Bug Bug
Status: Closed Closed
Resolution: Fixed
Priority: Major Major
Assignee: Andy Jefferson
Reporter: Andy Jefferson
Votes: 0
Watchers: 0
Operations

If you were logged in you would be able to see more operations.
DataNucleus Types : Geospatial

Fix Spatial to work with new SQL query mechanism (JDOQL2/JPQL2)

Created: 10/May/10 08:34 AM   Updated: 24/May/10 03:45 PM   Resolved: 12/May/10 03:16 PM
Component/s: None
Affects Version/s: 2.0.0.release, 2.1.0.m2
Fix Version/s: 2.1.0.m3


 Description  « Hide
Changes are likely needed to make the spatial plugin work with the new SQL expression package(s). See NUCSPATIAL-4 for some details.

Sort Order: Ascending order - Click to sort in descending order
Andy Jefferson added a comment - 10/May/10 08:45 AM
GeometryLiteral had a cast to Number, from a cut-paste. Now removed in SVN trunk.

plugin.xml didn't have sql-expression entries for specific type mappings. Now fixed in SVN trunk

Andy Jefferson added a comment - 10/May/10 11:32 AM
Comment from NUCSPATIAL-11

"I checked out the current trunk. I can retrieve spatial objects. But I cannot do spatial filtering:

NestedThrowables:
org.datanucleus.store.rdbms.sql.expression.IllegalExpressionOperationException: Cannot perform operation ".within" on org.datanucleus.store.rdbms.sql.expression.UnboundExpression@107b954b]]

The Query was: "Spatial.within(point,:bbox)"

Andy Jefferson added a comment - 10/May/10 11:33 AM - edited
Post the stack trace, and full JDOQL query, and the generic query compilation information (in the log ... starts "QueryCompilation")

Jan added a comment - 10/May/10 12:24 PM - edited
Ok, first a short question, is datanucleus.query.JDOQL.implementation=JDOQL correct? (Because JDOQL2 does not work and would be another issue unrelated to the spatial plugin)

I| 13:13:57,112 INFO JDO:87 - Exception thrown
Cannot perform operation ".within" on org.datanucleus.store.rdbms.sql.expression.UnboundExpression@4821f9c0
org.datanucleus.store.rdbms.sql.expression.IllegalExpressionOperationException: Cannot perform operation ".within" on org.datanucleus.store.rdbms.sql.expression.UnboundExpression@4821f9c0
at org.datanucleus.store.rdbms.sql.expression.SQLExpression.invoke(SQLExpression.java:563)
at org.datanucleus.store.rdbms.query.QueryToSQLMapper.processInvokeExpression(QueryToSQLMapper.java:2485)
at org.datanucleus.query.evaluator.AbstractExpressionEvaluator.compilePrimaryExpression(AbstractExpressionEvaluator.java:196)
at org.datanucleus.query.evaluator.AbstractExpressionEvaluator.compileUnaryExpression(AbstractExpressionEvaluator.java:165)
at org.datanucleus.query.evaluator.AbstractExpressionEvaluator.compileAdditiveMultiplicativeExpression(AbstractExpressionEvaluator.java:144)
at org.datanucleus.query.evaluator.AbstractExpressionEvaluator.compileRelationalExpression(AbstractExpressionEvaluator.java:119)
at org.datanucleus.query.evaluator.AbstractExpressionEvaluator.compileOrAndExpression(AbstractExpressionEvaluator.java:65)
at org.datanucleus.query.evaluator.AbstractExpressionEvaluator.evaluate(AbstractExpressionEvaluator.java:46)
at org.datanucleus.query.expression.Expression.evaluate(Expression.java:324)
at org.datanucleus.store.rdbms.query.QueryToSQLMapper.compileFilter(QueryToSQLMapper.java:402)
at org.datanucleus.store.rdbms.query.QueryToSQLMapper.compile(QueryToSQLMapper.java:322)
at org.datanucleus.store.rdbms.query.JDOQLQuery.compileQueryFull(JDOQLQuery.java:777)
at org.datanucleus.store.rdbms.query.JDOQLQuery.compileInternal(JDOQLQuery.java:260)
at org.datanucleus.store.query.Query.executeQuery(Query.java:1643)
at org.datanucleus.store.query.Query.executeWithArray(Query.java:1514)
at org.datanucleus.jdo.JDOQuery.execute(JDOQuery.java:243)


This are the lines before the exception occurs:
log4j.category.DataNucleus=debug
log4j.category.DataNucleus.Datastore=debug
log4j.category.DataNucleus.Persistence=debug

I| 13:21:33,279 DEBUG Query:58 - JDOQL Query : Compiling "SELECT FROM mypackage.Node WHERE Spatial.within(point,:bbox)"
I| 13:21:33,282 DEBUG ClassLoading:58 - Class "java.lang.Spatial" was not found in the CLASSPATH [Class resolver called from org.datanucleus.util.Imports.resolveClassDeclaration (line=177)]
I| 13:21:33,283 DEBUG ClassLoading:58 - Class "mypackage.Spatial" was not found in the CLASSPATH [Class resolver called from org.datanucleus.util.Imports.resolveClassDeclaration (line=177)]
I| 13:21:33,284 DEBUG Query:58 - JDOQL Query : Compile Time = 5 ms
I| 13:21:33,284 DEBUG Query:58 - QueryCompilation:
  [filter:InvokeExpression{[VariableExpression{Spatial}].within(PrimaryExpression{point}, ParameterExpression{bbox})}]
  [symbols: bbox type=unknown, Spatial type=unknown, this type=mypackage.Node]
I| 13:21:33,284 DEBUG Query:58 - JDOQL Query : Compiling "SELECT FROM de.komoot.hcn.server.routing.geodata.model.Node WHERE Spatial.within(point,:bbox)" for datastore
I| 13:21:33,285 DEBUG Query:58 - >> QueryToSQL.processVariable expr=VariableExpression{Spatial} var=Spatial
I| 13:21:33,285 DEBUG Query:58 - >> QueryToSQL.processVariable (unbound) variable=Spatial is not yet bound so returning UnboundExpression



Jan added a comment - 10/May/10 02:01 PM
Ok, great, the extension point works (if someone else wants to try it: check out the trunk of core + rdbms.spatial).

Now, there is another issue: At least the JTS Geometries (can) use a two-field mapping, meaning there is the geometry in one field and a "user object" in another field. This currently causes a query to be generated like: within(geometry, bytea, geometry)
I debugged through the code but I don't know at which point I should fix the issue without breaking other stuff. Do you know where to fix it?

But, I fixed another issue: In SpatialHelperMethod line 94 it must be:
        SQLExpression left = new StringExpression(stmt, m, funcName, funcArgs);
(There was "Intersects" sill hard coded)

Jan

Andy Jefferson added a comment - 10/May/10 02:19 PM
SpatialHelperMethod : now in SVN. Thx.

UserObject : What is expected to be generated ? Example of one such query. Does it work with "JDOQL-Legacy" query implementation ?

Jan added a comment - 10/May/10 02:32 PM
Ok, the spatiel queries must only have two parameters, like Within(geometrya,geometryb) but as the geometry can, in case of a JTS geometry, consists in two fields (geometry and geometry_0) the query is compiled with this additional field, too.

So to answer your query what is expected: The SqlExpression.class representing the Geometry must only return the first mapping field in its sql representation. In the constructor in line 78 it iterates over all mappings, but for spatial query, it must only use the first.

As a quickfix, I modified the GeometryExpression class:

    public GeometryExpression(SQLStatement stmt, SQLTable table, JavaTypeMapping mapping)
    {
        super(stmt, table, forceSingleJavaTypeMappingAdapter(mapping));
    }

    private static JavaTypeMapping forceSingleJavaTypeMappingAdapter(final JavaTypeMapping tm) {
     return new JavaTypeMapping() {
    
public int getNumberOfDatastoreMappings() {
return 1;
}

                        //calling super for the other ~30 methods
       }
   }

With this hack, the query is compiled correctly because only the geometry field is added to the Spatial query, not the user object.

Clearly, this hack will break something because adding / retrieving spatial data from the repository will need both field. So we still need to find the correct place.

It worked fine with the old implementation in Datanucleus <=2.0.0-release

Does this explain the issue or should I try the Legacy implementation, too?


Andy Jefferson added a comment - 10/May/10 03:16 PM
DN2.1 JDOQL-Legacy is DN2.0 default. Totally different code. There is nothing in the expression code of legacy for spatial that selects non-UserObject mappings, but then the legacy expression code was an unintelligible pile anyway, so maybe there is something in there.

As far as how to do it with DN2.1 and the new SQL expressions :-
Currently it calls SpatialMethodHelper.getBooleanExpression(...). This could easily detect if one of the expressions is one with a user object and generate an expression that only uses the java type mapping for the geometry object part.

Jan added a comment - 11/May/10 10:57 AM
Ok, I'll try to fix it in that method and submit a patch, then.

Andy Jefferson added a comment - 12/May/10 03:03 PM
Something I found, that could help you fix that situation. If you look in org.datanucleus.store.mapped.mapping.jts.GeometryMapping there is a cloneMapping method. This returns a mapping that just has the basic geometry and so could be used when generating any GeometryExpression.

Andy Jefferson added a comment - 12/May/10 03:16 PM
In fact I added a change using this to SVN trunk. All test.jdo.spatial tests pass as well as or better than with legacy now.

Jan added a comment - 12/May/10 04:40 PM
Ah, great! Just checked out the trunk. I'll now try to reproduce an error from Yesterday with the Spatial.distance method / geometry literal. But maybe this is now fixed, too.

Jan added a comment - 18/May/10 04:06 PM
I didn't find any further bugs, the new jdo methods work great! Thank you.

> This would need the query method sql expressions to be changed to take into account that there is an invoked expression (point), and there is a single
> argument (before there was no invoked expression, and there were 2 arguments). This would be a separate JIRA since involves many new sql method classes
> adding (even though the code in them would be v similar to what is currently there). If you're interested in this, I could give an example and you could
> write a few

That would be a much nicer syntax, I think but I have to solve a mandatory issue for me, before. Maybe we should discuss it in the forum or another ticket: The question is how a structure like a street graph can be efficiently fetched: The best way is to fetch a bounding box of objects at once, rather than only a single object (street). Is it possible for me to add such a functionality to DN? The optimal solution would be to fetch a street, put the bounding box around in some kind of cache and if a street is not in the cache but has to be loaded lazily, another bounding box of streets is fetched. Just an Idea...