GeoTools

SQLDialect.encodeGeometryEnvelope needs to pass in the descriptor and the connection

Details

  • Type: Bug Bug
  • Status: In Progress In Progress
  • Priority: Major Major
  • Resolution: Unresolved
  • Affects Version/s: None
  • Fix Version/s: 2.5.9
  • Component/s: jdbc
  • Labels:
    None

Description

With Oracle there are two functions to get the MBR. From the Oracle spatial docs:

The SDO_AGGR_MBR function, documented in Chapter 12, also returns the MBR of
geometries. The SDO_TUNE.EXTENT_OF function has better performance than the
SDO_AGGR_MBR function if the data is non-geodetic and if a spatial index is defined
on the geometry column; however, the SDO_TUNE.EXTENT_OF function is limited to
two-dimensional geometries, whereas the SDO_AGGR_MBR function is not. In
addition, the SDO_TUNE.EXTENT_OF function computes the extent for all
geometries in a table; by contrast, the SDO_AGGR_MBR function can operate on
subsets of rows.
The SDO_TUNE.EXTENT_OF function returns NULL if the data is inconsistent.
If a non-geodetic spatial index is used, this function may return an approximate MBR
that encloses the largest extent of data stored in the index, even if data was
subsequently deleted. This can occur because the function extracts MBR information
from a non-geodetic spatial index, if one exists.

SDO_AGGR_MBR will do a full scan of the table, we need to avoid it when possible.
This means we need to determine if the srid is considered geodetic by oracle
(look into the MDSYS.GEODETIC_SRIDS view) and then decide which function to use.
So we need the NATIVE_SRID and a connection to run the query.

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Andrea Aime added a comment -
Attaching partial patch, to really work this needs more changes
Show
Andrea Aime added a comment - Attaching partial patch, to really work this needs more changes
Hide
Permalink
Andrea Aime added a comment -
Justin, I realized only after partial coding that just modifying the method won't work.
When SDO_TUNE is used, the query must be (for a generic set of various geometries):
{code}
select SDO_TUNE(tableName, geom1) from dual
{code}

That "from dual" breaks the current code, which instead uses {{from tableName}}.
But the fun does not end here, in the case of Oracle, SDO_TUNE can be used only for flat, non geodetic geometries.
This means that it's not possible, in general, to extract the bounds of multiple geom columns in one shot as the
datastore is doing now, because the SDO_AGGR queries are instead expressed as:
{code}
select SDO_AGGR_MBR(geom) from tableName
{code}

Ouch... it seems to me Oracle could enjoy a custom datastore implementation that overrides the way bounds are computed?
This would mean removing the "final" modifier from JDBCDataStore and adding in the comments that subclassing the
JDBCDataStore class should be considered an extreme measure.
Oh, it's quite likely I'll also have to subclass JDBCDataStore in order to make versioning work with the new postgis datastore (I need to override the way primary keys are handled).
Show
Andrea Aime added a comment - Justin, I realized only after partial coding that just modifying the method won't work. When SDO_TUNE is used, the query must be (for a generic set of various geometries): {code} select SDO_TUNE(tableName, geom1) from dual {code} That "from dual" breaks the current code, which instead uses {{from tableName}}. But the fun does not end here, in the case of Oracle, SDO_TUNE can be used only for flat, non geodetic geometries. This means that it's not possible, in general, to extract the bounds of multiple geom columns in one shot as the datastore is doing now, because the SDO_AGGR queries are instead expressed as: {code} select SDO_AGGR_MBR(geom) from tableName {code} Ouch... it seems to me Oracle could enjoy a custom datastore implementation that overrides the way bounds are computed? This would mean removing the "final" modifier from JDBCDataStore and adding in the comments that subclassing the JDBCDataStore class should be considered an extreme measure. Oh, it's quite likely I'll also have to subclass JDBCDataStore in order to make versioning work with the new postgis datastore (I need to override the way primary keys are handled).
Hide
Permalink
Justin Deoliveira added a comment -
Hmmm... interesting. Well i would like to avoid subclassing at ALL costs. What I would rather do is perhaps add some sort of capabilities notion (or some flag) to the dialect. In which the datastore can check the flag and if set the datastore completely delegates to the dialect for computing bounds and does not try to do any of the work of building the query.
Show
Justin Deoliveira added a comment - Hmmm... interesting. Well i would like to avoid subclassing at ALL costs. What I would rather do is perhaps add some sort of capabilities notion (or some flag) to the dialect. In which the datastore can check the flag and if set the datastore completely delegates to the dialect for computing bounds and does not try to do any of the work of building the query.
Hide
Permalink
Andrea Aime added a comment -
Oh, I could go for that, it's trading a complexity in the code for one in the sql dialect, whose number of methods is ever growing.... personally I'd prefer to have just one method and have the default sql dialect to do the work done by the datastore now instead of having two more methods (caps and the full encode one) but I'm open to both options
Show
Andrea Aime added a comment - Oh, I could go for that, it's trading a complexity in the code for one in the sql dialect, whose number of methods is ever growing.... personally I'd prefer to have just one method and have the default sql dialect to do the work done by the datastore now instead of having two more methods (caps and the full encode one) but I'm open to both options
Hide
Permalink
Andrea Aime added a comment -
Ok, so let's define which methods need to be added to SQLDialect. What about:

boolean useVendorBoundsQuery(SimpleFeatureType featureType, Filter filter);
ReferencedEnvelope getBounds(SimpleFeatureType featureType, Filter filter, Connection cx);

The first can be used to avoid replicating all the code in the cases in which SDO_TUNE cannot be used anyways (when Filter is not INCLUDE)
and the second, well, basically replaces the one from JDBCDataStore when the dialect needs it.
Show
Andrea Aime added a comment - Ok, so let's define which methods need to be added to SQLDialect. What about: boolean useVendorBoundsQuery(SimpleFeatureType featureType, Filter filter); ReferencedEnvelope getBounds(SimpleFeatureType featureType, Filter filter, Connection cx); The first can be used to avoid replicating all the code in the cases in which SDO_TUNE cannot be used anyways (when Filter is not INCLUDE) and the second, well, basically replaces the one from JDBCDataStore when the dialect needs it.
Hide
Permalink
Justin Deoliveira added a comment -
Sounds good to me. Can we call the methods useNativeBounds(...) and getNativeBounds(...), i prefer the word native over vendor and having them both contain "native" more clearly defines they are related.
Show
Justin Deoliveira added a comment - Sounds good to me. Can we call the methods useNativeBounds(...) and getNativeBounds(...), i prefer the word native over vendor and having them both contain "native" more clearly defines they are related.

People

  • Assignee:
    Andrea Aime
    Reporter:
    Andrea Aime
Vote (0)
Watch (1)

Dates

  • Created:
    Updated: