New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add jdbc import #556

Merged

abrensch merged 12 commits into abrensch:master from afischerdev:jdbc-import

May 28, 2023

Collaborator

afischerdev commented May 20, 2023

This is according to discussion #486 and adds tags via database call to the way generation.
New is
forest to find nice ways in or near forest
river to file ways along water
noise to avoid bigger streets
town to avoid bigger citys
traffic was used before but is now generated by database call

The classpath in generation needs a jdbc driver
and the OsmFastCutter a call with some database parameter
Please see readme.

There is also a new lookups.dat that contains the new tags - before the node context. This should be neutral to older versions.
In the next generation lookups.dat (#458) I would prefer these new tags more moved up to get smaller rd5 files.

This needs also an update for the profiles to make use of the new tags.

afischerdev added 5 commits

May 20, 2023 18:12


          add jdbc call

e362e5c


          update test with sdbc call null

7750ba9


          modify hgt call in bef generation

3ea4090


          add scripts and readme for db use

1142c28


          set new lookups version

511ac27

afischerdev temporarily deployed to BRouter

May 20, 2023 16:38

— with

GitHub Actions Inactive

Owner

abrensch commented May 22, 2023

I tried to run this branch on the "mapcreator" account of new dev server, but I'm stuck with a very strange effect: Call to OsmFastCutter is returning immediatly, but the process is running on, so script is continuing with the second step (PosUnifier) but that fails...

Probably has noting to to with the jdbc option, more with the new PBF-Parser?

afischerdev mentioned this pull request

Merge OSM parsers #468

Closed

Collaborator Author

afischerdev commented May 22, 2023

@abrensch
I rechecked the jdbc handling.
wrong driver
wrong database
wrong user
wrong password
On all errors the process is killed by System.exit
And next process stops with 'bordernids.dat not found'.

So sounds as this is jdbc problem.
I could change this: remove System.exit with a smoother handling

Owner

abrensch commented May 22, 2023

So sounds as this is jdbc problem. I could change this: remove System.exit with a smoother handling

A non-zero exit code would be smooth enough :-)

However, I still do not understand why the process keeps running. Looks like a second thread forked or the like (?)

Collaborator Author

afischerdev commented May 22, 2023

@abrensch
Ok I will change to exi(1).
On my run next process stops with 'bordernids.dat not found'. May be no cleaning first?

I was preparing for smoother handling, but not sure if it's a good idea to continue a process when no database call is in progress. The (incorrect) results will therefore only be displayed to you once the entire process has been completed.


          set exit(1) on jdbc error

3602e42

afischerdev temporarily deployed to BRouter

May 22, 2023 09:32

— with

GitHub Actions Inactive

Collaborator Author

afischerdev commented May 22, 2023

@abrensch @mjaschen
Update is done. Zip file contains new lib

zod mentioned this pull request

Use elevation data in tests #557

Merged

zod requested changes

View reviewed changes

Collaborator

zod left a comment

In addition to the specific comments I have some other remarks

.sql and .lua don't use a consistent format, perhaps run some formatter/beautifier
documentation in mapcreation folder: Should we keep it there or move it to docs?
absolutly no tests: It requires quite some setup to import OSM data into postgis and process it. For tests we should perhaps use some (in-memory) SQLite database which has an all_tags table.
Is it possible to update the database using osm2pgsql?
Should the database use more (materialized) views instead of tables because we don't need to modify the data?

brouter-map-creator/src/main/java/btools/mapcreator/OsmCutter.java Outdated

@@ @@ -165,6 +192,93 @@ private void generatePseudoTags(Map<String, String> map) { @@
                 }
+              // Ess Bee : NEW function to add the new tags (estimated_noise_class , river, forest...)
+                private void generateSpecialTags(long osm_id, Map<String, String> map) {

Collaborator

zod May 22, 2023

Special doesn't tell much about the tags. Perhaps generateTagsFromDatabase?

brouter-map-creator/src/main/java/btools/mapcreator/OsmCutter.java Outdated

Comment on lines 195 to 196

		// Ess Bee : NEW function to add the new tags (estimated_noise_class , river, forest...)

Collaborator

zod May 22, 2023

Remove coment

brouter-map-creator/src/main/java/btools/mapcreator/OsmCutter.java Outdated


		String sql_all_tags = "SELECT * from all_tags where losmid = ?";

		System.err.println("OsmCutter start connection to the database........" + jdbcurl);

Collaborator

zod May 22, 2023

That's not an error, should not print to standard error.

brouter-map-creator/src/main/java/btools/mapcreator/OsmCutter.java Outdated

+                      conn = DriverManager.getConnection(jdbcurl);
+                      psAllTags = conn.prepareStatement(sql_all_tags);
+                      System.err.println("OsmCutter connect to the database ok........");

Collaborator

zod May 22, 2023

That's not an error, should not print to standard error.

brouter-map-creator/src/main/java/btools/mapcreator/OsmCutter.java Outdated

Comment on lines 239 to 259

+                          if (rsBrouter.getString("noise_class") != null) {
+                            map.put("estimated_noise_class", rsBrouter.getString("noise_class"));
+                            cntNewNoise = cntNewNoise + 1;
+                          }
+                          if (rsBrouter.getString("river_class") != null) {
+                            map.put("estimated_river_class", rsBrouter.getString("river_class"));
+                            cntNewRiver = cntNewRiver + 1;
+                          }
+                          if (rsBrouter.getString("forest_class") != null) {
+                            map.put("estimated_forest_class", rsBrouter.getString("forest_class"));
+                            cntNewForest = cntNewForest + 1;
+                          }
+                          if (rsBrouter.getString("town_class") != null) {
+                            map.put("estimated_town_class", rsBrouter.getString("town_class"));
+                            cntNewTown = cntNewTown + 1;
+                          }
+                          if (rsBrouter.getString("traffic_class") != null) {
+                            map.put("estimated_traffic_class", rsBrouter.getString("traffic_class"));
+                            cntNewTraffic = cntNewTraffic + 1;

Collaborator

zod May 22, 2023

Lots of copy-pase code. We should store source tags and BRouter tags in a map and just lookup the mapping. Same would work for those counters.

brouter-map-creator/src/main/java/btools/mapcreator/OsmCutter.java Outdated


		System.err.println("OsmCutter start connection to the database........" + jdbcurl);

		try {

Collaborator

zod May 22, 2023

This try-block shoudn't be nested in the if (conn == null) block

brouter-map-creator/src/main/java/btools/mapcreator/OsmFastCutter.java Outdated

-                  if (args.length != 11 && args.length != 12) {
-                    String common = "java OsmFastCutter <lookup-file> <node-dir> <way-dir> <node55-dir> <way55-dir> <border-file> <out-rel-file> <out-res-file> <filter-profile> <report-profile> <check-profile>";
+                  if (args.length != 11 && args.length != 12 && args.length != 13) {
+                    String common = "java OsmFastCutter <lookup-file> <node-dir> <way-dir> <node55-dir> <way55-dir> <border-file> <out-rel-file> <out-res-file> <filter-profile> <report-profile> <check-profile> <map-file> <jdbc-url>";

Collaborator

zod May 22, 2023

Optional parameters are usually in square brackets e.g. [jbdc-url]

misc/scripts/mapcreation/brouter.sql Outdated

		@@ -0,0 +1,649 @@
		-- calculation of new tags (estimated_noise_class, estimated_river_class,estimated_forest_class, estimated_town_class, estimated_traffic_class)
		-- Ess Bee version 08/05/2023

Collaborator

zod May 22, 2023

We should use git for versioning, not an additional number

afischerdev added 5 commits

May 23, 2023 11:54


          remove comment, set standard out

7ce31e3


          rename function

4fbe368


          mark optional parameter

7e57824


          reformat lua

72195d3


          rework database routine

f702100

Collaborator Author

afischerdev commented May 24, 2023

.sql and .lua don't use a consistent format, perhaps run some formatter/beautifier

Not easy to find a formatter, most breaking on postgres functions. I used https://sqlformat.darold.net/ online formatting for now, but also this breaks with errors on database generation. That needs more time.

documentation in mapcreation folder: Should we keep it there or move it to docs?

Yes, we should. But at the moment this is more a todo list for finding a right script for generation.
At the end we have a new db_gen.sh and new rd5_gen.sh. May be an db_update.sh
All this is not ready.

absolutly no tests: It requires quite some setup to import OSM data into postgis and process it. For tests we should perhaps use some (in-memory) SQLite database which has an all_tags table.

Yes, and I don't have an idea for a change. The scripts need postgres functions. So a SQLite could only hold pregenerated values. But that is a fake test.

Is it possible to update the database using osm2pgsql?

I hope so. But it is not developed or tested.

Should the database use more (materialized) views instead of tables because we don't need to modify the data?

This is not tested as well. But yes, it is an idea for the future.

afischerdev temporarily deployed to BRouter

May 24, 2023 10:43

— with

GitHub Actions Inactive

afischerdev requested a review from zod

May 24, 2023 10:50


          reformat sql by hand

6158c44

afischerdev temporarily deployed to BRouter

May 24, 2023 12:13

— with

GitHub Actions Inactive

Owner

abrensch commented May 26, 2023

I don't get it....
OsmFastCutter keeps putting itself in the background as soon as there is a 13. argument looking slighty like a jdbc-url. Not for "xyz", but for "jdbc:oracle:..." and even without the postgres driver in the classpath
And from the code I would expect that the JDBC-Logic is only initialized when the first Osm-Way is decoded (which would be after half an hour decoding nodes), but it happens immediaty...

Collaborator Author

afischerdev commented May 26, 2023 •

edited

Loading

I would expect that the JDBC-Logic is only initialized when the first Osm-Way ...

That is the way it works. Please see this line in nextWay routine. I was able to check it in my logs.
OsmFastCutter only forwards the jdbc argument to OsmCutter (please see here).

But the mapping to "jdbc:oracle:..." sounds strange.

Owner

abrensch commented May 26, 2023

OsmFastCutter keeps putting itself in the background as soon as there is a 13. argument looking slighty like a jdbc-url

Blame on me... the "&" in the jdbc-url is interpreted by the shell... just needs quotes.

abrensch merged commit 624edc6 into abrensch:master

Collaborator

zod commented Jun 2, 2023

Yes, and I don't have an idea for a change. The scripts need postgres functions. So a SQLite could only hold pregenerated values. But that is a fake test.

It's pretty common to use mocks/fakes/doubles to test functionality on a unit or module level. This also establishes a clear boundary because you can easily see which data is expected by generateTagsFromDatabase.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet