Skip optimize #895
Skip optimize #895
Conversation
|
This works for me and makes life better, implementing #27. |
There was a problem hiding this comment.
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. | |||
| 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()); |
There was a problem hiding this comment.
This printf() looks like it belongs into the if{} below.
| 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") { |
There was a problem hiding this comment.
To break down this horribly long code, move the contents of the if{} into its own private function.
| 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. |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
But this can IMHO be forced on inital parameter checks the way that "--skip-(optimiz|index)ing" is only allowed when "--unlogged" is requested as well - after all, the user should know what he is doing by requesting it.
| } | ||
|
|
||
| if (!skip_indexing) { | ||
| fprintf(stderr, "Creating geometry index on %s\n", name.c_str()); |
There was a problem hiding this comment.
This if{} too deserves its own private function.
Do you think it would make more sense to completely skip all the Postgres post-processing? (i.e. everything after pending) |
I was more thinking that the user could supply a custom script for pre- and post-processing. The default ones would do exactly the same stuff as today (optimisation and index creation) and could then be adapted as required. |
|
Closing because of unresponsiveness of submitter. |
Initially a rebase of patch by @thbeutin in #27.
Implements only skip for optimization, needs adding a skip for indexing.