GeoTools
  1. GeoTools
  2. GEOT-1998

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

        Hide
        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
        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
        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
        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
        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
        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
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated: