Skip to content

Optionally wrap polygon geometries in multipolygons#1327

Merged
lonvia merged 4 commits intoosm2pgsql-dev:masterfrom
joto:geom-transform-multi
Nov 24, 2020
Merged

Optionally wrap polygon geometries in multipolygons#1327
lonvia merged 4 commits intoosm2pgsql-dev:masterfrom
joto:geom-transform-multi

Conversation

@joto
Copy link
Copy Markdown
Collaborator

@joto joto commented Nov 18, 2020

When you have an area table in the flex output, you always had to use the "geometry" column type for the geometry column, because ways are turned into polygons and multipolygon relations are turned into polygons or multipolygons. The common type for these is the generic "geometry".

With this change, declaring a geometry column as type "multipolygon" now works. All polygon geometries are automatically wrapped in a multipolygon before adding them to the database. Depending on what you are doing this might be want you want or not. It will make the geometries larger, but it also makes the data more consistent and so easier to work with and some software can only handle this consistent data.

See #1316

Copy link
Copy Markdown
Collaborator

@lonvia lonvia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. Can we have nice error messages now instead of COPY failing on bad geometry types?

Comment thread src/geom-transform.cpp Outdated
create_multigeoms = geom::create_multi_t::always;
} else if (m_multi) {
create_multigeoms = geom::create_multi_t::optional;
}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I read this right, then the multi flag is ignored, when the type is MULTIPOLYGON. Makes perfect sense but it might be worth documenting (and possibly testing, to document that this was the intended behaviour ;).

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After some more thought, this wasn't the right choice. The multi setting should be independent of the target geometry type. The new code implements this that way.

@joto joto force-pushed the geom-transform-multi branch from 67a1ae0 to 9dc6421 Compare November 19, 2020 16:43
@joto
Copy link
Copy Markdown
Collaborator Author

joto commented Nov 19, 2020

I have restructured the code to better match intentions and added tests. Also added a new commit that switches the default for the multi setting from false to true as outlined in #1316. The config option is now called split_at to better match the somewhat similar option for lines, although the only allowed value is multi.

@joto
Copy link
Copy Markdown
Collaborator Author

joto commented Nov 19, 2020

Looks good to me. Can we have nice error messages now instead of COPY failing on bad geometry types?

Unfortunately not. The code creates WKB geometries and you can't tell later what type of geometry they are (without hacks), so we can't do the check. We'll rework the geometry code anyway in the near future using actual geometry types and then we can do this easily.

joto added 4 commits November 23, 2020 10:20
When you have an area table in the flex output, you always had to use
the "geometry" column type for the geometry column, because ways are
turned into polygons and multipolygon relations are turned into polygons
or multipolygons. The common type for these is the generic "geometry".

With this change, declaring a geometry column as type "multipolygon" now
works. All polygon geometries are automatically wrapped in a
multipolygon before adding them to the database. Depending on what you
are doing this might be want you want or not. It will make the
geometries larger, but it also makes the data more consistent and so
easier to work with and some software can only handle this consistent
data.

See osm2pgsql-dev#1316
Also changes the name of the setting in the flex_config from
multi=true/false to split_at=nil/'multi'.
@joto joto force-pushed the geom-transform-multi branch from 9dc6421 to 819d611 Compare November 23, 2020 09:21
@lonvia lonvia merged commit dd47ab6 into osm2pgsql-dev:master Nov 24, 2020
@joto joto deleted the geom-transform-multi branch November 24, 2020 15:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants