search

pgRouting / pgrouting

Repository contains pgRouting library. Development branch is "develop", stable branch is "master"
https://pgrouting.org
GNU General Public License v2.0
1.15k stars 366 forks source link

pgr_pointtoedgenode #315

Closed dkastl closed 7 years ago

dkastl commented 9 years ago

Referring to #309 we should rename pgr_pointtoedgenode to _pgr_pointtoedgenode and provide a wrapper (returning a notice) to ensure backwards compatibility.

cvvergara commented 9 years ago

@woodbri @robe2 prepared Jenkins to have some tests on develop_2_1_0 with postgis 2.2 which is going to be released soon. This warning show up in the tests of this function. 

 'src/common/test/pgrouting_conversion_tools-any-01.test.sql' => 'FAILED: 1a2
> WARNING:  ST_Line_Locate_Point signature was deprecated in 2.1.0. Please use ST_LineLocatePoint
2a4
> WARNING:  ST_Line_Locate_Point signature was deprecated in 2.1.0. Please use ST_LineLocatePoint
24a27
> WARNING:  ST_Line_Locate_Point signature was deprecated in 2.1.0. Please use ST_LineLocatePoint',
                                              'src/common/test/pgrouting_dmatrix_tools-any-01.test.sql' => 'FAILED: 3a4
> WARNING:  ST_Line_Locate_Point signature was deprecated in 2.1.0. Please use ST_LineLocatePoint',

As the function is considered proposed, I made this "quick fix" (which only hides the warning message)

        execute 'show client_min_messages' into debuglevel;
        SET client_min_messages='ERROR';
        pct := st_line_locate_point(rr.the_geom, pnt);
        execute 'set client_min_messages  to '|| debuglevel;
robe2 commented 9 years ago

PRobably better rather than embedding the call in the function to do this:

ALTER FUNCTION pgr_pointtoedgenode(text, geometry, double precision)
  SET client_min_messages='ERROR';

That will automatically revert it back to what it was before the function is called and only have the setting for duration of call. Function variables have existed since I think PostgreSQL 8.3 so safe to use for 2.1, since 8.3 is no longer a supported PostgreSQL version.

robe2 commented 9 years ago

I would suggest this function takes an optional argument -- geometry column name and have it default to 'the_geom'. I was unable to use this function in chapter on load in book because while osm2pgrouting does output the_geom, osm2po outputs geom_way.

So I settled on this syntax:

SELECT id FROM ospr.planet_ways_vertices_pgr ORDER BY the_geom <-> 
        ST_SetSRID(ST_Point(-77.0364906,38.8975428), 4326) limit 1 ;

Which by struck of luck returned the same answer. Though using distance by edge is better.

It would be nice for this function to determine if the user is using PostgreSQL 9.5 + PostGIS 2.2.

With PostGIS 2.2 + PostgreSQL 9.5, the distance operator because a true distance function, so would be faster than what this is currently doing, and wouldn't require tolerance guessing.

cvvergara commented 9 years ago

Related to this osm2pgrouting issue: https://github.com/pgRouting/osm2pgrouting/issues/111

robe2 commented 9 years ago

I'm not sure if any thing can be done about this, probably a discussion for 2.2 or 3.0, but one other issue with pgr_pointToEdge is that it returns an integer, which is fine for normal size sets of data, but will fail for larger sets. So while the new pgr_dijikstra functions support bigint, you have a disconnect with this function not supporting bigint..

cvvergara commented 8 years ago

@woodbri Q. was some progress was done to?:

cvvergara commented 8 years ago

without the wrapper fix it fails with postgis 2.2

woodbri commented 8 years ago

No, I will not have time to address these issues before the release.

This function is basically doing the preprocessing that withPoints family has to do, maybe it would be better to just expose that preprocessing instead of using this function?

cvvergara commented 7 years ago

There is being a year since we last talked about this issue. I am justifying the deprecation of this function.

The documentation says that:

The edges table must have the following columns: source target the_geom

The function forces the users to have those columns on the table, when the other pgRouting functions do not force the users to have those columns on the table.

This function does not do what the "preprocessing" of the withPoints functions. with points functions modify the graph, such that the "point of interest(s)" ([pid,] edge_id, fraction [, side]) given become vertices on the graph. Any way I'll expose the code, you can see it here: https://github.com/pgRouting/pgrouting/blob/54abb6aadfb7941b2a1d40342b807ea4f54fc582/src/withPoints/src/pgr_withPoints.cpp#L216 Forgot to mention that task is done in C++.

I made some tests using the sample data, that I will just post here:

test 1 (PASS) using the original sampledata

Testing things "work" as is on the sample data. I test that a pgr_dijkstra can be executed, and then "try" that the pgr_pointToEdgeNode.

pgr_dijkstra

SELECT pgr_dijkstra('SELECT id, source, target, cost, reverse_cost FROM edge_table', 2, 3);
  pgr_dijkstra  
----------------
 (1,1,2,4,1,0)
 (2,2,5,8,1,1)
 (3,3,6,9,1,2)
 (4,4,9,16,1,3)
 (5,5,4,3,1,4)
 (6,6,3,-1,0,5)
(6 rows)

pgr_pointToEdgeNode

SELECT pgr_pointToEdgeNode('edge_table', the_geom, 0.5) FROM pointsofinterest;
 pgr_pointtoedgenode 
---------------------
                   1
                   9
                  11
                   7
                   6
                   5
(6 rows)

test 2 (FAIL) different column name

I will rename the column source (English word) to desde (Spanish word). Like if in a non English speaking country where maybe the non english speaking company's standards require to have the columns in Spanish.

ALTER TABLE edge_table RENAME source TO desde;

pgr_dijkstra

SELECT pgr_dijkstra('SELECT id, desde AS source, target, cost, reverse_cost FROM edge_table', 2, 3);
  pgr_dijkstra  
----------------
 (1,1,2,4,1,0)
 (2,2,5,8,1,1)
 (3,3,6,9,1,2)
 (4,4,9,16,1,3)
 (5,5,4,3,1,4)
 (6,6,3,-1,0,5)
(6 rows)

pgr_pointToEdgeNode

 select pgr_pointToEdgeNode('edge_table', the_geom, 0.5) from pointsofinterest;
ERROR:  record "rr" has no field "source"
CONTEXT:  PL/pgSQL function pgr_pointtoedgenode(text,geometry,double precision) line 37 at RETURN

So, rr (which as a user I am not using) does not have source, there is no option to indicate to the function that source is the column named desde (like in pgr_dijkstra). The same thing happens with columns target, and the_geom as @robe2 pointed out on https://github.com/pgRouting/pgrouting/issues/315#issuecomment-129192727. The pgRouting functions do not require that the names of the columns be fixed. (the topology functions dont require that the column names to be fixed)

test 3 (FAIL) bigint values are lost

UPDATE edge_table SET source = source * 1000000000;
UPDATE edge_table SET target = target * 1000000000;

pgr_dijkstra

SELECT pgr_dijkstra('SELECT id, source, target, cost, reverse_cost
FROM edge_table', 2000000000, 3000000000);
      pgr_dijkstra       
-------------------------
 (1,1,2000000000,4,1,0)
 (2,2,5000000000,8,1,1)
 (3,3,6000000000,9,1,2)
 (4,4,9000000000,16,1,3)
 (5,5,4000000000,3,1,4)
 (6,6,3000000000,-1,0,5)
(6 rows)

pgr_pointToEdgeNode

SELECT pgr_pointToEdgeNode('edge_table', the_geom, 0.5) FROM pointsofinterest;
ERROR:  value "9000000000" is out of range for type integer
CONTEXT:  PL/pgSQL function pgr_pointtoedgenode(text,geometry,double precision) while casting return value to function's return type

Yet again no problem with pgr_dijkstra, and like @robe2 pointed out on https://github.com/pgRouting/pgrouting/issues/315#issuecomment-129243590 fails when BIGINT values are involved.

test 4 (FAIL) The geometry column exists but is in other column.

Suppose that an application has the geometry of the edge is in column geom and the geometry of the source point is in column the_geom.

ALTER TABLE edge_table ADD COLUMN geom GEOMETRY;
UPDATE edge_table SET geom = the_geom;
UPDATE edge_table SET the_geom = edge_table_vertices_pgr.the_geom
FROM edge_table_vertices_pgr WHERE source = edge_table_vertices_pgr.id;

pgr_dijkstra

SELECT pgr_dijkstra('SELECT id, source, target, cost, reverse_cost FROM edge_table', 2, 3);
  pgr_dijkstra  
----------------
 (1,1,2,4,1,0)
 (2,2,5,8,1,1)
 (3,3,6,9,1,2)
 (4,4,9,16,1,3)
 (5,5,4,3,1,4)
 (6,6,3,-1,0,5)
(6 rows)

pgr_pointToEdgeNode

SELECT pgr_pointToEdgeNode('edge_table', the_geom, 0.5) FROM pointsofinterest;
ERROR:  line_locate_point: 1st arg isn't a line
CONTEXT:  SQL function "st_line_locate_point" statement 2
PL/pgSQL function pgr_pointtoedgenode(text,geometry,double precision) line 32 at assignment

Yet again, even that the table has a line geometry column that is a line, but is stored on geom there is no way to tell that the LINE geometry to be used is in another column.

Conclusion

I consider this function application dependant, for example: If my application's tables columns are in Spanish, then I make a "similar one" using the "exposed code" of pgr_pointToEdgeNode as base and adjust to the application needs. If the other application's tables columns are in French then I make a "similar one" using the "exposed code" of pgr_pointToEdgeNode as base and adjust to the application needs. If the other application's tables come from osm2po then I make a "similar one" using the "exposed code" of pgr_pointToEdgeNode as base and adjust to the application needs.

For 2.5 I will deprecate.

There is this https://github.com/pgRouting/pgrouting-contrib where the code can be exposed. or it can be exposed in a gist or in a pgRouting wiki page.

woodbri commented 7 years ago

Sounds good to me.