osm2pgsql-dev / osm2pgsql

OpenStreetMap data to PostgreSQL converter
https://osm2pgsql.org
GNU General Public License v2.0
1.51k stars 474 forks source link

as_linestring() and likely other geometry transform functions can only be called from the process_way() function, not custom functions. #2171

Closed mboeringa closed 7 months ago

mboeringa commented 7 months ago

What version of osm2pgsql are you using?

osm2pgsql version 1.11.0 (1.8.1-397-gcf66989f)

What operating system and PostgreSQL/PostGIS version are you using?

Ubuntu 22.04 PostgreSQL 16.2 (Ubuntu 16.2-1.pgdg22.04+1), PostGIS 3.4

Tell us something about your system

What did you do exactly?

I have been trying to convert an existing style that still used the now deprecated 'add_row()' function to use the new 'insert()' function (the style is a derivative of Paul Norman's work for the flex style of OpenStreetMap carto: https://github.com/gravitystorm/openstreetmap-carto/pull/4431 and more specifically https://github.com/gravitystorm/openstreetmap-carto/blob/c7066c8a0b670fbd19532241f912c13f55eef215/openstreetmap-carto.lua).

Although I think I have the syntax right now, I run into an issue with a check that osm2pgsql seems to do, I get an error message:

ERROR: Failed to execute Lua function 'osm2pgsql.process_relation': osm2pgsql/flex-config/openstreetmap-carto.lua:1228: Error in 'as_linestring': The function as_linestring() can only be called from the process_way() function.

It is true that Paul's work for that style uses a couple of helper functions which implement the actual insertion 'add_row/insert', and that the 'as_linestring()' function call thus is not inside the 'process_way' function.

While I could theoretically rewrite everything to be inside the 'process_way' function, this not only would bloat the function, but this limitation also hampers general Lua development.

It also kind of breaks "backward compatibility" with the deprecated 'add_row' function, as using that function in a helper function outside 'process_way' was never a problem and simply worked.

Could this limitation with 'insert' and required 'as_linestring/multipolygon' etc. be lifted and allow more flexible function and code design?

What did you expect to happen?

Geometry transforms for the new 'insert' work outside the 'process_way' function.

What did happen instead?

What did you do to try analyzing the problem?

joto commented 7 months ago

What do you propose as_linestring() generate when called on a relation? What would happen if the relation doesn't "fit" into a LineString?

Typically you'd use something like this on a route relation where the result could be a LineString, but it could also be a MultiLineString. That's why there is only the as_multilinestring() function for relations. That function also exists for ways, although in that case it will always create a MultiLineString with a single LineString in it. You can then either use that MultiLineString as is or iterate over the geometries in it and work with each LineString. So common code for both ways and relations can just use the as_multilinestring() function.

See https://osm2pgsql.org/doc/tutorials/switching-from-add-row-to-insert/ for some more infos.

You can also look at the unitable.lua example config to see how you can write code that's mostly shared between nodes, ways, and relations but still handles geometries differently.

And finally: Functions are first-class objects in Lua, you can define your own. For an example see the themepark code.

mboeringa commented 7 months ago

What do you propose as_linestring() generate when called on a relation? What would happen if the relation doesn't "fit" into a LineString?

@joto

I am not suggesting allowing 'as_linestring' in any other processing callback function like the 'process_relation' callback function, instead, I suggest to allow 'process_way' to call helper functions to do a geometry transform, instead of forcing the geometry transform function to be used inside the 'process_way' function itself. This is what Paul's style did, and feels like a natural way of using functions in Lua.

I know you can define functions, in fact, it is exactly the fact that I want to define and use custom functions that is now limited because osm2pgsql performs this check.

If the goal of this check was to prevent users from making mistakes in the Lua code related to the processing callback functions and geometry transforms, it would be better to change the check's logic from:

_"'as_linestring' cannot be used in any other function than 'process_way'"_

to:

_"as_linestring" is forbidden to be used in the 'process_relation' or 'select_relationmembers' callback functions

thus allowing its usage in any other function except those two.

Similarly, as to your example of the 'as_multilinestring', this could be limited by:

_"'as_multilinestring' is forbidden to be used in the 'processway' callback function"

allowing it to be used in any other helper function defined by the user.

mboeringa commented 7 months ago

@joto

I have had a closer look at my style and Paul's work for it, and I really have to come to the conclusion that unless the logic for these checks is changed, I will be unable to upgrade to osm2pgsql v2.0 once it comes out. This is an absolute blocking issue for me.

There is just no way to convert 'add_row' to 'insert' with current logic and the processing needed in my style.

This is especially related to OSM boundary relations, where there is the direct need to insert relation members both in the 'planet_osm_line' and 'planet_osm_polygon' tables in the 'process_relation' callback function. This is related to Paul's nice work to allow de-duplicated OSM boundary relation line members to be used to solve the issue of overlapping boundary relation symbology as based on polygons.

AFAI can tell from the code review now, this part of the code cannot be converted to the new 'insert' function logic with current checks in osm2pgsql, it will fail on the error thrown. This was never a problem with the 'add_row' function.

joto commented 7 months ago

Ah, I think there is a misunderstanding here with the wording of the error message. You can call the as_* geometry creation functions from any function you want as long as that function is ultimately called from the correct osm2pgsql.process_node/way/relation() function. The issue isn't really about which function it is called from, but that the OSM object you call it on is of the correct type. I'll think about how to improve that error message.

I am pretty sure everything that could be done with add_row() can be done with insert(). If you can not get something to work, post a minimal working example here and we can have a look at it.

lonvia commented 7 months ago

I don't think there is any bug here, just a misunderstanding how the geometry conversion functions can/should be used. Converting to discussion.