[GEOT-5918] GeoPackage data store performance improvements - improve …

…primary key reading performance
geotools · Dec 27, 2017 · 123ec5b · 123ec5b
1 parent 88beeba
commit 123ec5b
Show file tree

Hide file tree

Showing 3 changed files with 31 additions and 7 deletions.
diff --git a/modules/library/jdbc/src/main/java/org/geotools/jdbc/JDBCDataStore.java b/modules/library/jdbc/src/main/java/org/geotools/jdbc/JDBCDataStore.java
@@ -71,7 +71,6 @@
 import org.geotools.feature.NameImpl;
 import org.geotools.feature.simple.SimpleFeatureBuilder;
 import org.geotools.feature.visitor.CountVisitor;
-import org.geotools.feature.visitor.FeatureAttributeVisitor;
 import org.geotools.feature.visitor.GroupByVisitor;
 import org.geotools.feature.visitor.LimitingVisitor;
 import org.geotools.filter.FilterAttributeExtractor;
@@ -2203,18 +2202,21 @@ protected String encodeFID(PrimaryKey pkey, ResultSet rs) throws SQLException, I
      */
     protected String encodeFID( PrimaryKey pkey, ResultSet rs, int offset ) throws SQLException, IOException {
         // no pk columns
-        if(pkey.getColumns().isEmpty()) {
+        List<PrimaryKeyColumn> columns = pkey.getColumns();
+        if(columns.isEmpty()) {
             return SimpleFeatureBuilder.createDefaultFeatureId();
         } 
 
         // just one, no need to build support structures
-        if(pkey.getColumns().size() == 1)
-            return rs.getString(offset+1);
+        if(columns.size() == 1) {
+            return dialect.getPkColumnValue(rs, columns.get(0), offset + 1);
+        }
 
         // more than one
         List<Object> keyValues = new ArrayList<Object>();
-        for(int i = 0; i < pkey.getColumns().size(); i++) {
-            String o = rs.getString(offset+i+1);
+        for(int i = 0; i < columns.size(); i++) {
+            PrimaryKeyColumn column = columns.get(i);
+            String o = dialect.getPkColumnValue(rs, columns.get(0), offset + i + 1);
             keyValues.add( o );
         }
         return encodeFID( keyValues );

diff --git a/modules/library/jdbc/src/main/java/org/geotools/jdbc/SQLDialect.java b/modules/library/jdbc/src/main/java/org/geotools/jdbc/SQLDialect.java
@@ -1490,4 +1490,12 @@ public void encodeAggregateFunctionPrefix(String function, StringBuffer sql) {
     public void encodeAggregateFunctionPostfix(String function, StringBuffer sql) {
         sql.append(")");
     }
+
+    /**
+     * Reads a primary key column value. By default uses {@link ResultSet#getString(int)}, subclasses
+     * can use a more efficient way should they wish to
+     */
+    public String getPkColumnValue(ResultSet rs, PrimaryKeyColumn pkey, int columnIdx) throws SQLException {
+        return rs.getString(columnIdx);
+    }
 }
diff --git a/modules/plugin/geopkg/src/main/java/org/geotools/geopkg/GeoPkgDialect.java b/modules/plugin/geopkg/src/main/java/org/geotools/geopkg/GeoPkgDialect.java
@@ -45,6 +45,7 @@
 import org.geotools.jdbc.PreparedFilterToSQL;
 import org.geotools.jdbc.PreparedStatementSQLDialect;
 import org.geotools.jdbc.PrimaryKey;
+import org.geotools.jdbc.PrimaryKeyColumn;
 import org.geotools.referencing.CRS;
 import org.opengis.feature.simple.SimpleFeatureType;
 import org.opengis.feature.type.AttributeDescriptor;
@@ -400,7 +401,7 @@ public Object getLastAutoGeneratedValue(String schemaName, String tableName, Str
             Connection cx) throws SQLException {
         Statement st = cx.createStatement();
         try {
-            ResultSet rs = st.executeQuery( "SELECT last_insert_rowid();");
+            ResultSet rs = st.executeQuery( "SELECT last_insert_rowid()");
             try {
                 if (rs.next()) {
                     return rs.getInt( 1 );
@@ -567,4 +568,17 @@ protected <T> T convert(Object value, Class<T> binding) {
         }
         return super.convert(value, binding);
     }
+
+    @Override
+    public String getPkColumnValue(ResultSet rs, PrimaryKeyColumn pkey, int columnIdx) throws SQLException {
+        // by spec primary key of a GeoPackage column is integer, but maybe other tables do not
+        // follow this rule. Calling rs.getInt avoids an inefficient block of code in sqlite driver
+        if (Integer.class.equals(pkey.getType())) {
+            // primary keys cannot contain null values, so no need to check rs.wasNull() and
+            // thus saving an expensive native call
+            return  String.valueOf(rs.getInt(columnIdx));
+        } else {
+            return rs.getString(columnIdx);
+        }
+    }
 }