Geospatial queries example fails in master

classic Classic list List threaded Threaded
10 messages Options
Reply | Threaded
Open this post in threaded view
|

Geospatial queries example fails in master

dmagda
Igniters,

I’ve just added the documentation about our geospatial queries [1] and wanted to contribute an example [2].

However, the example no longer works in the master branch producing the error you can see in the ticket [2].
The same example works perfectly fine in version 1.7 and earliest. You can download this project [3] and check that it’s true.

Sergi, please have a look at the issue reported in [1]. We’re approaching 1.8 release and looks like we overlooked some new bug introduced recently.

[1] http://apacheignite.gridgain.org/v1.7/docs/geospatial-queries <http://apacheignite.gridgain.org/v1.7/docs/geospatial-queries>
[2] https://issues.apache.org/jira/browse/IGNITE-4238 <https://issues.apache.org/jira/browse/IGNITE-4238>
[3] https://github.com/dmagda/geospatial <https://github.com/dmagda/geospatial>


Denis
Reply | Threaded
Open this post in threaded view
|

Re: Geospatial queries example fails in master

Andrew Mashenkov
Hi Denis,

Sql query parameters have being converted to binary before query is sent to
QueryProcessor. But they haven't being deserialized by QueryProcessor on
binding.
So, H2 failed to execute query due to wrong parameter type.

I've added PR [1] with fix. Sergi, please review if it correct place and
way to deserialize parameters.

[1] https://github.com/apache/ignite/pull/1248


On Thu, Nov 17, 2016 at 4:30 AM, Denis Magda <[hidden email]> wrote:

> Igniters,
>
> I’ve just added the documentation about our geospatial queries [1] and
> wanted to contribute an example [2].
>
> However, the example no longer works in the master branch producing the
> error you can see in the ticket [2].
> The same example works perfectly fine in version 1.7 and earliest. You can
> download this project [3] and check that it’s true.
>
> Sergi, please have a look at the issue reported in [1]. We’re approaching
> 1.8 release and looks like we overlooked some new bug introduced recently.
>
> [1] http://apacheignite.gridgain.org/v1.7/docs/geospatial-queries <
> http://apacheignite.gridgain.org/v1.7/docs/geospatial-queries>
> [2] https://issues.apache.org/jira/browse/IGNITE-4238 <
> https://issues.apache.org/jira/browse/IGNITE-4238>
> [3] https://github.com/dmagda/geospatial <https://github.com/dmagda/
> geospatial>
>
> —
> Denis




--
С уважением,
Машенков Андрей Владимирович
Тел. +7-921-932-61-82

Best regards,
Andrey V. Mashenkov
Cerr: +7-921-932-61-82
Reply | Threaded
Open this post in threaded view
|

Re: Geospatial queries example fails in master

Sergi
1. The change itself is buggy because if the parameter is null, then we
will call setNull and setObject after that. It must be `else if ...`

2. I'm not sure how this deserialization fits to our "no classes on server"
ideology.

Sergi



2016-11-18 14:26 GMT+03:00 Andrey Mashenkov <[hidden email]>:

> Hi Denis,
>
> Sql query parameters have being converted to binary before query is sent to
> QueryProcessor. But they haven't being deserialized by QueryProcessor on
> binding.
> So, H2 failed to execute query due to wrong parameter type.
>
> I've added PR [1] with fix. Sergi, please review if it correct place and
> way to deserialize parameters.
>
> [1] https://github.com/apache/ignite/pull/1248
>
>
> On Thu, Nov 17, 2016 at 4:30 AM, Denis Magda <[hidden email]> wrote:
>
> > Igniters,
> >
> > I’ve just added the documentation about our geospatial queries [1] and
> > wanted to contribute an example [2].
> >
> > However, the example no longer works in the master branch producing the
> > error you can see in the ticket [2].
> > The same example works perfectly fine in version 1.7 and earliest. You
> can
> > download this project [3] and check that it’s true.
> >
> > Sergi, please have a look at the issue reported in [1]. We’re approaching
> > 1.8 release and looks like we overlooked some new bug introduced
> recently.
> >
> > [1] http://apacheignite.gridgain.org/v1.7/docs/geospatial-queries <
> > http://apacheignite.gridgain.org/v1.7/docs/geospatial-queries>
> > [2] https://issues.apache.org/jira/browse/IGNITE-4238 <
> > https://issues.apache.org/jira/browse/IGNITE-4238>
> > [3] https://github.com/dmagda/geospatial <https://github.com/dmagda/
> > geospatial>
> >
> > —
> > Denis
>
>
>
>
> --
> С уважением,
> Машенков Андрей Владимирович
> Тел. +7-921-932-61-82
>
> Best regards,
> Andrey V. Mashenkov
> Cerr: +7-921-932-61-82
>
Reply | Threaded
Open this post in threaded view
|

Re: Geospatial queries example fails in master

Denis Magda
2. Most likely we don’t need to convert the spatial objects into the binary form.

I would investigate what has changed since the recent release where everything worked fine. Also don’t forget to add a test for the broken functionality.


Denis

> On Nov 18, 2016, at 8:54 AM, Sergi Vladykin <[hidden email]> wrote:
>
> 1. The change itself is buggy because if the parameter is null, then we
> will call setNull and setObject after that. It must be `else if ...`
>
> 2. I'm not sure how this deserialization fits to our "no classes on server"
> ideology.
>
> Sergi
>
>
>
> 2016-11-18 14:26 GMT+03:00 Andrey Mashenkov <[hidden email]>:
>
>> Hi Denis,
>>
>> Sql query parameters have being converted to binary before query is sent to
>> QueryProcessor. But they haven't being deserialized by QueryProcessor on
>> binding.
>> So, H2 failed to execute query due to wrong parameter type.
>>
>> I've added PR [1] with fix. Sergi, please review if it correct place and
>> way to deserialize parameters.
>>
>> [1] https://github.com/apache/ignite/pull/1248
>>
>>
>> On Thu, Nov 17, 2016 at 4:30 AM, Denis Magda <[hidden email]> wrote:
>>
>>> Igniters,
>>>
>>> I’ve just added the documentation about our geospatial queries [1] and
>>> wanted to contribute an example [2].
>>>
>>> However, the example no longer works in the master branch producing the
>>> error you can see in the ticket [2].
>>> The same example works perfectly fine in version 1.7 and earliest. You
>> can
>>> download this project [3] and check that it’s true.
>>>
>>> Sergi, please have a look at the issue reported in [1]. We’re approaching
>>> 1.8 release and looks like we overlooked some new bug introduced
>> recently.
>>>
>>> [1] http://apacheignite.gridgain.org/v1.7/docs/geospatial-queries <
>>> http://apacheignite.gridgain.org/v1.7/docs/geospatial-queries>
>>> [2] https://issues.apache.org/jira/browse/IGNITE-4238 <
>>> https://issues.apache.org/jira/browse/IGNITE-4238>
>>> [3] https://github.com/dmagda/geospatial <https://github.com/dmagda/
>>> geospatial>
>>>
>>> —
>>> Denis
>>
>>
>>
>>
>> --
>> С уважением,
>> Машенков Андрей Владимирович
>> Тел. +7-921-932-61-82
>>
>> Best regards,
>> Andrey V. Mashenkov
>> Cerr: +7-921-932-61-82
>>

Reply | Threaded
Open this post in threaded view
|

Re: Geospatial queries example fails in master

Andrew Mashenkov
Query parameters are converted to binary since commit "ae77653". This is
result of IGNITE-2208[1].

However, spatial queries work fine with OptimizedMarshaller configured. We
already have test GridH2IndexingGeoSelfTest, and we can run it with
BinaryMarshaller as well.


[1] https://issues.apache.org/jira/browse/IGNITE-2208


On Fri, Nov 18, 2016 at 8:20 PM, Denis Magda <[hidden email]> wrote:

> 2. Most likely we don’t need to convert the spatial objects into the
> binary form.
>
> I would investigate what has changed since the recent release where
> everything worked fine. Also don’t forget to add a test for the broken
> functionality.
>
> —
> Denis
>
> > On Nov 18, 2016, at 8:54 AM, Sergi Vladykin <[hidden email]>
> wrote:
> >
> > 1. The change itself is buggy because if the parameter is null, then we
> > will call setNull and setObject after that. It must be `else if ...`
> >
> > 2. I'm not sure how this deserialization fits to our "no classes on
> server"
> > ideology.
> >
> > Sergi
> >
> >
> >
> > 2016-11-18 14:26 GMT+03:00 Andrey Mashenkov <[hidden email]
> >:
> >
> >> Hi Denis,
> >>
> >> Sql query parameters have being converted to binary before query is
> sent to
> >> QueryProcessor. But they haven't being deserialized by QueryProcessor on
> >> binding.
> >> So, H2 failed to execute query due to wrong parameter type.
> >>
> >> I've added PR [1] with fix. Sergi, please review if it correct place and
> >> way to deserialize parameters.
> >>
> >> [1] https://github.com/apache/ignite/pull/1248
> >>
> >>
> >> On Thu, Nov 17, 2016 at 4:30 AM, Denis Magda <[hidden email]> wrote:
> >>
> >>> Igniters,
> >>>
> >>> I’ve just added the documentation about our geospatial queries [1] and
> >>> wanted to contribute an example [2].
> >>>
> >>> However, the example no longer works in the master branch producing the
> >>> error you can see in the ticket [2].
> >>> The same example works perfectly fine in version 1.7 and earliest. You
> >> can
> >>> download this project [3] and check that it’s true.
> >>>
> >>> Sergi, please have a look at the issue reported in [1]. We’re
> approaching
> >>> 1.8 release and looks like we overlooked some new bug introduced
> >> recently.
> >>>
> >>> [1] http://apacheignite.gridgain.org/v1.7/docs/geospatial-queries <
> >>> http://apacheignite.gridgain.org/v1.7/docs/geospatial-queries>
> >>> [2] https://issues.apache.org/jira/browse/IGNITE-4238 <
> >>> https://issues.apache.org/jira/browse/IGNITE-4238>
> >>> [3] https://github.com/dmagda/geospatial <https://github.com/dmagda/
> >>> geospatial>
> >>>
> >>> —
> >>> Denis
> >>
> >>
> >>
> >>
> >> --
> >> С уважением,
> >> Машенков Андрей Владимирович
> >> Тел. +7-921-932-61-82
> >>
> >> Best regards,
> >> Andrey V. Mashenkov
> >> Cerr: +7-921-932-61-82
> >>
>
>


--
С уважением,
Машенков Андрей Владимирович
Тел. +7-921-932-61-82

Best regards,
Andrey V. Mashenkov
Cerr: +7-921-932-61-82
Reply | Threaded
Open this post in threaded view
|

Re: Geospatial queries example fails in master

dmagda
Andrey,

Ok, now I see why the functionality has been broken. Basically, to fix the issue we should avoid the conversion of all “com.vividsolutions.jts parameters to the binary form. 

Please see in attach a possible fix. It would be more elegant to register the geospatial classes in BinaryContext but the classes are optional and I don’t see a way how to do this.

Vovan, is there more sophisticated fix for the issue or the one that is attached is already a generic one?

However, spatial queries work fine with OptimizedMarshaller configured. We
already have test GridH2IndexingGeoSelfTest, and we can run it with
BinaryMarshaller as well.

Alright, this is why that test didn’t fail. Yes, please make sure the test is executed for both optimized and binary marshallers.




Denis

On Nov 18, 2016, at 11:16 AM, Andrey Mashenkov <[hidden email]> wrote:

Query parameters are converted to binary since commit "ae77653". This is
result of IGNITE-2208[1].

However, spatial queries work fine with OptimizedMarshaller configured. We
already have test GridH2IndexingGeoSelfTest, and we can run it with
BinaryMarshaller as well.


[1] https://issues.apache.org/jira/browse/IGNITE-2208


On Fri, Nov 18, 2016 at 8:20 PM, Denis Magda <[hidden email]> wrote:

2. Most likely we don’t need to convert the spatial objects into the
binary form.

I would investigate what has changed since the recent release where
everything worked fine. Also don’t forget to add a test for the broken
functionality.


Denis

On Nov 18, 2016, at 8:54 AM, Sergi Vladykin <[hidden email]>
wrote:

1. The change itself is buggy because if the parameter is null, then we
will call setNull and setObject after that. It must be `else if ...`

2. I'm not sure how this deserialization fits to our "no classes on
server"
ideology.

Sergi



2016-11-18 14:26 GMT+03:00 Andrey Mashenkov <[hidden email]
:

Hi Denis,

Sql query parameters have being converted to binary before query is
sent to
QueryProcessor. But they haven't being deserialized by QueryProcessor on
binding.
So, H2 failed to execute query due to wrong parameter type.

I've added PR [1] with fix. Sergi, please review if it correct place and
way to deserialize parameters.

[1] https://github.com/apache/ignite/pull/1248


On Thu, Nov 17, 2016 at 4:30 AM, Denis Magda <[hidden email]> wrote:

Igniters,

I’ve just added the documentation about our geospatial queries [1] and
wanted to contribute an example [2].

However, the example no longer works in the master branch producing the
error you can see in the ticket [2].
The same example works perfectly fine in version 1.7 and earliest. You
can
download this project [3] and check that it’s true.

Sergi, please have a look at the issue reported in [1]. We’re
approaching
1.8 release and looks like we overlooked some new bug introduced
recently.

[1] http://apacheignite.gridgain.org/v1.7/docs/geospatial-queries <
http://apacheignite.gridgain.org/v1.7/docs/geospatial-queries>
[2] https://issues.apache.org/jira/browse/IGNITE-4238 <
https://issues.apache.org/jira/browse/IGNITE-4238>
[3] https://github.com/dmagda/geospatial <https://github.com/dmagda/
geospatial>


Denis




--
С уважением,
Машенков Андрей Владимирович
Тел. +7-921-932-61-82

Best regards,
Andrey V. Mashenkov
Cerr: +7-921-932-61-82





--
С уважением,
Машенков Андрей Владимирович
Тел. +7-921-932-61-82

Best regards,
Andrey V. Mashenkov
Cerr: +7-921-932-61-82


