Opened 12 years ago

Closed 12 years ago

Last modified 12 years ago

#1345 closed defect (fixed)

ST_StartPoint, ST_EndPoint regress failure

Reported by: robe Owned by: pramsey
Priority: critical Milestone: PostGIS 2.0.0
Component: postgis Version: master
Keywords: Cc:

Description

I just noticed (by accident of course when testing my cross street logic) and I forgot my data is multilinestrings (though most are just single linestrings inside a multi wrapper).

This works on my 1.5 install:

SELECT ST_AsText(ST_StartPoint('MULTILINESTRING((1 2, 3 4))'::geometry) );

--returns --
POINT(1 2)

This works on my 2.0 install:

SELECT ST_AsText(ST_StartPoint('MULTILINESTRING((1 2, 3 4))'::geometry) );

--returns --
NULL

If this is an intentional change, it needs to be documented since it stands to break a lot of applications.

Change History (11)

comment:1 by strk, 12 years ago

r48 contains instructive commit log about this

comment:2 by strk, 12 years ago

I see the behavior or returning NULL as consistent and documented. Haven't checked the documentation of 1.5 to see if the not-null return was documented as expected. Robe: did you ?

comment:3 by robe, 12 years ago

Nope not for that function, but why would I when it was by pure accident that I realized my code was happily working with some undocumented feature. I suspect it may have been introduced when we made this change possibly?

http://www.postgis.org/documentation/manual-1.5/ST_Line_Locate_Point.html

though that doesn't work even on my 1.5 install and I think we have a bug ticket on that somewhere.

SELECT ST_Line_Locate_Point('MULTILINESTRING((1 2, 3 4))'::geometry, ST_Point(1,2));

I hope that wasn't my imagination that we made this change. I assume I availabilities aren't a figment of my imagination. But I could be wrong. Paul you ever remember making this change? I suspect if we made this change — and it could be purely in my imagination as well as the availability comment, it may have been made for the special one line string multilinestring cases that people have because they brought there data in as multilinestring because they happened to have 1 or 2 multis out of a swarm of linestrings.

comment:4 by robe, 12 years ago

While I found a couple of tickets and it might have been my misunderstanding when Paul made ST_AddMeasure work with multilinestrings.

At r5420 I made the availability note about Substring. So if its a figment of my imagination, probably happened in 1.5.1

r5168 - Paul's commit for ST_AddMeasure which change some linear referencing stuff

#1242 - I complaining about how I must have been living in some parallel time warp and how the hell did I get this idea

But of course none of this explains why ST_StartPoint/EndPoint works on my 1.5 installs. Does it work on yours strk or you don't have 1.5 installed?

For the availability note, the only reason I can think of aside from pure imagination is that my monkey tests would test for that and perhaps when I looked at the results I said "hmm that's interesting this works for multilinestrings". I did hat a lot for the 3D functions since there wasn't an easy/fast way to tell if they did or didn't.

comment:5 by robe, 12 years ago

It might be this change: r6264

I'll try to compile the version before to see if that used to work. Maybe it was an accidental change and that the old behavior was smart enough to work with a single line string wrapped in a multi wrapper and peel it out.

comment:7 by robe, 12 years ago

Resolution: fixed
Status: newclosed

Since apparently I'm the only one who noticed or cares about this regression failure, I have documented the issue in r8484 as an expected side effect and closing this out.

comment:8 by robe, 12 years ago

pramsey finally notices something is different in #1964

comment:9 by pramsey, 12 years ago

Judging by the differences in code from 1.5 to 2.0 the change was a side effect of the serialization change, since the original code worked against "inspected geometries", that is, direct access to the serialization. The original code was also pretty clearly deliberately unwrapping the first instance of the targeted type within any multi* geometries it was handed. So if it was handed a geometry collection or multilinestring, it would unwrap and then use the *first* instance it found to answer the call. Whether the benefit of auto-unwrapping of things like single-entry linestrings outweighs the surprise of the end point of a double entry multlinestring being the end of the first entry, I leave as an exercise to the user.

I noticed the problem working on my tutorial, which includes some calls to NumInteriorRings that previously transparently unwrapped a multipolygon to the first entry.

comment:10 by robe, 12 years ago

The ST_NumInteriorRings one might be something we should fix (or at least think about a bit more), as that is a well documented behavior which arguably is a sometimes useful but stupid behavior as it could trap people into thinking multipolygons are properly handled unless they scrutinized the docs :)

http://www.postgis.org/documentation/manual-svn/ST_NumInteriorRings.html

My ST_StartPoint/ST_EndPoint was an undocumented behavior so people relying on that behavior were probably doing so out of share ignorance :). It just so happens that tiger edges I always brought them in as multilinestrings because even though they were mostly single linestrings, there are a few multilinestrings in there.

comment:11 by robe, 12 years ago

Hmm looking at SQL Server, IBM docs, and ESRI docs, I'm tempted to say that even though this is a breaking change we should document it as such and perhaps even go so far as to throw an error if handed a multipolygon. My dirty self says make it the way it was, but the purist Leo coughing down my throat says short-lived pain must be endured for long-lived purity.

http://msdn.microsoft.com/en-us/library/bb933845%28v=sql.100%29.aspx

http://publib.boulder.ibm.com/infocenter/db2luw/v8/index.jsp?topic=/com.ibm.db2.udb.doc/opt/rsbp4098.htm

http://webhelp.esri.com/arcgisserver/9.3/java/index.htm#geodatabases/st_numinteriorring.htm

It seems we are the only ones who think this is okay to use with multipolygons,

Note: See TracTickets for help on using tickets.