[Tile-serving] [openstreetmap/osm2pgsql] Skip optimize (#895)

[Sarah Hoffmann]

lonvia requested changes on this pull request.

I admit that I have some reservations about adding yet more options for relatively limited use-cases. For one this is because it is getting harder and harder to judge if the combination of parameters makes sense and doesn't have any unintended side-effects. The other reason is that there are a couple of other issues open that address something similar (e.g. #799) and I would rather see the pre- and postprocessing of the tables reconsidered as a whole. But I don't expect that to happen any time soon, so I guess we have to be realistic here.

> @@ -211,96 +224,143 @@ void table_t::start()
 void table_t::stop()
 {
     stop_copy();
-    if (!append)
-    {
+    // Post-procrssing for initial import only.

typo

>          time_t start, end;
         time(&start);
 
-        fprintf(stderr, "Sorting data and creating indexes for %s\n", name.c_str());
-
-        if (srid == "4326") {
-            /* libosmium assures validity of geometries in 4326, so the WHERE can be skipped.
-               Because we know the geom is already in 4326, no reprojection is needed for GeoHashing */
+        fprintf(stderr, "Sorting data in %s\n", name.c_str());

This printf() looks like it belongs into the if{} below.

>          time_t start, end;
         time(&start);
 
-        fprintf(stderr, "Sorting data and creating indexes for %s\n", name.c_str());
-
-        if (srid == "4326") {
-            /* libosmium assures validity of geometries in 4326, so the WHERE can be skipped.
-               Because we know the geom is already in 4326, no reprojection is needed for GeoHashing */
+        fprintf(stderr, "Sorting data in %s\n", name.c_str());
+        if (!skip_optimizing) {
+            if (srid == "4326") {

To break down this horribly long code, move the contents of the if{} into its own private function.

> -        pgsql_exec_simple(sql_conn, PGRES_COMMAND_OK, (fmt("ALTER TABLE %1%_tmp RENAME TO %1%") % name).str());
-        fprintf(stderr, "Copying %s to cluster by geometry finished\n", name.c_str());
-        fprintf(stderr, "Creating geometry index on %s\n", name.c_str());
-
-        // Use fillfactor 100 for un-updatable imports
-        pgsql_exec_simple(sql_conn, PGRES_COMMAND_OK, (fmt("CREATE INDEX ON %1% USING GIST (way) %2% %3%") % name %
-            (slim && !drop_temp ? "" : "WITH (FILLFACTOR=100)") %
-            (table_space_index ? "TABLESPACE " + table_space_index.get() : "")).str());
-
-        /* slim mode needs this to be able to apply diffs */
-        if (slim && !drop_temp)
-        {
-            fprintf(stderr, "Creating osm_id index on %s\n", name.c_str());
-            pgsql_exec_simple(sql_conn, PGRES_COMMAND_OK, (fmt("CREATE INDEX ON %1% USING BTREE (osm_id) %2%") % name %
-                (table_space_index ? "TABLESPACE " + table_space_index.get() : "")).str());
+            // We haven't removed invalid geometries as we did not perform a COPY.

The table has originally been created UNLOGGED because we know that it will be dropped here. If the sorting step is skipped, then the table needs to be a standard one.

>  
+            pgsql_exec_simple(sql_conn, PGRES_COMMAND_OK,
+                              (fmt("CREATE TRIGGER %1%_osm2pgsql_valid BEFORE "
+                                   "INSERT OR UPDATE\n"
+                                   "  ON %1%\n"
+                                   "    FOR EACH ROW EXECUTE PROCEDURE "
+                                   "%1%_osm2pgsql_valid();") %
+                               name)
+                                  .str());
+        }
+
+        if (!skip_indexing) {
+            fprintf(stderr, "Creating geometry index on %s\n", name.c_str());

This if{} too deserves its own private function.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/osm2pgsql/pull/895#pullrequestreview-192898933
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstreetmap.org/pipermail/tile-serving/attachments/20190115/6a350ad3/attachment.html>