quick_fix.diff (1006 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Geospatial queries example fails in master

dmagda
Well,

The example still fails in the multi-node mode (SpatialQueryExampleMultiNodeSelfTest) which means that a similar filtering, that is applied in the patch from the previous message, has to be done in some other place. Unfortunately, I can’t keep at investigating the issue at the moment any longer. Andrey and Vovan, please complete it on your side.


Denis

> On Nov 18, 2016, at 12:40 PM, Denis Magda <[hidden email]> wrote:
>
> Andrey,
>
> Ok, now I see why the functionality has been broken. Basically, to fix the issue we should avoid the conversion of all “com.vividsolutions.jts” parameters to the binary form.
>
> Please see in attach a possible fix. It would be more elegant to register the geospatial classes in BinaryContext but the classes are optional and I don’t see a way how to do this.
>
> Vovan, is there more sophisticated fix for the issue or the one that is attached is already a generic one?
>
>> However, spatial queries work fine with OptimizedMarshaller configured. We
>> already have test GridH2IndexingGeoSelfTest, and we can run it with
>> BinaryMarshaller as well.
>
> Alright, this is why that test didn’t fail. Yes, please make sure the test is executed for both optimized and binary marshallers.
>
> <quick_fix.diff>
>
>
> —
> Denis
>
>> On Nov 18, 2016, at 11:16 AM, Andrey Mashenkov <[hidden email] <mailto:[hidden email]>> wrote:
>>
>> Query parameters are converted to binary since commit "ae77653". This is
>> result of IGNITE-2208[1].
>>
>> However, spatial queries work fine with OptimizedMarshaller configured. We
>> already have test GridH2IndexingGeoSelfTest, and we can run it with
>> BinaryMarshaller as well.
>>
>>
>> [1] https://issues.apache.org/jira/browse/IGNITE-2208 <https://issues.apache.org/jira/browse/IGNITE-2208>
>>
>>
>> On Fri, Nov 18, 2016 at 8:20 PM, Denis Magda <[hidden email] <mailto:[hidden email]>> wrote:
>>
>>> 2. Most likely we don’t need to convert the spatial objects into the
>>> binary form.
>>>
>>> I would investigate what has changed since the recent release where
>>> everything worked fine. Also don’t forget to add a test for the broken
>>> functionality.
>>>
>>> —
>>> Denis
>>>
>>>> On Nov 18, 2016, at 8:54 AM, Sergi Vladykin <[hidden email] <mailto:[hidden email]>>
>>> wrote:
>>>>
>>>> 1. The change itself is buggy because if the parameter is null, then we
>>>> will call setNull and setObject after that. It must be `else if ...`
>>>>
>>>> 2. I'm not sure how this deserialization fits to our "no classes on
>>> server"
>>>> ideology.
>>>>
>>>> Sergi
>>>>
>>>>
>>>>
>>>> 2016-11-18 14:26 GMT+03:00 Andrey Mashenkov <[hidden email] <mailto:[hidden email]>
>>>> :
>>>>
>>>>> Hi Denis,
>>>>>
>>>>> Sql query parameters have being converted to binary before query is
>>> sent to
>>>>> QueryProcessor. But they haven't being deserialized by QueryProcessor on
>>>>> binding.
>>>>> So, H2 failed to execute query due to wrong parameter type.
>>>>>
>>>>> I've added PR [1] with fix. Sergi, please review if it correct place and
>>>>> way to deserialize parameters.
>>>>>
>>>>> [1] https://github.com/apache/ignite/pull/1248 <https://github.com/apache/ignite/pull/1248>
>>>>>
>>>>>
>>>>> On Thu, Nov 17, 2016 at 4:30 AM, Denis Magda <[hidden email] <mailto:[hidden email]>> wrote:
>>>>>
>>>>>> Igniters,
>>>>>>
>>>>>> I’ve just added the documentation about our geospatial queries [1] and
>>>>>> wanted to contribute an example [2].
>>>>>>
>>>>>> However, the example no longer works in the master branch producing the
>>>>>> error you can see in the ticket [2].
>>>>>> The same example works perfectly fine in version 1.7 and earliest. You
>>>>> can
>>>>>> download this project [3] and check that it’s true.
>>>>>>
>>>>>> Sergi, please have a look at the issue reported in [1]. We’re
>>> approaching
>>>>>> 1.8 release and looks like we overlooked some new bug introduced
>>>>> recently.
>>>>>>
>>>>>> [1] http://apacheignite.gridgain.org/v1.7/docs/geospatial-queries <http://apacheignite.gridgain.org/v1.7/docs/geospatial-queries> <
>>>>>> http://apacheignite.gridgain.org/v1.7/docs/geospatial-queries <http://apacheignite.gridgain.org/v1.7/docs/geospatial-queries>>
>>>>>> [2] https://issues.apache.org/jira/browse/IGNITE-4238 <https://issues.apache.org/jira/browse/IGNITE-4238> <
>>>>>> https://issues.apache.org/jira/browse/IGNITE-4238 <https://issues.apache.org/jira/browse/IGNITE-4238>>
>>>>>> [3] https://github.com/dmagda/geospatial <https://github.com/dmagda/geospatial> <https://github.com/dmagda/ <https://github.com/dmagda/>
>>>>>> geospatial>
>>>>>>
>>>>>> —
>>>>>> Denis
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> --
>>>>> С уважением,
>>>>> Машенков Андрей Владимирович
>>>>> Тел. +7-921-932-61-82
>>>>>
>>>>> Best regards,
>>>>> Andrey V. Mashenkov
>>>>> Cerr: +7-921-932-61-82
>>>>>
>>>
>>>
>>
>>
>> --
>> С уважением,
>> Машенков Андрей Владимирович
>> Тел. +7-921-932-61-82
>>
>> Best regards,
>> Andrey V. Mashenkov
>> Cerr: +7-921-932-61-82
>

Reply | Threaded
Open this post in threaded view
|

Re: Geospatial queries example fails in master

Andrew Mashenkov
I've added GridH2IndexingGeoBinarySelfTest (GridH2IndexingGeoSelfTest with
BinaryMarshaller). Test failed on cache configuration step due to
inconsistent types.
It seems, excluding should be done in BinaryMarshaller internals.

Caused by: org.h2.message.DbException: Feature not supported: "spatial
index on non-geometry column, _VAL OTHER" [50100-191]
at org.h2.message.DbException.get(DbException.java:179)
at org.h2.message.DbException.get(DbException.java:155)
at org.h2.message.DbException.getUnsupportedException(DbException.java:216)
at
org.apache.ignite.internal.processors.query.h2.opt.GridH2SpatialIndex.<init>(GridH2SpatialIndex.java:104)


On Fri, Nov 18, 2016 at 11:57 PM, Denis Magda <[hidden email]> wrote:

