Rewrite pgr node network v3.8 #2857
Conversation
WalkthroughThis update introduces significant changes to the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant pgr_nodeNetwork
participant pgr_separateCrossing
participant pgr_separateTouching
participant OutputTable
User->>pgr_nodeNetwork: Call with edge table and parameters
pgr_nodeNetwork->>pgr_separateCrossing: Pass edge data and tolerance
pgr_separateCrossing-->>pgr_nodeNetwork: Return split crossing edges
pgr_nodeNetwork->>pgr_separateTouching: Pass edge data and tolerance
pgr_separateTouching-->>pgr_nodeNetwork: Return split touching edges
pgr_nodeNetwork->>OutputTable: Insert results
pgr_nodeNetwork->>User: Return output table name
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (13)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (15)
NEWS.md(1 hunks)doc/src/release_notes.rst(1 hunks)doc/topology/pgr_nodeNetwork.rst(2 hunks)docqueries/topology/nodeNetwork.result(2 hunks)docqueries/utilities/separateTouching.result(1 hunks)locale/en/LC_MESSAGES/pgrouting_doc_strings.po(4 hunks)locale/pot/pgrouting_doc_strings.pot(4 hunks)pgtap/topology/nodeNetwork/issue_1074.pg(2 hunks)pgtap/topology/nodeNetwork/issue_1882.pg(1 hunks)pgtap/topology/nodeNetwork/issue_2628.pg(1 hunks)pgtap/topology/nodeNetwork/issue_280.pg(1 hunks)pgtap/topology/nodeNetwork/issue_623.pg(1 hunks)pgtap/utilities/separateTouching/issue_623.pg(3 hunks)sql/topology/nodeNetwork.sql(3 hunks)sql/utilities/separateTouching.sql(1 hunks)
👮 Files not reviewed due to content moderation or server errors (1)
- locale/pot/pgrouting_doc_strings.pot
🧰 Additional context used
🧠 Learnings (3)
NEWS.md (1)
Learnt from: cvvergara
PR: pgRouting/pgrouting#2744
File: doc/src/release_notes.rst:94-94
Timestamp: 2025-02-06T20:50:07.967Z
Learning: In pgRouting, changes to functions can happen at two levels:
1. SQL level - removal/addition of function signatures
2. C/C++ level - deprecation/changes to the underlying implementation
These are documented separately in the release notes.
pgtap/utilities/separateTouching/issue_623.pg (1)
Learnt from: cvvergara
PR: pgRouting/pgrouting#2725
File: src/coloring/edgeColoring_driver.cpp:87-87
Timestamp: 2025-01-19T16:17:53.496Z
Learning: Changes to error message handling in pgRouting functions require corresponding updates to pgtap tests to maintain test coverage.
doc/src/release_notes.rst (1)
Learnt from: cvvergara
PR: pgRouting/pgrouting#2744
File: doc/src/release_notes.rst:94-94
Timestamp: 2025-02-06T20:50:07.967Z
Learning: In pgRouting, changes to functions can happen at two levels:
1. SQL level - removal/addition of function signatures
2. C/C++ level - deprecation/changes to the underlying implementation
These are documented separately in the release notes.
🪛 markdownlint-cli2 (0.17.2)
NEWS.md
65-65: Emphasis used instead of a heading
null
(MD036, no-emphasis-as-heading)
⏰ Context from checks skipped due to timeout of 90000ms (16)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Ubuntu Boost (84)
- GitHub Check: Ubuntu Boost (78)
- GitHub Check: Ubuntu Boost (80)
- GitHub Check: Ubuntu Boost (56)
- GitHub Check: Ubuntu Boost (83)
- GitHub Check: Ubuntu Boost (86)
- GitHub Check: Ubuntu Boost (79)
- GitHub Check: Ubuntu Boost (68)
- GitHub Check: Ubuntu Boost (76)
- GitHub Check: Ubuntu Boost (77)
- GitHub Check: Ubuntu Boost (75)
- GitHub Check: Check queries
- GitHub Check: macos (macos-latest)
- GitHub Check: Ubuntu clang
- GitHub Check: macos (macos-14)
🔇 Additional comments (34)
sql/utilities/separateTouching.sql (1)
76-79: Enhanced filtering criteria for touching geometries.The filtering condition in the
touchingsCTE has been expanded to better identify cases where geometries touch. Previously, rows wheregeomequaledg1were excluded. Now, the condition also handles geometries that touch (ST_touches(g1, g2)) but don't intersect at the endpoints ofg1. This should improve edge splitting at touching points that aren't at the endpoints.NEWS.md (1)
65-69: Appropriate documentation of the pgr_nodeNetwork rewrite.The addition of the "SQL code enhancements" section properly documents the rewrite of the
pgr_nodeNetworkfunction (referencing issue #2850). This logically fits between the C/C++ code enhancements and the new proposed functions sections.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
65-65: Emphasis used instead of a heading
null(MD036, no-emphasis-as-heading)
doc/src/release_notes.rst (1)
95-99: Consistent documentation in release notes.The addition of the "SQL code enhancements" section with reference to issue #2850 (rewrite of pgr_nodeNetwork) maintains consistency with the changes in NEWS.md. This properly documents the significant enhancement to this function's implementation.
pgtap/topology/nodeNetwork/issue_1074.pg (2)
25-25: Updated test plan count.The test plan count has been increased from 15 to 16 to account for the new assertion being added (line 64). This maintains proper test accounting.
64-64: Added verification for total edge count.This new assertion validates that the combined table
original_nodedcontains exactly 13 edges (9 from original_roads + 4 from original_bridges). This helps ensure the node network operation produces the expected number of edges, which is important for the rewritten function.doc/topology/pgr_nodeNetwork.rst (2)
39-44: Documentation update correctly reflects API changes.The added availability section for version 3.8.0 clearly documents the important changes made in this release:
- Removing automatic index checking/creation
- Using the modular helper functions for geometry processing
- Updated data type for the output table
These changes align well with the PR objectives to rewrite the node network functionality.
87-89: Data type update from integer to bigint.The documentation now correctly specifies that the
sourceandtargetcolumns in the output table usebigintdata type instead ofinteger, which is consistent with the changes mentioned in the version 3.8.0 availability section.docqueries/utilities/separateTouching.result (1)
34-37: Enhanced geometry filtering logic.The WHERE clause in the
touchingsCTE has been improved to:
- Keep the original condition that excludes cases where
geom = g1- Add a new condition that includes geometries that touch (
ST_touches(g1, g2)) but don't intersect at the start or end points ofg1This change broadens the detection criteria for touching geometries, which should improve the accuracy of the node network creation.
pgtap/utilities/separateTouching/issue_623.pg (3)
21-21: Test plan count updated for both version branches.The test plan count is correctly incremented to accommodate the new test case, maintaining proper test structure for both old and new versions of pgRouting.
121-121: Added validation for total row count.Good addition of a row count validation test to ensure the total number of rows in the
houses.simplenettable equals the expected 96 (14 original edges + 82 houses).
139-139: Updated edge and vertex count expectations.The test now expects 102 edges and 101 vertices after running
pgr_separateTouching, updated from the previous values (91, 98). This change reflects the different behavior in the rewritten node network processing.locale/en/LC_MESSAGES/pgrouting_doc_strings.po (4)
11-11: Updated POT-Creation-Date timestampThis is a standard update to the timestamp when translation files are regenerated.
4019-4025: Documentation updated with new SQL code enhancements sectionThis change properly adds new message entries that document SQL code enhancements, specifically referencing issue #2850 about the rewrite of pgr_nodeNetwork.
13397-13404: Clear documentation of pgr_nodeNetwork behavior changesThe added messages clearly explain the key changes to the pgr_nodeNetwork function:
- No longer checks or creates indexes
- Uses pgr_separateTouching and pgr_separateCrossing functions
- Creates tables with BIGINT data type
This provides important information to users about the new behavior they should expect.
13455-13455: Updated data type from integer to bigintThe data type for the source and target columns has been properly updated from integer to bigint in the documentation, which correctly reflects the implementation changes mentioned in the PR objectives.
Also applies to: 13458-13458
pgtap/topology/nodeNetwork/issue_280.pg (1)
1-42: Well-structured test for basic node network functionality.This test verifies that the rewritten
pgr_nodeNetworkfunction correctly handles intersecting linestrings by:
- Creating a simple network with two intersecting linestrings
- Running
pgr_nodeNetworkwith a small tolerance- Verifying the resulting network has 4 edges (original edges split at intersection)
- Extracting vertices and confirming there are 5 vertices (4 endpoints + 1 intersection)
- Specifically checking that the intersection point at (5,5) exists
The test provides good coverage of the core functionality of splitting intersecting edges.
docqueries/topology/nodeNetwork.result (2)
170-172: Minor coordinate precision changes reflect the rewritten function.The slight change in coordinate precision (now showing
1.999999999999 3.5instead of exactly2 3.5) reflects the internal implementation changes from using the newpgr_separateCrossingandpgr_separateTouchingfunctions.
219-222: Result update reflects new implementation behavior.The updated result for the
pgr_findCloseEdgesquery shows different values for edge_id, distance and geometry compared to the previous implementation, which is expected given the function rewrite using the new helper functions.pgtap/topology/nodeNetwork/issue_1882.pg (1)
23-78: Comprehensive test for schema-qualified tables and version-specific behavior.This test thoroughly validates the rewritten
pgr_nodeNetworkfunction with:
- Schema qualification support (using 'ferry' schema)
- Complex real-world ferry route geometries
- Proper validation of table structure in the resulting noded table
- Verification that each original edge is split into exactly 4 sub-edges
- Version-conditional testing that accounts for behavior differences in pgRouting 3.8.0+
The conditional logic on line 68-78 correctly handles version-specific testing, ensuring backward compatibility while validating the new implementation.
pgtap/topology/nodeNetwork/issue_623.pg (3)
24-107: Well-designed test for complex network with custom schema.This test creates a realistic road network scenario with:
- A custom 'houses' schema
- A road network table with multiple road segments
- House locations as points
- Proper indexes and constraints
The test data provides good coverage for a real-world scenario where a complex network would need to be noded.
108-123: Realistic connectivity testing between houses and road network.This section adds edges connecting each house to its nearest road segment using
ST_ShortestLine, which creates a realistic network topology that will need to be properly handled by the node network function.
124-161: Thorough verification of connectivity and version-specific behavior.The test provides comprehensive validation by:
- Running
pgr_nodeNetworkon the complex network- Extracting vertices and updating source/target relationships
- Conditionally checking for version-specific edge counts (190 in v3.8.0+, 189 in earlier versions)
- Computing connected components to verify network integrity
- Confirming the entire network forms a single connected component
The version-specific checks on lines 144-149 ensure compatibility with both pre-3.8.0 and post-3.8.0 behavior.
pgtap/topology/nodeNetwork/issue_2628.pg (5)
22-22: Good use of conditional test planningThe test intelligently adapts the number of tests based on version compatibility, which is a good practice for maintaining backward compatibility while testing new functionality.
46-46: Function usage validates the new implementationThe test correctly calls
pgr_nodenetworkwith the appropriate parameters, including the schema-qualified table name, tolerance, and column identifiers, which helps validate the refactored implementation.
54-58: Well-implemented version-specific testThe conditional check for the column type ensures that tests are version-aware, accommodating the data type changes in 3.8.0 while skipping incompatible tests in earlier versions.
83-93: Comprehensive results validationThe test block effectively verifies multiple aspects of the function's behavior, including edge subdivision counts, total edge counts, and vertex extraction. The conditional execution based on version ensures compatibility with older pgRouting versions.
1-3: 🧹 Nitpick (assertive)Check the copyright year
The copyright year is set to 2025, which is in the future. Consider updating it to the current year.
-Copyright (c) 2025 pgRouting developers +Copyright (c) 2024 pgRouting developersLikely an incorrect or invalid review comment.
sql/topology/nodeNetwork.sql (7)
47-63: Improved variable declarations for schema awarenessThe refactored variable declarations support proper schema handling and provide clearer naming conventions. The BIGINT type for counters ensures compatibility with large datasets.
77-87: Well-implemented schema-aware table handlingThe function now properly parses the input table name into schema and table components, defaulting to the current schema if unspecified. This improves flexibility and ensures correct schema qualification in all SQL operations.
88-90: Cleaner handling of filtering conditionsThe code elegantly normalizes the filtering condition based on the provided
rows_whereparameter andoutallflag, ensuring consistent behavior regardless of input format.
91-110: Improved query construction and validationThe function now builds a SQL query string using the format function for better readability and maintainability. The error handling with detailed diagnostics helps identify issues more easily.
165-165: Appropriate data type change for old_idChanged the data type of old_id to BIGINT to maintain consistency with the source table's ID column type, which improves compatibility with large datasets.
183-191: Modular implementation using helper functionsThe core edge-splitting logic has been elegantly replaced with calls to
pgr_separateCrossingandpgr_separateTouching, which significantly improves code maintainability and follows the single responsibility principle. This approach makes the code cleaner and easier to understand.
204-215: Improved handling of untouched edgesThe function now uses a cleaner approach to insert untouched edges (those not requiring splitting) into the result table. The WITH clause and subqueries improve readability and performance compared to the previous implementation.
cvvergara
force-pushed
the
rewrite-pgr_nodeNetwork-v3.8
branch
from
0cf144c to
5be4230
Compare
3 participants
Fixes #2850
Changes proposed in this pull request:
pgr_separateCrossingandpgr_separateTouchingas the back bone of the calculation@pgRouting/admins
Summary by CodeRabbit
Documentation
Bug Fixes
Tests
Refactor