Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GSoC 2023: Yige Huang Week 2 #299

Merged
merged 8 commits into from
Jun 10, 2023
Merged

GSoC 2023: Yige Huang Week 2 #299

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
3cbc379
[withPointsDD][SQL] output columns include start_vid in SINGLE
squarege Jun 8, 2023
c7e56ae
[withPointsDD][pgtap] updating to include new output columns
squarege Jun 9, 2023
ef24f0f
[withPointsDD][docqueries] documentation queries result changes
squarege Jun 9, 2023
8756440
[withPointsDD][SQL] set the default param of driving_side to r
squarege Jun 9, 2023
7d3af8a
[withPointsDD][C] any other value will be considered as r
squarege Jun 9, 2023
a7f46ef
[withPoints][pgtap] updating edge_cases
squarege Jun 9, 2023
dd845f7
[withPoints][pgtap] removing unused code
squarege Jun 9, 2023
184c357
[withPoints][docqueries] removing the both driving side test
squarege Jun 9, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 8 additions & 29 deletions docqueries/withPoints/doc-pgr_withPointsDD.result
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,13 @@ SELECT * FROM pgr_withPointsDD(
-1, 3.3,
driving_side => 'r',
details => true);
seq | node | edge | cost | agg_cost
-----+------+------+------+----------
1 | -1 | -1 | 0 | 0
2 | 5 | 1 | 0.4 | 0.4
3 | 6 | 1 | 1 | 1.4
4 | -6 | 4 | 0.7 | 2.1
5 | 7 | 4 | 0.3 | 2.4
seq | start_vid | node | edge | cost | agg_cost
-----+-----------+------+------+------+----------
1 | -1 | -1 | -1 | 0 | 0
2 | -1 | 5 | 1 | 0.4 | 0.4
3 | -1 | 6 | 1 | 1 | 1.4
4 | -1 | -6 | 4 | 0.7 | 2.1
5 | -1 | 7 | 4 | 0.3 | 2.4
(5 rows)