> Well,
>
> The example still fails in the multi-node mode (
> SpatialQueryExampleMultiNodeSelfTest) which means that a similar
> filtering, that is applied in the patch from the previous message, has to
> be done in some other place. Unfortunately, I can’t keep at investigating
> the issue at the moment any longer. Andrey and Vovan, please complete it on
> your side.
>
> —
> Denis
>
> > On Nov 18, 2016, at 12:40 PM, Denis Magda <[hidden email]> wrote:
> >
> > Andrey,
> >
> > Ok, now I see why the functionality has been broken. Basically, to fix
> the issue we should avoid the conversion of all “com.vividsolutions.jts”
> parameters to the binary form.
> >
> > Please see in attach a possible fix. It would be more elegant to
> register the geospatial classes in BinaryContext but the classes are
> optional and I don’t see a way how to do this.
> >
> > Vovan, is there more sophisticated fix for the issue or the one that is
> attached is already a generic one?
> >
> >> However, spatial queries work fine with OptimizedMarshaller configured.
> We
> >> already have test GridH2IndexingGeoSelfTest, and we can run it with
> >> BinaryMarshaller as well.
> >
> > Alright, this is why that test didn’t fail. Yes, please make sure the
> test is executed for both optimized and binary marshallers.
> >
> > <quick_fix.diff>
> >
> >
> > —
> > Denis
> >
> >> On Nov 18, 2016, at 11:16 AM, Andrey Mashenkov <
> [hidden email] <mailto:[hidden email]>> wrote:
> >>
> >> Query parameters are converted to binary since commit "ae77653". This is
> >> result of IGNITE-2208[1].
> >>
> >> However, spatial queries work fine with OptimizedMarshaller configured.
> We
> >> already have test GridH2IndexingGeoSelfTest, and we can run it with
> >> BinaryMarshaller as well.
> >>
> >>
> >> [1] https://issues.apache.org/jira/browse/IGNITE-2208 <
> https://issues.apache.org/jira/browse/IGNITE-2208>
> >>
> >>
> >> On Fri, Nov 18, 2016 at 8:20 PM, Denis Magda <[hidden email]
> <mailto:[hidden email]>> wrote:
> >>
> >>> 2. Most likely we don’t need to convert the spatial objects into the
> >>> binary form.
> >>>
> >>> I would investigate what has changed since the recent release where
> >>> everything worked fine. Also don’t forget to add a test for the broken
> >>> functionality.
> >>>
> >>> —
> >>> Denis
> >>>
> >>>> On Nov 18, 2016, at 8:54 AM, Sergi Vladykin <[hidden email]
> <mailto:[hidden email]>>
> >>> wrote:
> >>>>
> >>>> 1. The change itself is buggy because if the parameter is null, then
> we
> >>>> will call setNull and setObject after that. It must be `else if ...`
> >>>>
> >>>> 2. I'm not sure how this deserialization fits to our "no classes on
> >>> server"
> >>>> ideology.
> >>>>
> >>>> Sergi
> >>>>
> >>>>
> >>>>
> >>>> 2016-11-18 14:26 GMT+03:00 Andrey Mashenkov <
> [hidden email] <mailto:[hidden email]>
> >>>> :
> >>>>
> >>>>> Hi Denis,
> >>>>>
> >>>>> Sql query parameters have being converted to binary before query is
> >>> sent to
> >>>>> QueryProcessor. But they haven't being deserialized by
> QueryProcessor on
> >>>>> binding.
> >>>>> So, H2 failed to execute query due to wrong parameter type.
> >>>>>
> >>>>> I've added PR [1] with fix. Sergi, please review if it correct place
> and
> >>>>> way to deserialize parameters.
> >>>>>
> >>>>> [1] https://github.com/apache/ignite/pull/1248 <
> https://github.com/apache/ignite/pull/1248>
> >>>>>
> >>>>>
> >>>>> On Thu, Nov 17, 2016 at 4:30 AM, Denis Magda <[hidden email]
> <mailto:[hidden email]>> wrote:
> >>>>>
> >>>>>> Igniters,
> >>>>>>
> >>>>>> I’ve just added the documentation about our geospatial queries [1]
> and
> >>>>>> wanted to contribute an example [2].
> >>>>>>
> >>>>>> However, the example no longer works in the master branch producing
> the
> >>>>>> error you can see in the ticket [2].
> >>>>>> The same example works perfectly fine in version 1.7 and earliest.
> You
> >>>>> can
> >>>>>> download this project [3] and check that it’s true.
> >>>>>>
> >>>>>> Sergi, please have a look at the issue reported in [1]. We’re
> >>> approaching
> >>>>>> 1.8 release and looks like we overlooked some new bug introduced
> >>>>> recently.
> >>>>>>
> >>>>>> [1] http://apacheignite.gridgain.org/v1.7/docs/geospatial-queries <
> http://apacheignite.gridgain.org/v1.7/docs/geospatial-queries> <
> >>>>>> http://apacheignite.gridgain.org/v1.7/docs/geospatial-queries <
> http://apacheignite.gridgain.org/v1.7/docs/geospatial-queries>>
> >>>>>> [2] https://issues.apache.org/jira/browse/IGNITE-4238 <
> https://issues.apache.org/jira/browse/IGNITE-4238> <
> >>>>>> https://issues.apache.org/jira/browse/IGNITE-4238 <
> https://issues.apache.org/jira/browse/IGNITE-4238>>
> >>>>>> [3] https://github.com/dmagda/geospatial <
> https://github.com/dmagda/geospatial> <https://github.com/dmagda/ <
> https://github.com/dmagda/>
> >>>>>> geospatial>
> >>>>>>
> >>>>>> —
> >>>>>> Denis
> >>>>>
> >>>>>
> >>>>>
> >>>>>
> >>>>> --
> >>>>> С уважением,
> >>>>> Машенков Андрей Владимирович
> >>>>> Тел. +7-921-932-61-82
> >>>>>
> >>>>> Best regards,
> >>>>> Andrey V. Mashenkov
> >>>>> Cerr: +7-921-932-61-82
> >>>>>
> >>>
> >>>
> >>
> >>
> >> --
> >> С уважением,
> >> Машенков Андрей Владимирович
> >> Тел. +7-921-932-61-82
> >>
> >> Best regards,
> >> Andrey V. Mashenkov
> >> Cerr: +7-921-932-61-82
> >
>
>


--
С уважением,
Машенков Андрей Владимирович
Тел. +7-921-932-61-82

Best regards,
Andrey V. Mashenkov
Cerr: +7-921-932-61-82
Reply | Threaded
Open this post in threaded view
|

Re: Geospatial queries example fails in master

Andrew Mashenkov
I've found BinaryContext.mustDeserialize(Class cls) method can prevent
serialization if it return true.

We can add a classname check against exclusions patterns, that can be found
in certain classpath, e.g. in "META/binary.context.excluded.classes". Then
file with exclusion patterns can be added to module jar along with geometry
classes jar only.

1) We will be able to add exclusions for BinaryMarshaller.
2) There will no mention of foreing classes in core module code.

Thoughts?

On Mon, Nov 21, 2016 at 6:11 PM, Andrey Mashenkov <
[hidden email]> wrote:

> I've added GridH2IndexingGeoBinarySelfTest (GridH2IndexingGeoSelfTest
> with BinaryMarshaller). Test failed on cache configuration step due to
> inconsistent types.
> It seems, excluding should be done in BinaryMarshaller internals.
>
> Caused by: org.h2.message.DbException: Feature not supported: "spatial
> index on non-geometry column, _VAL OTHER" [50100-191]
> at org.h2.message.DbException.get(DbException.java:179)
> at org.h2.message.DbException.get(DbException.java:155)
> at org.h2.message.DbException.getUnsupportedException(
> DbException.java:216)
> at org.apache.ignite.internal.processors.query.h2.opt.
> GridH2SpatialIndex.<init>(GridH2SpatialIndex.java:104)
>
>
> On Fri, Nov 18, 2016 at 11:57 PM, Denis Magda <[hidden email]> wrote:
>
>> Well,
>>
>> The example still fails in the multi-node mode
>> (SpatialQueryExampleMultiNodeSelfTest) which means that a similar
>> filtering, that is applied in the patch from the previous message, has to
>> be done in some other place. Unfortunately, I can’t keep at investigating
>> the issue at the moment any longer. Andrey and Vovan, please complete it on
>> your side.
>>
>> —
>> Denis
>>
>> > On Nov 18, 2016, at 12:40 PM, Denis Magda <[hidden email]> wrote:
>> >
>> > Andrey,
>> >
>> > Ok, now I see why the functionality has been broken. Basically, to fix
>> the issue we should avoid the conversion of all “com.vividsolutions.jts”
>> parameters to the binary form.
>> >
>> > Please see in attach a possible fix. It would be more elegant to
>> register the geospatial classes in BinaryContext but the classes are
>> optional and I don’t see a way how to do this.
>> >
>> > Vovan, is there more sophisticated fix for the issue or the one that is
>> attached is already a generic one?
>> >
>> >> However, spatial queries work fine with OptimizedMarshaller
>> configured. We
>> >> already have test GridH2IndexingGeoSelfTest, and we can run it with
>> >> BinaryMarshaller as well.
>> >
>> > Alright, this is why that test didn’t fail. Yes, please make sure the
>> test is executed for both optimized and binary marshallers.
>> >
>> > <quick_fix.diff>
>> >
>> >
>> > —
>> > Denis
>> >
>> >> On Nov 18, 2016, at 11:16 AM, Andrey Mashenkov <
>> [hidden email] <mailto:[hidden email]>> wrote:
>> >>
>> >> Query parameters are converted to binary since commit "ae77653". This
>> is
>> >> result of IGNITE-2208[1].
>> >>
>> >> However, spatial queries work fine with OptimizedMarshaller
>> configured. We
>> >> already have test GridH2IndexingGeoSelfTest, and we can run it with
>> >> BinaryMarshaller as well.
>> >>
>> >>
>> >> [1] https://issues.apache.org/jira/browse/IGNITE-2208 <
>> https://issues.apache.org/jira/browse/IGNITE-2208>
>> >>
>> >>
>> >> On Fri, Nov 18, 2016 at 8:20 PM, Denis Magda <[hidden email]
>> <mailto:[hidden email]>> wrote:
>> >>
>> >>> 2. Most likely we don’t need to convert the spatial objects into the
>> >>> binary form.
>> >>>
>> >>> I would investigate what has changed since the recent release where
>> >>> everything worked fine. Also don’t forget to add a test for the broken
>> >>> functionality.
>> >>>
>> >>> —
>> >>> Denis
>> >>>
>> >>>> On Nov 18, 2016, at 8:54 AM, Sergi Vladykin <
>> [hidden email] <mailto:[hidden email]>>
>> >>> wrote:
>> >>>>
>> >>>> 1. The change itself is buggy because if the parameter is null, then
>> we
>> >>>> will call setNull and setObject after that. It must be `else if ...`
>> >>>>
>> >>>> 2. I'm not sure how this deserialization fits to our "no classes on
>> >>> server"
>> >>>> ideology.
>> >>>>
>> >>>> Sergi
>> >>>>
>> >>>>
>> >>>>
>> >>>> 2016-11-18 14:26 GMT+03:00 Andrey Mashenkov <
>> [hidden email] <mailto:[hidden email]>
>> >>>> :
>> >>>>
>> >>>>> Hi Denis,
>> >>>>>
>> >>>>> Sql query parameters have being converted to binary before query is
>> >>> sent to
>> >>>>> QueryProcessor. But they haven't being deserialized by
>> QueryProcessor on
>> >>>>> binding.
>> >>>>> So, H2 failed to execute query due to wrong parameter type.
>> >>>>>
>> >>>>> I've added PR [1] with fix. Sergi, please review if it correct
>> place and
>> >>>>> way to deserialize parameters.
>> >>>>>
>> >>>>> [1] https://github.com/apache/ignite/pull/1248 <
>> https://github.com/apache/ignite/pull/1248>
>> >>>>>
>> >>>>>
>> >>>>> On Thu, Nov 17, 2016 at 4:30 AM, Denis Magda <[hidden email]
>> <mailto:[hidden email]>> wrote:
>> >>>>>
>> >>>>>> Igniters,
>> >>>>>>
>> >>>>>> I’ve just added the documentation about our geospatial queries [1]
>> and
>> >>>>>> wanted to contribute an example [2].
>> >>>>>>
>> >>>>>> However, the example no longer works in the master branch
>> producing the
>> >>>>>> error you can see in the ticket [2].
>> >>>>>> The same example works perfectly fine in version 1.7 and earliest.
>> You
>> >>>>> can
>> >>>>>> download this project [3] and check that it’s true.
>> >>>>>>
>> >>>>>> Sergi, please have a look at the issue reported in [1]. We’re
>> >>> approaching
>> >>>>>> 1.8 release and looks like we overlooked some new bug introduced
>> >>>>> recently.
>> >>>>>>
>> >>>>>> [1] http://apacheignite.gridgain.org/v1.7/docs/geospatial-queries
>> <http://apacheignite.gridgain.org/v1.7/docs/geospatial-queries> <
>> >>>>>> http://apacheignite.gridgain.org/v1.7/docs/geospatial-queries <
>> http://apacheignite.gridgain.org/v1.7/docs/geospatial-queries>>
>> >>>>>> [2] https://issues.apache.org/jira/browse/IGNITE-4238 <
>> https://issues.apache.org/jira/browse/IGNITE-4238> <
>> >>>>>> https://issues.apache.org/jira/browse/IGNITE-4238 <
>> https://issues.apache.org/jira/browse/IGNITE-4238>>
>> >>>>>> [3] https://github.com/dmagda/geospatial <
>> https://github.com/dmagda/geospatial> <https://github.com/dmagda/ <
>> https://github.com/dmagda/>
>> >>>>>> geospatial>
>> >>>>>>
>> >>>>>> —
>> >>>>>> Denis
>> >>>>>
>> >>>>>
>> >>>>>
>> >>>>>
>> >>>>> --
>> >>>>> С уважением,
>> >>>>> Машенков Андрей Владимирович
>> >>>>> Тел. +7-921-932-61-82
>> >>>>>
>> >>>>> Best regards,
>> >>>>> Andrey V. Mashenkov
>> >>>>> Cerr: +7-921-932-61-82
>> >>>>>
>> >>>
>> >>>
>> >>
>> >>
>> >> --
>> >> С уважением,
>> >> Машенков Андрей Владимирович
>> >> Тел. +7-921-932-61-82
>> >>
>> >> Best regards,
>> >> Andrey V. Mashenkov
>> >> Cerr: +7-921-932-61-82
>> >
>>
>>
>
>
> --
> С уважением,
> Машенков Андрей Владимирович
> Тел. +7-921-932-61-82
>
> Best regards,
> Andrey V. Mashenkov
> Cerr: +7-921-932-61-82
>



--
С уважением,
Машенков Андрей Владимирович
Тел. +7-921-932-61-82

Best regards,
Andrey V. Mashenkov
Cerr: +7-921-932-61-82
Reply | Threaded
Open this post in threaded view
|

Re: Geospatial queries example fails in master

dmagda
Have you tried to alter the code to check that the idea works? If it’s so, then let’s wait for Vladimir Ozerov input. He should confirm that this is what’s needed.


Denis

> On Nov 21, 2016, at 7:59 AM, Andrey Mashenkov <[hidden email]> wrote:
>
> I've found BinaryContext.mustDeserialize(Class cls) method can prevent
> serialization if it return true.
>
> We can add a classname check against exclusions patterns, that can be found
> in certain classpath, e.g. in "META/binary.context.excluded.classes". Then
> file with exclusion patterns can be added to module jar along with geometry
> classes jar only.
>
> 1) We will be able to add exclusions for BinaryMarshaller.
> 2) There will no mention of foreing classes in core module code.
>
> Thoughts?
>
> On Mon, Nov 21, 2016 at 6:11 PM, Andrey Mashenkov <
> [hidden email]> wrote:
>
>> I've added GridH2IndexingGeoBinarySelfTest (GridH2IndexingGeoSelfTest
>> with BinaryMarshaller). Test failed on cache configuration step due to
>> inconsistent types.
>> It seems, excluding should be done in BinaryMarshaller internals.
>>
>> Caused by: org.h2.message.DbException: Feature not supported: "spatial
>> index on non-geometry column, _VAL OTHER" [50100-191]
>> at org.h2.message.DbException.get(DbException.java:179)
>> at org.h2.message.DbException.get(DbException.java:155)
>> at org.h2.message.DbException.getUnsupportedException(
>> DbException.java:216)
>> at org.apache.ignite.internal.processors.query.h2.opt.
>> GridH2SpatialIndex.<init>(GridH2SpatialIndex.java:104)
>>
>>
>> On Fri, Nov 18, 2016 at 11:57 PM, Denis Magda <[hidden email]> wrote:
>>
>>> Well,
>>>
>>> The example still fails in the multi-node mode
>>> (SpatialQueryExampleMultiNodeSelfTest) which means that a similar
>>> filtering, that is applied in the patch from the previous message, has to
>>> be done in some other place. Unfortunately, I can’t keep at investigating
>>> the issue at the moment any longer. Andrey and Vovan, please complete it on
>>> your side.
>>>
>>> —
>>> Denis
>>>
>>>> On Nov 18, 2016, at 12:40 PM, Denis Magda <[hidden email]> wrote:
>>>>
>>>> Andrey,
>>>>
>>>> Ok, now I see why the functionality has been broken. Basically, to fix
>>> the issue we should avoid the conversion of all “com.vividsolutions.jts”
>>> parameters to the binary form.
>>>>
>>>> Please see in attach a possible fix. It would be more elegant to
>>> register the geospatial classes in BinaryContext but the classes are
>>> optional and I don’t see a way how to do this.
>>>>
>>>> Vovan, is there more sophisticated fix for the issue or the one that is
>>> attached is already a generic one?
>>>>
>>>>> However, spatial queries work fine with OptimizedMarshaller
>>> configured. We
>>>>> already have test GridH2IndexingGeoSelfTest, and we can run it with
>>>>> BinaryMarshaller as well.
>>>>
>>>> Alright, this is why that test didn’t fail. Yes, please make sure the
>>> test is executed for both optimized and binary marshallers.
>>>>
>>>> <quick_fix.diff>
>>>>
>>>>
>>>> —
>>>> Denis
>>>>
>>>>> On Nov 18, 2016, at 11:16 AM, Andrey Mashenkov <
>>> [hidden email] <mailto:[hidden email]>> wrote:
>>>>>
>>>>> Query parameters are converted to binary since commit "ae77653". This
>>> is
>>>>> result of IGNITE-2208[1].
>>>>>
>>>>> However, spatial queries work fine with OptimizedMarshaller
>>> configured. We
>>>>> already have test GridH2IndexingGeoSelfTest, and we can run it with
>>>>> BinaryMarshaller as well.
>>>>>
>>>>>
>>>>> [1] https://issues.apache.org/jira/browse/IGNITE-2208 <
>>> https://issues.apache.org/jira/browse/IGNITE-2208>
>>>>>
>>>>>
>>>>> On Fri, Nov 18, 2016 at 8:20 PM, Denis Magda <[hidden email]
>>> <mailto:[hidden email]>> wrote:
>>>>>
>>>>>> 2. Most likely we don’t need to convert the spatial objects into the
>>>>>> binary form.
>>>>>>
>>>>>> I would investigate what has changed since the recent release where
>>>>>> everything worked fine. Also don’t forget to add a test for the broken
>>>>>> functionality.
>>>>>>
>>>>>> —
>>>>>> Denis
>>>>>>
>>>>>>> On Nov 18, 2016, at 8:54 AM, Sergi Vladykin <
>>> [hidden email] <mailto:[hidden email]>>
>>>>>> wrote:
>>>>>>>
>>>>>>> 1. The change itself is buggy because if the parameter is null, then
>>> we
>>>>>>> will call setNull and setObject after that. It must be `else if ...`
>>>>>>>
>>>>>>> 2. I'm not sure how this deserialization fits to our "no classes on
>>>>>> server"
>>>>>>> ideology.
>>>>>>>
>>>>>>> Sergi
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> 2016-11-18 14:26 GMT+03:00 Andrey Mashenkov <
>>> [hidden email] <mailto:[hidden email]>
>>>>>>> :
>>>>>>>
>>>>>>>> Hi Denis,
>>>>>>>>
>>>>>>>> Sql query parameters have being converted to binary before query is
>>>>>> sent to
>>>>>>>> QueryProcessor. But they haven't being deserialized by
>>> QueryProcessor on
>>>>>>>> binding.
>>>>>>>> So, H2 failed to execute query due to wrong parameter type.
>>>>>>>>
>>>>>>>> I've added PR [1] with fix. Sergi, please review if it correct
>>> place and
>>>>>>>> way to deserialize parameters.
>>>>>>>>
>>>>>>>> [1] https://github.com/apache/ignite/pull/1248 <
>>> https://github.com/apache/ignite/pull/1248>
>>>>>>>>
>>>>>>>>
>>>>>>>> On Thu, Nov 17, 2016 at 4:30 AM, Denis Magda <[hidden email]
>>> <mailto:[hidden email]>> wrote:
>>>>>>>>
>>>>>>>>> Igniters,
>>>>>>>>>
>>>>>>>>> I’ve just added the documentation about our geospatial queries [1]
>>> and
>>>>>>>>> wanted to contribute an example [2].
>>>>>>>>>
>>>>>>>>> However, the example no longer works in the master branch
>>> producing the
>>>>>>>>> error you can see in the ticket [2].
>>>>>>>>> The same example works perfectly fine in version 1.7 and earliest.
>>> You
>>>>>>>> can
>>>>>>>>> download this project [3] and check that it’s true.
>>>>>>>>>
>>>>>>>>> Sergi, please have a look at the issue reported in [1]. We’re
>>>>>> approaching
>>>>>>>>> 1.8 release and looks like we overlooked some new bug introduced
>>>>>>>> recently.
>>>>>>>>>
>>>>>>>>> [1] http://apacheignite.gridgain.org/v1.7/docs/geospatial-queries
>>> <http://apacheignite.gridgain.org/v1.7/docs/geospatial-queries> <
>>>>>>>>> http://apacheignite.gridgain.org/v1.7/docs/geospatial-queries <
>>> http://apacheignite.gridgain.org/v1.7/docs/geospatial-queries>>
>>>>>>>>> [2] https://issues.apache.org/jira/browse/IGNITE-4238 <
>>> https://issues.apache.org/jira/browse/IGNITE-4238> <
>>>>>>>>> https://issues.apache.org/jira/browse/IGNITE-4238 <
>>> https://issues.apache.org/jira/browse/IGNITE-4238>>
>>>>>>>>> [3] https://github.com/dmagda/geospatial <
>>> https://github.com/dmagda/geospatial> <https://github.com/dmagda/ <
>>> https://github.com/dmagda/>
>>>>>>>>> geospatial>
>>>>>>>>>
>>>>>>>>> —
>>>>>>>>> Denis
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> --
>>>>>>>> С уважением,
>>>>>>>> Машенков Андрей Владимирович
>>>>>>>> Тел. +7-921-932-61-82
>>>>>>>>
>>>>>>>> Best regards,
>>>>>>>> Andrey V. Mashenkov
>>>>>>>> Cerr: +7-921-932-61-82
>>>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>> --
>>>>> С уважением,
>>>>> Машенков Андрей Владимирович
>>>>> Тел. +7-921-932-61-82
>>>>>
>>>>> Best regards,
>>>>> Andrey V. Mashenkov
>>>>> Cerr: +7-921-932-61-82
>>>>
>>>
>>>
>>
>>
>> --
>> С уважением,
>> Машенков Андрей Владимирович
>> Тел. +7-921-932-61-82
>>
>> Best regards,
>> Andrey V. Mashenkov
>> Cerr: +7-921-932-61-82
>>
>
>
>
> --
> С уважением,
> Машенков Андрей Владимирович
> Тел. +7-921-932-61-82
>
> Best regards,
> Andrey V. Mashenkov
> Cerr: +7-921-932-61-82