GeoLatte / geolatte-geom

A geometry model that conforms to the OGC Simple Features for SQL specification.
Other
134 stars 63 forks source link

Store points in Oracle as SDO_POINT_TYPE instead of arrays of points. #42

Closed IgnacioCalvo closed 7 years ago

IgnacioCalvo commented 8 years ago

Hi there, first of all, apologies in case I don't follow your procedures, as I'm new in this project. Our use case is that we use Hibernate-spatial to store and query millions of points in Oracle that will be shown on a map. I saw in OracleJDBCTypeFactorythat the createStructwill store points as array of points (SDO_ORDINATE_ARRAY) instead of actual points (SDO_POINT_TYPE) which is not that performant. My current pull request suggests that we could use SDO_POINT_TYPE. I understand that maybe this is something that was already discussed in this project so I'm open to any kind of comments. Thanks for your time.

maesenka commented 8 years ago

Have you tested whether this actually increases performance? For what type of operations?

The reason this was never implemented is because I was very doubtful it would lead to a significant improvement in performance. Any data on this would therefore be very welcome.

IgnacioCalvo commented 8 years ago

Hi @maesenka ,

Thanks for your fast response.

We have always dealt with geometries as SDO_POINTS in the database. People in the forums would talk about the convenience of using it that way, but most would not provide an evidence. I could only find the reference in the official Oracle documentation : https://docs.oracle.com/cd/E11882_01/appdev.112/e11830/sdo_objrelschema.htm#SPATL493

You should store point geometries in the SDO_POINT attribute for optimal storage; and if you have only point geometries in a layer, it is strongly recommended that you store the point geometries in the SDO_POINT attribute.

Another reason was that it was easier for us to access x and y this way : geom_field.SDO_POINT.X and use it for different purposes : global clustering, etc. Probably performance was in the back of our minds, but I didn't test that myself.

Anyway I can understand your concerns about changing this now. But on the other side, I couldn't help bringing this up and share the idea.

From my personal perspective, I think that I may use SDO_POINT in my own implementation. From the community perspective, I guess it is up to those of you who know more about the project than me to decide if something like this goes in, stays away, or can be made configurable.

andrewleedev commented 8 years ago

Can somebody please move forward with making this decision? SDO_POINT has been an industry standard since inception.

Have you tested whether this actually increases performance? For what type of operations?

I work in the IT sector for the aeronautical industry. 80% of the data stored are points, and rarely lines or geometries, in both private and public sector. The use cases are vast, using SDO_POINT.

Many of the systems are legacy systems as well, in which SQL is written to retrieve the coordinates via SDO_POINT.X and SDO_POINT.Y. If we were to use Hibernate-Spatial to store points, the SQL for majority of the aeronautical industry will suffer, when "modern" technology such as Hibernate-Spatial is storing in a manner that "legacy" systems are unprepared to read. This circumstance is a near overhaul of the current IT systems. Great job security for software developers, but good luck convincing customers modernize what isn't broken.

The reason this was never implemented is because I was very doubtful it would lead to a significant improvement in performance. Any data on this would therefore be very welcome.

I doubt there would be any degradation in performance, in fact, even without testing, I would surmise the performance in terms of latency to be at least on par with current implementation, if not better. The improvement in performance in terms of storage, however, is Oracle's "strong recommendation".

Per Oracle's documentation. https://docs.oracle.com/cd/E11882_01/appdev.112/e11830/sdo_objrelschema.htm#SPATL493

matthias24 commented 7 years ago

I could use this change as well. Thumb up.

maesenka commented 7 years ago

I'll merge this in, but it needs to be controlled by a feature-toggle. If not we risk breaking client code that for some reason relies on the previous method of storing data.

maesenka commented 7 years ago

Can someone test this?

You must set a system property "GEOLATTE_USE_SDO_POINT_TYPE=true" to enable this feature.

matthias24 commented 7 years ago

Had not much time to test, but found out something strange. I loaded an Oracle Point with coordinates in the geom_field.SDO_POINT.X and Y, not in the SDO_ORDINATE_ARRAY. With the older version of geolatte I used, it worked. With these new version of geolatte the coordinates where not set properly in the Point class. The Problem seems to be in the class org.geolatte.geom.codec.db.oracle.PointSdoDecoder in Line 24. Here the code is if (nativeGeom.getOrdinates() == null) { but should be if (nativeGeom.getOrdinates() == null || nativeGeom.getOrdinates().getOrdinateArray().length == 0) { because the Ordinates class is not null, but the array contains no coordinates, as would be expected. Not sure, if the problem is realy here or already at the point of creating the SDOGeometry.

maesenka commented 7 years ago

This problem should be fixed now.