/* -- q3 */
Expand All @@ -44,27 +44,6 @@ SELECT * FROM pgr_withPointsDD(
(12 rows)

/* -- q4 */
SELECT * FROM pgr_withPointsDD(
'SELECT id, source, target, cost, reverse_cost FROM edges ORDER BY id',
'SELECT pid, edge_id, fraction, side from pointsOfInterest',
-1, 3.3,
driving_side => 'b',
details => true);
seq | node | edge | cost | agg_cost
-----+------+------+------+----------
1 | -1 | -1 | 0 | 0
2 | 5 | 1 | 0.4 | 0.4
3 | 6 | 1 | 0.6 | 0.6
4 | -6 | 4 | 0.7 | 1.3
5 | 7 | 4 | 0.3 | 1.6
6 | 3 | 7 | 1 | 2.6
7 | 8 | 10 | 1 | 2.6
8 | 11 | 8 | 1 | 2.6
9 | -3 | 12 | 0.6 | 3.2
10 | -4 | 6 | 0.7 | 3.3
(10 rows)

/* -- q5 */
SELECT * FROM pgr_withPointsDD(
$e$ SELECT * FROM edges $e$,
$p$ SELECT edge_id, round(fraction::numeric, 2) AS fraction, side
Expand Down Expand Up @@ -101,6 +80,6 @@ SELECT * FROM pgr_withPointsDD(
21 | -2 | 17 | 13 | 1 | 2.1
(21 rows)

/* -- q6 */
/* -- q5 */
ROLLBACK;
ROLLBACK
9 changes: 1 addition & 8 deletions docqueries/withPoints/doc-pgr_withPointsDD.test.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,6 @@ SELECT * FROM pgr_withPointsDD(
driving_side => 'l',
equicost => true);
/* -- q4 */
SELECT * FROM pgr_withPointsDD(
Copy link
Member

Choose a reason for hiding this comment

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

Do not delete the query its been used in the documentation.
Put it back.

'SELECT id, source, target, cost, reverse_cost FROM edges ORDER BY id',
'SELECT pid, edge_id, fraction, side from pointsOfInterest',
-1, 3.3,
driving_side => 'b',
details => true);
/* -- q5 */
SELECT * FROM pgr_withPointsDD(
$e$ SELECT * FROM edges $e$,
$p$ SELECT edge_id, round(fraction::numeric, 2) AS fraction, side
Expand All @@ -34,4 +27,4 @@ SELECT * FROM pgr_withPointsDD(
ARRAY[-1, -2], 2.3,
driving_side => 'r',
details => true);
/* -- q6 */
/* -- q5 */
81 changes: 16 additions & 65 deletions pgtap/withPoints/withPointsDD/edge_cases/issue_979.pg
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,53 +20,17 @@ Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.

BEGIN;

SELECT PLAN(6);
Copy link
Member

Choose a reason for hiding this comment

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

Please do not change pgtap tests at this time, put it back as it was ...
Of course pgtap will fail, it is expected.

SELECT PLAN(4);


---
--- DIRECTED GRAPH
---
-------- both driving sides

PREPARE q1 AS
SELECT node, edge, round(cost::numeric, 12) AS cost, round(agg_cost::numeric, 12) AS agg_cost FROM pgr_withPointsDD(
'SELECT id, source, target, cost, reverse_cost FROM edge_table ORDER BY id',
'SELECT pid, edge_id, fraction, side from pointsOfInterest',
-1, 6.8, driving_side := 'b', details := false)
ORDER BY seq;


SELECT
node::BIGINT, edge::BIGINT, round(cost, 12) AS cost, round(agg_cost, 12) AS agg_cost
INTO test1
FROM
(VALUES
( -1 , -1 , 0 , 0),
( 1 , 1 , 0.4 , 0.4),
( 2 , 1 , 0.6 , 0.6),
( 5 , 4 , 1 , 1.6),
( 6 , 8 , 1 , 2.6),
( 8 , 7 , 1 , 2.6),
( 10 , 10 , 1 , 2.6),
( 7 , 6 , 1 , 3.6),
( 9 , 9 , 1 , 3.6),
( 11 , 11 , 1 , 3.6),
( 13 , 14 , 1 , 3.6),
( 4 , 16 , 1 , 4.6),
( 12 , 13 , 1 , 4.6),
( 3 , 3 , 1 , 5.6)
) AS t (node, edge, cost, agg_cost);


SELECT set_eq('q1',
$$SELECT node, edge, round(cost::numeric, 12) AS cost, round(agg_cost::numeric, 12) AS agg_cost FROM test1$$,
'Should be aggregating individual costs: both driving sides, DIR');



-------- right driving side

PREPARE q2 AS
PREPARE q1 AS
SELECT node, edge, round(cost::numeric, 12) AS cost, round(agg_cost::numeric, 12) AS agg_cost FROM pgr_withPointsDD(
'SELECT id, source, target, cost, reverse_cost FROM edge_table ORDER BY id',
'SELECT pid, edge_id, fraction, side from pointsOfInterest',
Expand All @@ -75,7 +39,7 @@ SELECT node, edge, round(cost::numeric, 12) AS cost, round(agg_cost::numeric, 12

SELECT
node::BIGINT, edge::BIGINT, cost::FLOAT, agg_cost::FLOAT
INTO test2
INTO test1
FROM
(VALUES
( -1 , -1 , 0 , 0),
Expand All @@ -95,13 +59,13 @@ FROM
) AS t (node, edge, cost, agg_cost);


SELECT set_eq('q2',
$$SELECT node, edge, round(cost::numeric, 12) AS cost, round(agg_cost::numeric, 12) AS agg_cost FROM test2$$,
SELECT set_eq('q1',
$$SELECT node, edge, round(cost::numeric, 12) AS cost, round(agg_cost::numeric, 12) AS agg_cost FROM test1$$,
'Should be aggregating individual costs: right driving side, DIR');

-------- left driving side

PREPARE q3 AS
PREPARE q2 AS
SELECT node, edge, round(cost::numeric, 12) AS cost, round(agg_cost::numeric, 12) AS agg_cost FROM pgr_withPointsDD(
'SELECT id, source, target, cost, reverse_cost FROM edge_table ORDER BY id',
'SELECT pid, edge_id, fraction, side from pointsOfInterest',
Expand All @@ -111,7 +75,7 @@ ORDER BY seq;

SELECT
node::BIGINT, edge::BIGINT, cost::FLOAT, agg_cost::FLOAT
INTO test3
INTO test2
FROM
(VALUES
(-1 , -1 , 0 , 0),
Expand All @@ -131,8 +95,8 @@ FROM
) AS t (node, edge, cost, agg_cost);


SELECT set_eq('q3',
$$SELECT node, edge, round(cost::numeric, 12) AS cost, round(agg_cost::numeric, 12) AS agg_cost FROM test3$$,
SELECT set_eq('q2',
$$SELECT node, edge, round(cost::numeric, 12) AS cost, round(agg_cost::numeric, 12) AS agg_cost FROM test2$$,
'Should be aggregating individual costs: left driving side, DIR');

---
Expand All @@ -142,7 +106,7 @@ SELECT set_eq('q3',
-- all results on udirected graph are "allegedly" equal
SELECT
node::BIGINT, cost::FLOAT, agg_cost::FLOAT
INTO test4
INTO test3
FROM
(VALUES
( -1 , 0 , 0),
Expand All @@ -161,44 +125,31 @@ FROM
( 12 , 1 , 4.6)
) AS t (node, cost, agg_cost);

-------- both driving sides

PREPARE q4 AS
SELECT node, round(cost::numeric, 12) AS cost, round(agg_cost::numeric, 12) AS agg_cost FROM pgr_withPointsDD(
'SELECT id, source, target, cost, reverse_cost FROM edge_table ORDER BY id',
'SELECT pid, edge_id, fraction, side from pointsOfInterest',
-1, 6.8, driving_side := 'b', details := false, directed:=false);

SELECT set_eq('q4',
$$SELECT node, round(cost::numeric, 12) AS cost, round(agg_cost::numeric, 12) AS agg_cost FROM test4$$,
'Should be aggregating individual costs: both driving sides, UNDI');



-------- right driving side

PREPARE q5 AS
PREPARE q4 AS
SELECT node, round(cost::numeric, 12) AS cost, round(agg_cost::numeric, 12) AS agg_cost FROM pgr_withPointsDD(
'SELECT id, source, target, cost, reverse_cost FROM edge_table ORDER BY id',
'SELECT pid, edge_id, fraction, side from pointsOfInterest',
-1, 6.8, driving_side := 'r', details := false, directed:=false);


SELECT set_eq('q5',
$$SELECT node, round(cost::numeric, 12) AS cost, round(agg_cost::numeric, 12) AS agg_cost FROM test4$$,
SELECT set_eq('q4',
$$SELECT node, round(cost::numeric, 12) AS cost, round(agg_cost::numeric, 12) AS agg_cost FROM test3$$,
'Should be aggregating individual costs: right driving side, UNDI');

-------- left driving side

PREPARE q6 AS
PREPARE q5 AS
SELECT node, round(cost::numeric, 12) AS cost, round(agg_cost::numeric, 12) AS agg_cost FROM pgr_withPointsDD(
'SELECT id, source, target, cost, reverse_cost FROM edge_table ORDER BY id',
'SELECT pid, edge_id, fraction, side from pointsOfInterest',
-1, 6.8, driving_side := 'l', details := false, directed:=false);


SELECT set_eq('q6',
$$SELECT node, round(cost::numeric, 12) AS cost, round(agg_cost::numeric, 12) AS agg_cost FROM test4$$,
SELECT set_eq('q5',
$$SELECT node, round(cost::numeric, 12) AS cost, round(agg_cost::numeric, 12) AS agg_cost FROM test3$$,
'Should be aggregating individual costs: left driving side, UNDI');

SELECT * FROM finish();
Expand Down
20 changes: 3 additions & 17 deletions pgtap/withPoints/withPointsDD/edge_cases/undirected_driving_side.pg
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.

BEGIN;

SELECT PLAN(3);
SELECT PLAN(1);


-- because the graph is undirected, It will not matter the side of the point
Expand All @@ -29,17 +29,9 @@ SELECT PLAN(3);
---


-------- both driving sides
PREPARE q1 AS
SELECT seq, node, edge, cost::TEXT, agg_cost::TEXT FROM pgr_withPointsDD(
'SELECT id, source, target, cost, reverse_cost FROM edge_table ORDER BY id',
'SELECT pid, edge_id, fraction, side from pointsOfInterest',
-1, 4.8, driving_side := 'b', details := true, directed:=false)
ORDER BY seq;

-------- right driving side

PREPARE q2 AS
PREPARE q1 AS
SELECT seq, node, edge, cost::TEXT, agg_cost::TEXT FROM pgr_withPointsDD(
'SELECT id, source, target, cost, reverse_cost FROM edge_table ORDER BY id',
'SELECT pid, edge_id, fraction, side from pointsOfInterest',
Expand All @@ -49,7 +41,7 @@ ORDER BY seq;

-------- left driving side

PREPARE q3 AS
PREPARE q2 AS
SELECT seq, node, edge, cost::TEXT, agg_cost::TEXT FROM pgr_withPointsDD(
'SELECT id, source, target, cost, reverse_cost FROM edge_table ORDER BY id',
'SELECT pid, edge_id, fraction, side from pointsOfInterest',
Expand All @@ -58,12 +50,6 @@ ORDER BY seq;


SELECT set_eq('q1', 'q2',
'Should be equal: both driving sides with right driving side');

SELECT set_eq('q1', 'q3',
'Should be equal: both driving sides with left driving side');

SELECT set_eq('q2', 'q3',
'Should be equal: right driving sides with left driving side');

SELECT * FROM finish();
Expand Down
4 changes: 2 additions & 2 deletions pgtap/withPoints/withPointsDD/types_check.pg
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,14 +32,14 @@ SELECT function_returns('pgr_withpointsdd', ARRAY['text','text','anyarray','doub
SELECT set_eq(
$$SELECT proargnames from pg_proc where proname = 'pgr_withpointsdd'$$,
$$VALUES
('{"","","","","directed","driving_side","details","seq","node","edge","cost","agg_cost"}'::TEXT[]),
('{"","","","","directed","driving_side","details","seq","start_vid","node","edge","cost","agg_cost"}'::TEXT[]),
('{"","","","","directed","driving_side","details","equicost","seq","start_vid","node","edge","cost","agg_cost"}'::TEXT[])
$$);

SELECT set_eq(
$$SELECT proallargtypes from pg_proc where proname = 'pgr_withpointsdd'$$,
$$VALUES
('{25,25,20,701,16,1042,16,23,20,20,701,701}'::OID[]),
('{25,25,20,701,16,1042,16,23,20,20,20,701,701}'::OID[]),
('{25,25,2277,701,16,1042,16,16,23,20,20,20,701,701}'::OID[])
$$);

Expand Down
2 changes: 1 addition & 1 deletion sql/driving_distance/_withPointsDD.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ CREATE FUNCTION _pgr_withPointsDD(
distance FLOAT,

directed BOOLEAN DEFAULT true,
driving_side CHAR DEFAULT 'b',
driving_side CHAR DEFAULT 'r',
Copy link
Member

Choose a reason for hiding this comment

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

I still like the default value to be r, About getting rid of b we need to discuss

details BOOLEAN DEFAULT false,
equicost BOOLEAN DEFAULT false,

Expand Down
15 changes: 8 additions & 7 deletions sql/driving_distance/withPointsDD.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,41 +23,42 @@ Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
********************************************************************PGR-GNU*/

-- SINGLE
--v2.6
--v3.6
CREATE FUNCTION pgr_withPointsDD(
TEXT, --edges_sql (required)
TEXT, -- points_sql (required)
BIGINT, -- from_vid (required)
FLOAT, -- distance (required)

directed BOOLEAN DEFAULT true,
driving_side CHAR DEFAULT 'b',
driving_side CHAR DEFAULT 'r',
details BOOLEAN DEFAULT false,

OUT seq INTEGER,
OUT start_vid BIGINT,
OUT node BIGINT,
OUT edge BIGINT,
OUT cost FLOAT,
OUT agg_cost FLOAT)
RETURNS SETOF RECORD AS
$BODY$
SELECT seq, node, edge, cost, agg_cost
SELECT seq, start_vid, node, edge, cost, agg_cost
FROM _pgr_withPointsDD(_pgr_get_statement($1), _pgr_get_statement($2), ARRAY[$3]::BIGINT[], $4, $5, $6, $7, false);
$BODY$
LANGUAGE SQL VOLATILE STRICT
COST 100
ROWS 1000;

-- MULTIPLE
--v2.6
--v3.6
CREATE FUNCTION pgr_withPointsDD(
TEXT, --edges_sql (required)
TEXT, -- points_sql (required)
ANYARRAY, -- from_vid (required)
FLOAT, -- distance (required)

directed BOOLEAN DEFAULT true,
driving_side CHAR DEFAULT 'b',
driving_side CHAR DEFAULT 'r',
Copy link
Member

Choose a reason for hiding this comment

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

This is a tricky change,
Basically the default for versions 3.5 & under is band the default for 3.6 and after is r
For the moment lest leave it like it is here, but open an issue "reminder of default value" in your fork and put a link to this comment

Copy link
Contributor Author

Choose a reason for hiding this comment

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

issue for reminder: squarege#5

Copy link
Member

Choose a reason for hiding this comment

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

See #300

details BOOLEAN DEFAULT false,
equicost BOOLEAN DEFAULT false,

Expand Down Expand Up @@ -89,7 +90,7 @@ IS 'pgr_withPointsDD(Single Vertex)
- Distance
- Optional Parameters
- directed := true
- driving_side := b
- driving_side := r
- details := false
- Documentation:
- ${PROJECT_DOC_LINK}/pgr_withPointsDD.html
Expand All @@ -106,7 +107,7 @@ IS 'pgr_withPointsDD(Multiple Vertices)
- Distance
- Optional Parameters
- directed := true
- driving_side := b
- driving_side := r
- details := false
- equicost := false
- Documentation:
Expand Down
5 changes: 4 additions & 1 deletion src/driving_distance/many_to_dist_withPointsDD.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,9 @@ void process(
Path_rt **result_tuples,
size_t *result_count) {
driving_side[0] = estimate_drivingSide(driving_side[0]);
if (driving_side[0] != 'r' && driving_side[0] != 'l') {
Copy link
Member

@cvvergara cvvergara Jun 11, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Probably not the best place to check, did you analyze what estimate_drivingSide is doing?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I did. estimate_drivingSide converts the input character to lowercase, and returns r, l or b (if it is neither r nor l)
As the estimate_drivingSide function is also used by other withPoints functions, modifying it will have a greater impact,
so I made modifications above with reference to the following code:

GSoC-pgRouting/src/trsp/trsp_withPoints.c

Lines 68 to 71 in 81bcd0c

driving_side[0] = estimate_drivingSide(driving_side[0]);
if (driving_side[0] != 'r' && driving_side[0] != 'l') {
driving_side[0] = 'l';
}

But if the default for versions 3.6 and after is r, maybe I can change estimate_drivingSide?

Copy link
Member

Choose a reason for hiding this comment

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

See #300

driving_side[0] = 'r';
}
PGR_DBG("estimated driving side:%c", driving_side[0]);

pgr_SPI_connect();
Expand Down Expand Up @@ -167,7 +170,7 @@ _pgr_withpointsdd(PG_FUNCTION_ARGS) {
// distance FLOAT,
//
// directed BOOLEAN -- DEFAULT true,
// driving_side CHAR -- DEFAULT 'b',
// driving_side CHAR -- DEFAULT 'r',
// details BOOLEAN -- DEFAULT false,
// equicost BOOLEAN -- DEFAULT false,

Expand Down