Hi Nuno,
No problem about the misformatting, it is a very common thing for people
contributing initial patches. The new patch looks much nicer, thanks!
So, its looking good for the most part. Although it looks like 3d is only
respected if the 2d hint is specified to be false. I think it should be
respected if the 2d hint is false, or the 2d hint is not specified at all.
Also one thing is that we need to deprecate the old
SQLDialect.encodeColumnName method, and probably have the implementation
call through to the new one.
As for the test i think it looks good, although with the behavior i
mentioned above about interpreting the 2d hint it should no longer be
necessary to override any of hte test methods in Postgis3D test.
-Justin
On Thu, May 3, 2012 at 11:15 AM, Nuno Miguel Carvalho Oliveira <
[email protected]> wrote:
> Hi Justin,
>
> I'm really sorry about the patch i haven't seen the formatting trick
> (sorry for the waste of time).
> I send in annex two patchs one who makes the changes in the existing
> classes and another to add the PostGIS 3D tests.
>
> I hope i did it well this time.
>
> Regards,
>
> Nuno Oliveira
>
>
>
>
>
> 2012/5/3 Justin Deoliveira <[email protected]>
>
>> Hi Nuno,
>>
>> The changes make sense, unfortunately the patch still has issues. The
>> patch contains largely formatting changes. Looks like spaces were converted
>> to tabs at some point. This unfortunately makes it nearly impossible to
>> figure out what actually changed in that class. Seems to be mostly
>> the PostGISDialect class, but also the test classes contain misformatting
>> in terms of tabs vs spaces.
>>
>> Not sure what IDE you are using but if you are using eclipse there are
>> existing formatting templates that will help with this. See this for more
>> details.
>>
>> http://docs.geotools.org/latest/developer/conventions/code/style.html
>>
>> Again, to reiterate, for any patches that are proposed it is imperative
>> that they only contain relevant changes, and no formatting changes.
>>
>> -Justin
>>
>>
>> On Wed, May 2, 2012 at 8:22 AM, Nuno Miguel Carvalho Oliveira <
>> [email protected]> wrote:
>>
>>> Hi,
>>>
>>> (Sorry for the late reply, i had a very very busy week).
>>>
>>> First, thank you for your very complete answer.
>>> I create a new patch based on your advices (in annex).
>>> Basically to read 3D coordinates from a PostGIS data
>>> base we need to put a hint in the query:
>>>
>>> Hints hints = query.getHints();
>>> hints.put(Hints.FEATURE_2D, Boolean.FALSE);
>>> query.setHints(hints);
>>>
>>> This hint will remove 'ST_Force_2D' from the final querry
>>> and set the encode to 'ST_AsEWKB' instead of 'ST_AsBinary'.
>>>
>>> I run the tests and all works fine (PostGIS3DTest).
>>>
>>> To resume what i have changed:
>>>
>>> - SQLDialect.java and PostGISDialect.java: I extend the
>>> 'encodeGeometryColumn' function to take also the 'Hints'
>>> as a parameter.
>>> - JDBCDataStore.java: I give the 'Hints' as a parameter
>>> to the 'encodeGeometryColumn' function.
>>> - PostGIS3DTest and PostGIS3DTestSetup: I add 3D tests
>>> to PostGIS.
>>>
>>>
>>> Any suggestions are very welcome.
>>>
>>> Regards,
>>>
>>> Nuno Oliveira
>>>
>>>
>>> 2012/4/21 Andrea Aime <[email protected]>
>>>
>>>> On Thu, Apr 19, 2012 at 7:01 AM, Nuno Miguel Carvalho Oliveira <
>>>> [email protected]> wrote:
>>>>
>>>>> Hi,
>>>>>
>>>>> Follows in anexe two patchs, one to integrate W3DS as a community
>>>>> module in GeoServer and another to GeoTools who permits the acess of 3D
>>>>> data from a PostGIS data base. Related to the acess of 3D data in GeoTools
>>>>> maybe it exists a better way to do this, any idea is welcome.
>>>>>
>>>>>
>>>> Hi,
>>>> let's start discussing the Geotools patch, since it's really short.
>>>> The patch is almost correct, but it's mis-using the hints somehow.
>>>>
>>>> Hints are used in the query as a way to tell the store that we'd
>>>> like the returned objects to have some particular property, such as
>>>> being
>>>> built with a particular geometry factory, or being flat (2D).
>>>> The geometry column user data is instead used to carry some extra info
>>>> that describes it, such as the fact the column is a "geography" instead
>>>> of a
>>>> "geometry".
>>>>
>>>> Query hints are created per request and can vary over time, for example
>>>> WMS will always ask for 2D geometries, whilst W3DS or WFS won't.
>>>> Geometry descriptor attributes instead are structural, once set they
>>>> describe
>>>> something about the geometry column "forever".
>>>>
>>>> As such you should not mix them, otherwise one request can force the
>>>> geometry to be 2d by attaching the hint and the next request looking for
>>>> 3d data won't get it (because the hint is already in place).
>>>> To do what you want is better to extend the encodeGeometryColumn in
>>>> SQLDialect to take also the Hints as a parameter: it is an API break
>>>> that
>>>> I believe we can take on trunk (Justin, thoughts?).
>>>>
>>>> The other thing that's missing is a test.
>>>> The JDBC testing framework is shared between all the databases and is
>>>> setup so that there is a base abstract class which needs to be
>>>> subclasses
>>>> and return a "test setup" object that knows how to setup the expected
>>>> spatial tables with the DDL specific of that particular database.
>>>> You can extend JDBC3DTest and JDBC3DTestSetup, which are
>>>> now implemented only for Oracle and DB2, and create an implementation
>>>> for PostGIS.
>>>>
>>>> I'll try to have a look at the W3DS community module too, but as long
>>>> as it's a
>>>> community module you don't really need to pass any review.
>>>> One thing is important tough, in order for GeoServer to include your
>>>> module
>>>> in the future you, or the company/authority/adminstration that employs
>>>> you
>>>> have to sign and send a "copyright assignment" contract that gives
>>>> Openplans,
>>>> the no-profit managing GeoServer, rights on the source code that you
>>>> created.
>>>> This is necessary to keep the ownership in one place and defend
>>>> GeoServer in
>>>> court should a license violation arise.
>>>>
>>>> Cheers
>>>> Andrea
>>>>
>>>> --
>>>> -------------------------------------------------------
>>>> Ing. Andrea Aime
>>>> GeoSolutions S.A.S.
>>>> Tech lead
>>>>
>>>> Via Poggio alle Viti 1187
>>>> 55054 Massarosa (LU)
>>>> Italy
>>>>
>>>> phone: +39 0584 962313
>>>> fax: +39 0584 962313
>>>> mob: +39 339 8844549
>>>>
>>>> http://www.geo-solutions.it
>>>> http://geo-solutions.blogspot.com/
>>>> http://www.youtube.com/user/GeoSolutionsIT
>>>> http://www.linkedin.com/in/andreaaime
>>>> http://twitter.com/geowolf
>>>>
>>>> -------------------------------------------------------
>>>>
>>>
>>>
>>
>>
>> --
>> Justin Deoliveira
>> OpenGeo - http://opengeo.org
>> Enterprise support for open source geospatial.
>>
>>
>
--
Justin Deoliveira
OpenGeo - http://opengeo.org
Enterprise support for open source geospatial.
------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and
threat landscape has changed and how IT managers can respond. Discussions
will include endpoint security, mobile security and the latest in malware
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
_______________________________________________
Geoserver-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/geoserver-devel
