GeoTools
  1. GeoTools
  2. GEOT-2294

Sql encoder is not provided with enough context when filter uses an attribute which is not part of the returned feature type

    Details

    • Type: Improvement Improvement
    • Status: Closed Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.5.2
    • Fix Version/s: 2.5.3
    • Component/s: jdbc
    • Labels:
      None

      Description

      Looking into the Oracle CHAR issue yesterday (GEOT-2293) I noticed that when an attribute used for filtering is not part of the returned feature type, the encoder does not get enough context to decide what the type of the attribute is.
      The issue is simple, the filtered feature type gets passed down to the encoder instead of the full feature type, resulting in the filtering attribute information being missing.
      This in turn makes the literal encoder try to guess the type, with the usual problems when strings like "021" get turned into numbers and then compared against strings (which sometimes works, sometimes does not).

      The patch is simple, alwasy grab the full feature type when doing sql encoding. Actually, that is only needed when doing selects, so the patch is quite small. There is a test for this already, but it's evidently not covering all cases (and to be frank, I noticed the problem, but even Oracle was happy
      enough to compare the string with a number, and it was still doing the right thing).

        Activity

        Hide
        Andrea Aime added a comment -
        The full patch is just:

        ### Eclipse Workspace Patch 1.0
        #P gt-jdbc-core
        Index: src/main/java/org/geotools/jdbc/JDBCDataStore.java
        ===================================================================
        --- src/main/java/org/geotools/jdbc/JDBCDataStore.java (revision 32286)
        +++ src/main/java/org/geotools/jdbc/JDBCDataStore.java (working copy)
        @@ -2261,7 +2261,7 @@
              * statement
              */
             protected PreparedStatement selectSQLPS( SimpleFeatureType featureType, Filter filter, SortBy[] sort, Connection cx )
        - throws SQLException {
        + throws SQLException, IOException {
                 
                 StringBuffer sql = new StringBuffer();
                 sql.append("SELECT ");
        @@ -2305,7 +2305,7 @@
                 if (filter != null && !Filter.INCLUDE.equals(filter)) {
                     //encode filter
                     try {
        - toSQL = createPreparedFilterToSQL(featureType);
        + toSQL = createPreparedFilterToSQL(getSchema(featureType.getTypeName()));
                         sql.append(" ").append(toSQL.encodeToString(filter));
                     } catch (FilterToSQLException e) {
                         throw new RuntimeException(e);




        Ok to commit?
        Show
        Andrea Aime added a comment - The full patch is just: ### Eclipse Workspace Patch 1.0 #P gt-jdbc-core Index: src/main/java/org/geotools/jdbc/JDBCDataStore.java =================================================================== --- src/main/java/org/geotools/jdbc/JDBCDataStore.java (revision 32286) +++ src/main/java/org/geotools/jdbc/JDBCDataStore.java (working copy) @@ -2261,7 +2261,7 @@       * statement       */      protected PreparedStatement selectSQLPS( SimpleFeatureType featureType, Filter filter, SortBy[] sort, Connection cx ) - throws SQLException { + throws SQLException, IOException {                    StringBuffer sql = new StringBuffer();          sql.append("SELECT "); @@ -2305,7 +2305,7 @@          if (filter != null && !Filter.INCLUDE.equals(filter)) {              //encode filter              try { - toSQL = createPreparedFilterToSQL(featureType); + toSQL = createPreparedFilterToSQL(getSchema(featureType.getTypeName()));                  sql.append(" ").append(toSQL.encodeToString(filter));              } catch (FilterToSQLException e) {                  throw new RuntimeException(e); Ok to commit?
        Hide
        Justin Deoliveira added a comment -
        Patch looks good, apply as you see fit. One thing is you might want to add a quick comment as to why the feature type is being reloaded, since at a glance with no context one might assume that reload is unnecessary.
        Show
        Justin Deoliveira added a comment - Patch looks good, apply as you see fit. One thing is you might want to add a quick comment as to why the feature type is being reloaded, since at a glance with no context one might assume that reload is unnecessary.
        Hide
        Andrea Aime added a comment -
        Fixed in 2.5.x and trunk
        Show
        Andrea Aime added a comment - Fixed in 2.5.x and trunk

          People

          • Assignee:
            Justin Deoliveira
            Reporter:
            Andrea Aime
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: