{"action":"created","comment":{"url":"https://api.github.com/repos/geotools/geotools/pulls/comments/42965582","id":42965582,"diff_hunk":"@@ -1549,70 +1547,201 @@ protected void insert(SimpleFeature feature, SimpleFeatureType featureType, Conn\n      * Inserts a collection of new features into the database for a particular\n      * feature type / table.\n      */\n-    protected void insert(Collection features, SimpleFeatureType featureType, Connection cx)\n+    protected void insert(Collection<? extends SimpleFeature> features, SimpleFeatureType featureType, Connection cx)\n         throws IOException {\n         PrimaryKey key = getPrimaryKey(featureType);\n \n         // we do this in a synchronized block because we need to do two queries,\n         // first to figure out what the id will be, then the insert statement\n         synchronized (this) {\n-            Statement st = null;\n-\n             try {\n-                if ( !(dialect instanceof PreparedStatementSQLDialect) ) {\n-                    st = cx.createStatement();    \n+                if (dialect instanceof PreparedStatementSQLDialect) {\n+                    Map<InsertionClassifier, Collection<SimpleFeature>> kinds =\n+                            InsertionClassifier.classify(featureType, features);\n+                    for (InsertionClassifier kind: kinds.keySet()) {\n+                        insertPS(kinds.get(kind), kind, featureType, cx, key);\n+                    }\n+                } else {\n+                    Collection<SimpleFeature> useExistings = new ArrayList<>();\n+                    Collection<SimpleFeature> notUseExistings = new ArrayList<>();\n+                    for (SimpleFeature cur : features) {\n+                        (InsertionClassifier.useExisting(cur) ? useExistings : notUseExistings).\n+                                add(cur);\n+                    }\n+                    insertNonPS(useExistings, featureType, cx, key, true);\n+                    insertNonPS(notUseExistings, featureType, cx, key, false);\n                 }\n-                \n-                // figure out if we should determine what the fid is pre or post insert\n-                boolean postInsert = dialect.lookupGeneratedValuesPostInsert() && isGenerated(key);\n-                \n-                for (Iterator f = features.iterator(); f.hasNext();) {\n-                    SimpleFeature feature = (SimpleFeature) f.next();\n-                    \n-                    List<Object> keyValues = null;\n-                    boolean useExisting = Boolean.TRUE.equals(feature.getUserData().get(Hints.USE_PROVIDED_FID));\n-                    if(useExisting) {\n-                        keyValues = decodeFID(key, feature.getID(), true);\n-                    } else if (!postInsert) {\n-                        keyValues = getNextValues( key, cx );\n+            } catch (SQLException e) {\n+                String msg = \"Error inserting features\";\n+                throw (IOException) new IOException(msg).initCause(e);\n+            }\n+        }\n+    }\n+\n+    /**\n+     * Specialized insertion for dialects that are using prepared statements.\n+     */\n+    private void insertPS(Collection<SimpleFeature> features, InsertionClassifier kind,\n+                          SimpleFeatureType featureType, Connection cx, PrimaryKey key)\n+            throws IOException, SQLException {\n+        final PreparedStatementSQLDialect dialect = (PreparedStatementSQLDialect) getSQLDialect();\n+\n+        final KeysFetcher keysFetcher = KeysFetcher.create(this, cx,kind.useExisting, key);\n+\n+        final String sql = buildInsertPS(kind, featureType, keysFetcher, dialect);\n+        LOGGER.log(Level.FINE, \"Inserting new features with ps: {0}\", sql);\n+\n+        //create the prepared statement\n+        final PreparedStatement ps;\n+        if (keysFetcher.isPostInsert()) {\n+            // ask the DB to return the values of all the keys after the insertion\n+            ps = cx.prepareStatement(sql, keysFetcher.getColumnNames());\n+        } else {\n+            ps = cx.prepareStatement(sql);\n+        }\n+        try {\n+            for (SimpleFeature feature : features) {\n+                //set the attribute values\n+                int i = 1;\n+                for(AttributeDescriptor att : featureType.getAttributeDescriptors()) {\n+                    String colName = att.getLocalName();\n+                    // skip the pk columns in case we have exposed them, we grab the\n+                    // value from the pk itself\n+                    if(keysFetcher.isKey(colName)) {\n+                        continue;\n                     }\n-                    \n \n-                    if ( dialect instanceof PreparedStatementSQLDialect ) {\n-                        PreparedStatement ps = insertSQLPS( featureType, feature, keyValues, cx );\n-                        try {\n-                            ((PreparedStatementSQLDialect)dialect).onInsert(ps, cx, featureType);\n-                            ps.execute();\n-                        } finally {\n-                            closeSafe( ps );\n+                    Class binding = att.getType().getBinding();\n+\n+                    Object value = feature.getAttribute(colName);\n+                    /*if (value == null) {\n+                        if (!att.isNillable()) {\n+                            //TODO: throw an exception\n                         }\n+                    }*/\n+\n+                    if (Geometry.class.isAssignableFrom(binding)) {\n+                        Geometry g = (Geometry) value;\n+                        int srid = getGeometrySRID(g, att);\n+                        int dimension = getGeometryDimension(g, att);\n+                        dialect.setGeometryValue(g, dimension, srid, binding, ps, i);\n                     } else {\n-                        String sql = insertSQL(featureType, feature, keyValues, cx);\n-                        \n-                        //TODO: execute in batch to improve performance?\n-                        ((BasicSQLDialect)dialect).onInsert(st, cx, featureType);\n-                        \n-                        LOGGER.log(Level.FINE, \"Inserting new feature: {0}\", sql);\n-                        st.execute(sql);\n+                        dialect.setValue(value, binding, ps, i, cx);\n                     }\n-                    \n-                    if ( keyValues == null ) {\n-                        //grab the key values post insert\n-                        keyValues = getLastValues(key,cx);\n+                    if ( LOGGER.isLoggable( Level.FINE ) ) {\n+                        LOGGER.fine( (i) + \" = \" + value );\n                     }\n-                    \n-                    //report the feature id as user data since we cant set the fid\n-                    String fid = featureType.getTypeName() + \".\" + encodeFID(keyValues);\n-                    feature.getUserData().put(\"fid\", fid);\n+                    i++;\n                 }\n \n-                //st.executeBatch();\n-            } catch (SQLException e) {\n-                String msg = \"Error inserting features\";\n-                throw (IOException) new IOException(msg).initCause(e);\n-            } finally {\n-                closeSafe(st);\n+                keysFetcher.setKeyValues(dialect, ps, cx, featureType, feature, i);\n+\n+                dialect.onInsert(ps, cx, featureType);\n+                ps.addBatch();\n+            }\n+            int[] inserts = ps.executeBatch();\n+            checkAllInserted(inserts, features.size());\n+            keysFetcher.postInsert(featureType, features, ps);\n+        } finally {\n+            closeSafe(ps);\n+        }\n+    }\n+\n+    private static void checkAllInserted(int[] inserts, int size) throws IOException {","path":"modules/library/jdbc/src/main/java/org/geotools/jdbc/JDBCDataStore.java","position":160,"original_position":160,"commit_id":"35be6b3aac4011fb3041f8ad1174bb673712ed06","original_commit_id":"35be6b3aac4011fb3041f8ad1174bb673712ed06","user":{"login":"pvalsecc","id":1180608,"avatar_url":"https://avatars.githubusercontent.com/u/1180608?v=3","gravatar_id":"","url":"https://api.github.com/users/pvalsecc","html_url":"https://github.com/pvalsecc","followers_url":"https://api.github.com/users/pvalsecc/followers","following_url":"https://api.github.com/users/pvalsecc/following{/other_user}","gists_url":"https://api.github.com/users/pvalsecc/gists{/gist_id}","starred_url":"https://api.github.com/users/pvalsecc/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/pvalsecc/subscriptions","organizations_url":"https://api.github.com/users/pvalsecc/orgs","repos_url":"https://api.github.com/users/pvalsecc/repos","events_url":"https://api.github.com/users/pvalsecc/events{/privacy}","received_events_url":"https://api.github.com/users/pvalsecc/received_events","type":"User","site_admin":false},"body":"Because it's more a function than a method and it's more testable like that. By the way, I'v forgot to add the test. Fixing that.","created_at":"2015-10-26T07:38:19Z","updated_at":"2015-10-26T07:38:19Z","html_url":"https://github.com/geotools/geotools/pull/1009#discussion_r42965582","pull_request_url":"https://api.github.com/repos/geotools/geotools/pulls/1009","_links":{"self":{"href":"https://api.github.com/repos/geotools/geotools/pulls/comments/42965582"},"html":{"href":"https://github.com/geotools/geotools/pull/1009#discussion_r42965582"},"pull_request":{"href":"https://api.github.com/repos/geotools/geotools/pulls/1009"}}},"pull_request":{"url":"https://api.github.com/repos/geotools/geotools/pulls/1009","id":48340135,"html_url":"https://github.com/geotools/geotools/pull/1009","diff_url":"https://github.com/geotools/geotools/pull/1009.diff","patch_url":"https://github.com/geotools/geotools/pull/1009.patch","issue_url":"https://api.github.com/repos/geotools/geotools/issues/1009","number":1009,"state":"open","locked":false,"title":"[GEOT-5264] Batch inserts","user":{"login":"pvalsecc","id":1180608,"avatar_url":"https://avatars.githubusercontent.com/u/1180608?v=3","gravatar_id":"","url":"https://api.github.com/users/pvalsecc","html_url":"https://github.com/pvalsecc","followers_url":"https://api.github.com/users/pvalsecc/followers","following_url":"https://api.github.com/users/pvalsecc/following{/other_user}","gists_url":"https://api.github.com/users/pvalsecc/gists{/gist_id}","starred_url":"https://api.github.com/users/pvalsecc/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/pvalsecc/subscriptions","organizations_url":"https://api.github.com/users/pvalsecc/orgs","repos_url":"https://api.github.com/users/pvalsecc/repos","events_url":"https://api.github.com/users/pvalsecc/events{/privacy}","received_events_url":"https://api.github.com/users/pvalsecc/received_events","type":"User","site_admin":false},"body":"Those commits changes geotools to group the features to be inserted by batch and uses JDBC batch insert to speed up big insertions. It improves as well the way we get the primary key values (avoid doing on select per primary key field and per feature inserted).\r\n\r\nI ran the online tests for those DBs:    \r\n* Postgis\r\n* H2\r\n* Mysql\r\n* Oracle\r\n\r\nImprovements I've mesured with a geogig Oracle export of a table with 17000 records (4 simple attributes and a single point geometry) without sequence is run in:\r\n* 640 seconds without my modifs\r\n* 13 seconds with my modifs","created_at":"2015-10-21T14:38:52Z","updated_at":"2015-10-26T07:38:20Z","closed_at":null,"merged_at":null,"merge_commit_sha":"6cd13a0a3e27667229857304113988c9b319328c","assignee":null,"milestone":null,"commits_url":"https://api.github.com/repos/geotools/geotools/pulls/1009/commits","review_comments_url":"https://api.github.com/repos/geotools/geotools/pulls/1009/comments","review_comment_url":"https://api.github.com/repos/geotools/geotools/pulls/comments{/number}","comments_url":"https://api.github.com/repos/geotools/geotools/issues/1009/comments","statuses_url":"https://api.github.com/repos/geotools/geotools/statuses/35be6b3aac4011fb3041f8ad1174bb673712ed06","head":{"label":"pvalsecc:batch_inserts","ref":"batch_inserts","sha":"35be6b3aac4011fb3041f8ad1174bb673712ed06","user":{"login":"pvalsecc","id":1180608,"avatar_url":"https://avatars.githubusercontent.com/u/1180608?v=3","gravatar_id":"","url":"https://api.github.com/users/pvalsecc","html_url":"https://github.com/pvalsecc","followers_url":"https://api.github.com/users/pvalsecc/followers","following_url":"https://api.github.com/users/pvalsecc/following{/other_user}","gists_url":"https://api.github.com/users/pvalsecc/gists{/gist_id}","starred_url":"https://api.github.com/users/pvalsecc/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/pvalsecc/subscriptions","organizations_url":"https://api.github.com/users/pvalsecc/orgs","repos_url":"https://api.github.com/users/pvalsecc/repos","events_url":"https://api.github.com/users/pvalsecc/events{/privacy}","received_events_url":"https://api.github.com/users/pvalsecc/received_events","type":"User","site_admin":false},"repo":{"id":43433507,"name":"geotools","full_name":"pvalsecc/geotools","owner":{"login":"pvalsecc","id":1180608,"avatar_url":"https://avatars.githubusercontent.com/u/1180608?v=3","gravatar_id":"","url":"https://api.github.com/users/pvalsecc","html_url":"https://github.com/pvalsecc","followers_url":"https://api.github.com/users/pvalsecc/followers","following_url":"https://api.github.com/users/pvalsecc/following{/other_user}","gists_url":"https://api.github.com/users/pvalsecc/gists{/gist_id}","starred_url":"https://api.github.com/users/pvalsecc/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/pvalsecc/subscriptions","organizations_url":"https://api.github.com/users/pvalsecc/orgs","repos_url":"https://api.github.com/users/pvalsecc/repos","events_url":"https://api.github.com/users/pvalsecc/events{/privacy}","received_events_url":"https://api.github.com/users/pvalsecc/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/pvalsecc/geotools","description":"Official GeoTools repository","fork":true,"url":"https://api.github.com/repos/pvalsecc/geotools","forks_url":"https://api.github.com/repos/pvalsecc/geotools/forks","keys_url":"https://api.github.com/repos/pvalsecc/geotools/keys{/key_id}","collaborators_url":"https://api.github.com/repos/pvalsecc/geotools/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/pvalsecc/geotools/teams","hooks_url":"https://api.github.com/repos/pvalsecc/geotools/hooks","issue_events_url":"https://api.github.com/repos/pvalsecc/geotools/issues/events{/number}","events_url":"https://api.github.com/repos/pvalsecc/geotools/events","assignees_url":"https://api.github.com/repos/pvalsecc/geotools/assignees{/user}","branches_url":"https://api.github.com/repos/pvalsecc/geotools/branches{/branch}","tags_url":"https://api.github.com/repos/pvalsecc/geotools/tags","blobs_url":"https://api.github.com/repos/pvalsecc/geotools/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/pvalsecc/geotools/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/pvalsecc/geotools/git/refs{/sha}","trees_url":"https://api.github.com/repos/pvalsecc/geotools/git/trees{/sha}","statuses_url":"https://api.github.com/repos/pvalsecc/geotools/statuses/{sha}","languages_url":"https://api.github.com/repos/pvalsecc/geotools/languages","stargazers_url":"https://api.github.com/repos/pvalsecc/geotools/stargazers","contributors_url":"https://api.github.com/repos/pvalsecc/geotools/contributors","subscribers_url":"https://api.github.com/repos/pvalsecc/geotools/subscribers","subscription_url":"https://api.github.com/repos/pvalsecc/geotools/subscription","commits_url":"https://api.github.com/repos/pvalsecc/geotools/commits{/sha}","git_commits_url":"https://api.github.com/repos/pvalsecc/geotools/git/commits{/sha}","comments_url":"https://api.github.com/repos/pvalsecc/geotools/comments{/number}","issue_comment_url":"https://api.github.com/repos/pvalsecc/geotools/issues/comments{/number}","contents_url":"https://api.github.com/repos/pvalsecc/geotools/contents/{+path}","compare_url":"https://api.github.com/repos/pvalsecc/geotools/compare/{base}...{head}","merges_url":"https://api.github.com/repos/pvalsecc/geotools/merges","archive_url":"https://api.github.com/repos/pvalsecc/geotools/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/pvalsecc/geotools/downloads","issues_url":"https://api.github.com/repos/pvalsecc/geotools/issues{/number}","pulls_url":"https://api.github.com/repos/pvalsecc/geotools/pulls{/number}","milestones_url":"https://api.github.com/repos/pvalsecc/geotools/milestones{/number}","notifications_url":"https://api.github.com/repos/pvalsecc/geotools/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/pvalsecc/geotools/labels{/name}","releases_url":"https://api.github.com/repos/pvalsecc/geotools/releases{/id}","created_at":"2015-09-30T13:09:23Z","updated_at":"2015-09-30T13:10:07Z","pushed_at":"2015-10-23T12:18:33Z","git_url":"git://github.com/pvalsecc/geotools.git","ssh_url":"git@github.com:pvalsecc/geotools.git","clone_url":"https://github.com/pvalsecc/geotools.git","svn_url":"https://github.com/pvalsecc/geotools","homepage":"http://geotools.org","size":113395,"stargazers_count":0,"watchers_count":0,"language":"Java","has_issues":false,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":0,"mirror_url":null,"open_issues_count":0,"forks":0,"open_issues":0,"watchers":0,"default_branch":"master"}},"base":{"label":"geotools:master","ref":"master","sha":"1da8bfdb53d7ea52e4d3282d23032606facb54f3","user":{"login":"geotools","id":1187031,"avatar_url":"https://avatars.githubusercontent.com/u/1187031?v=3","gravatar_id":"","url":"https://api.github.com/users/geotools","html_url":"https://github.com/geotools","followers_url":"https://api.github.com/users/geotools/followers","following_url":"https://api.github.com/users/geotools/following{/other_user}","gists_url":"https://api.github.com/users/geotools/gists{/gist_id}","starred_url":"https://api.github.com/users/geotools/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/geotools/subscriptions","organizations_url":"https://api.github.com/users/geotools/orgs","repos_url":"https://api.github.com/users/geotools/repos","events_url":"https://api.github.com/users/geotools/events{/privacy}","received_events_url":"https://api.github.com/users/geotools/received_events","type":"Organization","site_admin":false},"repo":{"id":2752522,"name":"geotools","full_name":"geotools/geotools","owner":{"login":"geotools","id":1187031,"avatar_url":"https://avatars.githubusercontent.com/u/1187031?v=3","gravatar_id":"","url":"https://api.github.com/users/geotools","html_url":"https://github.com/geotools","followers_url":"https://api.github.com/users/geotools/followers","following_url":"https://api.github.com/users/geotools/following{/other_user}","gists_url":"https://api.github.com/users/geotools/gists{/gist_id}","starred_url":"https://api.github.com/users/geotools/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/geotools/subscriptions","organizations_url":"https://api.github.com/users/geotools/orgs","repos_url":"https://api.github.com/users/geotools/repos","events_url":"https://api.github.com/users/geotools/events{/privacy}","received_events_url":"https://api.github.com/users/geotools/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/geotools/geotools","description":"Official GeoTools repository","fork":false,"url":"https://api.github.com/repos/geotools/geotools","forks_url":"https://api.github.com/repos/geotools/geotools/forks","keys_url":"https://api.github.com/repos/geotools/geotools/keys{/key_id}","collaborators_url":"https://api.github.com/repos/geotools/geotools/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/geotools/geotools/teams","hooks_url":"https://api.github.com/repos/geotools/geotools/hooks","issue_events_url":"https://api.github.com/repos/geotools/geotools/issues/events{/number}","events_url":"https://api.github.com/repos/geotools/geotools/events","assignees_url":"https://api.github.com/repos/geotools/geotools/assignees{/user}","branches_url":"https://api.github.com/repos/geotools/geotools/branches{/branch}","tags_url":"https://api.github.com/repos/geotools/geotools/tags","blobs_url":"https://api.github.com/repos/geotools/geotools/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/geotools/geotools/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/geotools/geotools/git/refs{/sha}","trees_url":"https://api.github.com/repos/geotools/geotools/git/trees{/sha}","statuses_url":"https://api.github.com/repos/geotools/geotools/statuses/{sha}","languages_url":"https://api.github.com/repos/geotools/geotools/languages","stargazers_url":"https://api.github.com/repos/geotools/geotools/stargazers","contributors_url":"https://api.github.com/repos/geotools/geotools/contributors","subscribers_url":"https://api.github.com/repos/geotools/geotools/subscribers","subscription_url":"https://api.github.com/repos/geotools/geotools/subscription","commits_url":"https://api.github.com/repos/geotools/geotools/commits{/sha}","git_commits_url":"https://api.github.com/repos/geotools/geotools/git/commits{/sha}","comments_url":"https://api.github.com/repos/geotools/geotools/comments{/number}","issue_comment_url":"https://api.github.com/repos/geotools/geotools/issues/comments{/number}","contents_url":"https://api.github.com/repos/geotools/geotools/contents/{+path}","compare_url":"https://api.github.com/repos/geotools/geotools/compare/{base}...{head}","merges_url":"https://api.github.com/repos/geotools/geotools/merges","archive_url":"https://api.github.com/repos/geotools/geotools/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/geotools/geotools/downloads","issues_url":"https://api.github.com/repos/geotools/geotools/issues{/number}","pulls_url":"https://api.github.com/repos/geotools/geotools/pulls{/number}","milestones_url":"https://api.github.com/repos/geotools/geotools/milestones{/number}","notifications_url":"https://api.github.com/repos/geotools/geotools/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/geotools/geotools/labels{/name}","releases_url":"https://api.github.com/repos/geotools/geotools/releases{/id}","created_at":"2011-11-10T23:22:12Z","updated_at":"2015-10-21T17:47:39Z","pushed_at":"2015-10-25T15:55:09Z","git_url":"git://github.com/geotools/geotools.git","ssh_url":"git@github.com:geotools/geotools.git","clone_url":"https://github.com/geotools/geotools.git","svn_url":"https://github.com/geotools/geotools","homepage":"http://geotools.org","size":259183,"stargazers_count":231,"watchers_count":231,"language":"Java","has_issues":false,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":359,"mirror_url":null,"open_issues_count":18,"forks":359,"open_issues":18,"watchers":231,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/geotools/geotools/pulls/1009"},"html":{"href":"https://github.com/geotools/geotools/pull/1009"},"issue":{"href":"https://api.github.com/repos/geotools/geotools/issues/1009"},"comments":{"href":"https://api.github.com/repos/geotools/geotools/issues/1009/comments"},"review_comments":{"href":"https://api.github.com/repos/geotools/geotools/pulls/1009/comments"},"review_comment":{"href":"https://api.github.com/repos/geotools/geotools/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/geotools/geotools/pulls/1009/commits"},"statuses":{"href":"https://api.github.com/repos/geotools/geotools/statuses/35be6b3aac4011fb3041f8ad1174bb673712ed06"}}}}