Fix sline_sline_pos

[ggnmstr]

The problem fixed by this pull request is described in Issue #35

[esabol]

Always connect your pull requests to their relevant issue numbers, please. You can do this by mentioning the issue number in the title or description of the PR.

Issue #35

[esabol]

Seems like a simple enough change, but I'd like to see a test case added which fails without the PR and passes with the PR.

[ggnmstr]

Sure, I added a pair of tests to sql/poly.sql The first one definitely passes with the PR and fails without it (both polygons shouldn't be created).

[esabol]

Looks good to me! Thanks, @ggnmstr!

What do you think, @vitcpp?

[vitcpp]

Colleagues, I have some doubts about this change. I think, degenerate polygons can be accepted by most of the algorithms. This PR disables some types of polygons that may affect on user experience - if the old version constructed such degenerate polygons, the new version reports an error. Some user datasets may contain degenerate polygons. Explicit error reporting may stop data pipelines to process some datasets. Give me please some more time to think about it.

[esabol]

This PR disables some types of polygons that may affect on user experience - if the old version constructed such degenerate polygons, the new version reports an error. Some user datasets may contain degenerate polygons. Explicit error reporting may stop data pipelines to process some datasets. Give me please some more time to think about it.

If there's a use-case for supporting degenerate polygons, I'd be open to it, but it seems to me that giving correct results is what should matter. Degenerate polygons are not valid polygons, according to the definition given by the OGC Simple Features Specification. See https://cs.stackexchange.com/a/112703.

[ggnmstr]

I found out that PGSphere is kind of inconsistent in terms of degenerate polygons. It doesn't allow to create all types of degenerate polygons: Meanwhile PR I suggest removes some possibilities to create degenerate polygons (connected by 0d coordinate), but not all: So it may be some kind of problem with these 0d coordinates, maybe math, not sure now. But first, of course, we need to decide whether degenerate polygons should be allowed or not.

[esabol]

Interesting. Thank you for testing that and reporting your findings, @ggnmstr.

[vitcpp]

It seems there is a more complex issue with degenerate polygons. Thank you @ggnmstr for your findings. There is the working idea to separate two concepts - storage and algorithms. spoly is a storage for polygon data. The storage may or may not validate the input data because in most cases the storage just used to manipulate data but not to validate it. My position is to allow to keep both valid and invalid data in storage. Data import algorithms just load data into storage objects as is. Some functions to validate and normalize data should be implemented as well. The algorithms assume that the input storage objects contain valid data. Such check is not trivial for polygons. It is the developer responsibility to pass valid data to algorithms. Otherwise, the behaviour is undefined. For most of the algorithms degenerate polygons are ok but it is important to avoid edge crosses that may lead to the wrong results. Overlapping edges may be used to construct polygons with holes (if needed).

Now we have some inconsistent behaviour with spoly. Once we can't create degenerate spoly with 4 points and it is the adopted behaviour I think I'm ok to disable creation of all types of degenerate polygons. Later, if we decide to allow degenerate polygons we can change this behaviour.

Lets wait for PR #22 and then merge it.

[vitcpp]

@ggnmstr Could you please rebase the branch to the newer version and start the commit description from the capital letter? We will merge it if tests are passed.

[ggnmstr]

@vitcpp Done

[ggnmstr]

Done. @esabol, @vitcpp, thank you for review.