New function pgr contraction dead end #2793
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughThe changes add a new proposed function, Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client Query
participant DB as Database
participant FUNC as pgr_contractionDeadEnd
Client->>+DB: Execute SELECT pgr_contractionDeadEnd(Edges SQL, options)
DB->>+FUNC: Process dead end contraction on provided edges
FUNC-->>-DB: Return set (type, id, contracted_vertices, source, target, cost)
DB-->>-Client: Return result set
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Tip ⚡💬 Agentic Chat (Pro Plan, General Availability)
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (22)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 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: 19
🔭 Outside diff range comments (1)
doc/contraction/contraction-family.rst (1)
53-54: 🛠️ Refactor suggestionMissing detailed explanation for Dead end contraction.
While you mention "Dead end contraction" as one of the two supported algorithms, the detailed explanation for it has been removed from the documentation. Since a dedicated function is now available, consider adding a brief explanation with a reference to the full documentation for
pgr_contractionDeadEnd.1. Dead end contraction 2. Linear contraction +Dead end contraction identifies and contracts nodes that have only one adjacent edge, +reducing the graph size while preserving the connectivity. For detailed explanation, +see :doc:`pgr_contractionDeadEnd`. + Allowing the user to:
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (22)
NEWS.md(1 hunks)doc/_static/page_history.js(1 hunks)doc/contraction/CMakeLists.txt(1 hunks)doc/contraction/contraction-family.rst(1 hunks)doc/contraction/pgr_contraction.rst(1 hunks)doc/contraction/pgr_contractionDeadEnd.rst(1 hunks)doc/src/proposed.rst(1 hunks)doc/src/release_notes.rst(1 hunks)docqueries/contraction/CMakeLists.txt(1 hunks)docqueries/contraction/contractionDeadEnd.pg(1 hunks)docqueries/contraction/contractionDeadEnd.result(1 hunks)docqueries/contraction/test.conf(1 hunks)locale/en/LC_MESSAGES/pgrouting_doc_strings.po(7 hunks)locale/pot/pgrouting_doc_strings.pot(7 hunks)pgtap/contraction/contractionDeadEnd/edge_cases/compare_dijkstra.pg(1 hunks)pgtap/contraction/contractionDeadEnd/edge_cases/edge_cases.pg(1 hunks)pgtap/contraction/contractionDeadEnd/inner_query.pg(1 hunks)pgtap/contraction/contractionDeadEnd/no_crash_test.pg(1 hunks)pgtap/contraction/contractionDeadEnd/types_check.pg(1 hunks)sql/contraction/CMakeLists.txt(1 hunks)sql/contraction/deadEndContraction.sql(1 hunks)sql/sigs/pgrouting--3.8.sig(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
sql/sigs/pgrouting--3.8.sig (2)
Learnt from: cvvergara
PR: pgRouting/pgrouting#2764
File: sql/sigs/pgrouting--3.8.sig:125-136
Timestamp: 2025-03-18T01:02:51.111Z
Learning: The file sql/sigs/pgrouting--*.sig is automatically generated by a command and cannot be manually modified. Comments about naming conventions or other improvements should be directed at the source code that generates these signatures, not at the signature file itself.
Learnt from: cvvergara
PR: pgRouting/pgrouting#2764
File: sql/sigs/pgrouting--3.8.sig:274-275
Timestamp: 2025-03-18T01:02:51.111Z
Learning: The file `sql/sigs/pgrouting--3.8.sig` is auto-generated by a command and should not be modified manually.
🪛 markdownlint-cli2 (0.17.2)
NEWS.md
40-40: 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 (86)
- GitHub Check: Ubuntu Boost (80)
- GitHub Check: Ubuntu Boost (79)
- GitHub Check: Ubuntu Boost (77)
- GitHub Check: Ubuntu Boost (78)
- GitHub Check: Ubuntu clang
- GitHub Check: Ubuntu Boost (83)
- GitHub Check: Ubuntu Boost (84)
- GitHub Check: Ubuntu Boost (76)
- GitHub Check: Ubuntu Boost (75)
- GitHub Check: Ubuntu Boost (56)
- GitHub Check: Ubuntu Boost (68)
- GitHub Check: Check queries
- GitHub Check: macos (macos-14)
- GitHub Check: macos (macos-latest)
🔇 Additional comments (51)
docqueries/contraction/test.conf (1)
7-7: Addition of test file looks good!The inclusion of
contractionDeadEnd.pgto the test configuration appropriately adds test coverage for the newpgr_contractionDeadEndfunction.doc/contraction/CMakeLists.txt (1)
5-5: Documentation file addition is appropriateThe addition of
pgr_contractionDeadEnd.rstto the documentation build system properly integrates the documentation for the new function.sql/contraction/CMakeLists.txt (1)
5-5: SQL implementation file added correctlyAdding
deadEndContraction.sqlto the SQL build system ensures the new function implementation is properly included in the build process.docqueries/contraction/CMakeLists.txt (1)
4-4: Example queries file addition looks goodThe addition of
contractionDeadEndto the documentation query build system appropriately provides example queries for the new function.sql/sigs/pgrouting--3.8.sig (1)
89-89: New function signature looks good.The new function
pgr_contractiondeadend(text,boolean,bigint[])has been properly added to the signature file in the correct alphabetical order.Note: As indicated in the retrieved learnings, this file is automatically generated and should not be manually modified.
locale/en/LC_MESSAGES/pgrouting_doc_strings.po (8)
11-11: No issues found.
These metadata updates look fine and do not raise any concerns.
2772-2773: Reference to pgr_contractionDeadEnd looks consistent.
All good: the documentation string for:doc:\pgr_contractionDeadEnd`` is aligned with the newly introduced function.
2835-2853: Documentation entries for node adjacency seem fine.
The added lines defining “Node,” “Adjacent nodes,” and “Number of adjacent nodes” appear correct.
2869-2891: No issues found with these doc strings.
The clarifications about symmetrical properties and related examples are acceptable as given.
4109-4119: No issues found with the “New proposed functions” references.
References to “pgr_contractionDeadEnd” and issue #2790 are accurate and informative.
9292-9292: No issues with referencing Dead End Contraction doc.
The pointer topgr_contractionDeadEndis correct and clear.
9595-9749: Additions detailing pgr_contractionDeadEnd usage are well-structured.
No concerns here: the explanations and examples provide the necessary clarity on new function behavior.
9760-9761: 🧹 Nitpick (assertive)Correct the spelling of “contrcted.”
Replace “contrcted” with “contracted” for clarity and correctness.-After contracting :math:`2`, stop. Node :math:`1` has the information of nodes that were contrcted. +After contracting :math:`2`, stop. Node :math:`1` has the information of nodes that were contracted.Likely an incorrect or invalid review comment.
doc/src/release_notes.rst (1)
40-40: Formatting Note: Emphasis vs. HeadingA static analysis warning (MD036) flagged the use of emphasis instead of a proper heading on line 40. Given that this file is written in reStructuredText (RST) and that the formatting is chosen for consistency with pgRouting’s documentation style, this warning can be safely ignored. However, if you plan to convert or align the formatting with Markdown standards later, consider using a proper heading syntax.
locale/pot/pgrouting_doc_strings.pot (5)
11-11: Creation date update looks fine.
No issues found with this metadata change.
2557-2559: Addition ofpgr_contractionDeadEndreference is valid.
Ensure translations are added in the future when available.
2611-2628: New doc strings for node references appear correct.
All references to adjacency and node counts are consistent.
3685-3693: New references to proposed functions are clear.
Links to the GitHub issue (#2790) are correctly stated.
7978-7978: Reference to Dead End Contraction is appropriate.
Documentation cross-links align well with the new function.doc/src/proposed.rst (1)
71-76: LGTM: Clean addition of contraction-family documentation.The addition of the contraction-family section follows the same pattern as other family sections in this file, maintaining consistency with the existing documentation structure.
doc/contraction/pgr_contraction.rst (1)
78-78: Good reference redirection to the new specialized function.The modification appropriately redirects readers to the new specialized
pgr_contractionDeadEndfunction documentation, which is a better approach than keeping the implementation details in the generalpgr_contractiondocumentation.doc/_static/page_history.js (1)
18-19: LGTM: Proper versioning for the new function.The new entry is correctly added to the
newpagesarray for version 3.8, following the established pattern of the file. This ensures that the documentation system properly associates the new function with the appropriate version.sql/contraction/deadEndContraction.sql (3)
1-24: LGTM: License header looks good.The license header follows the project's standard format and includes the appropriate copyright notice and GPL license text.
25-44: Function implementation is clean and follows project conventions.The function signature and implementation look good:
- Version marking (--v3.8) is included
- Parameters are clearly documented with comments
- Default values are appropriate
- Implementation correctly calls the internal
_pgr_contractionfunction with the dead end contraction method (ARRAY[1])This specialized wrapper function will make it easier for users to perform specifically dead end contractions without having to understand the internals of the contraction methods.
45-54: Documentation comment is well structured.The function comment provides clear information about the parameters and includes a link to the detailed documentation. The format follows the project's standards.
pgtap/contraction/contractionDeadEnd/types_check.pg (4)
21-21: Function plan and version check are well structured.Good job setting up the plan count conditionally based on version compatibility.
33-35: Comprehensive function existence and signature checks.The tests appropriately verify that the function exists with the correct signature and return type.
37-41: Thorough parameter name verification.The test correctly verifies that the function has the expected parameter names in the right order.
43-44: Verify parameter types match function signature.This test ensures the function accepts the correct parameter types.
doc/contraction/contraction-family.rst (2)
24-32: Correctly includes the new function in proposed section.The documentation properly includes the warning about proposed functionality and lists the new
pgr_contractionDeadEndfunction.
38-38: Function added to documentation navigation.The new function is correctly added to the hidden toctree for proper documentation navigation.
pgtap/contraction/contractionDeadEnd/inner_query.pg (3)
34-36: Good tests for different directed parameter values.The tests correctly verify both directed and undirected graph functionality.
37-71: Comprehensive input validation tests.The tests thoroughly verify the function's behavior with different array types and dimensions, ensuring it properly handles valid inputs and rejects invalid ones.
3-3: 🧹 Nitpick (assertive)Update copyright year to current year.
The copyright year is set to 2025, but it should reflect the current year (2025) or when the code was actually created.
-Copyright (c) 2025 pgRouting developers +Copyright (c) 2024 pgRouting developersLikely an incorrect or invalid review comment.
docqueries/contraction/contractionDeadEnd.pg (3)
3-16: Well-structured basic usage examples.The basic examples demonstrate proper usage of the function with both table and inline values.
17-42: Good examples with different graph structures.These examples show how the function handles various graph structures including cul-de-sacs and directed edges.
78-99: Complex Dijkstra examples demonstrate practical usage.These examples effectively show how to integrate the contraction results with Dijkstra algorithm for different scenarios, including handling cul-de-sac vertices.
pgtap/contraction/contractionDeadEnd/no_crash_test.pg (2)
31-55: Good approach for conditional version checks.
The structure oftest_function()correctly accounts formin_version('3.8.0')and skips tests on earlier versions, ensuring forward-compatibility.
22-22:❓ Verification inconclusive
Verify potential zero-cost edges.
Usingsign(cost)andsign(reverse_cost)may unintentionally convert zero-cost edges to zero, possibly affecting edge traversal logic.Please run the following script to find occurrences of zero-cost edges and confirm if they need special handling:
🏁 Script executed:
#!/bin/bash # Searching for edges with zero cost. rg -A 3 --line-number 'INSERT INTO edges|UPDATE edges|VALUES' . rg "('cost'|sign\(cost\))"Length of output: 67828
Action: Verify Handling of Zero‐Cost Edges
The update statement in this file (and similar ones across the test suite) applies the built‐in PostgreSQL function
sign(), which maps positive values to 1, negative values to –1, and leaves zero unchanged. Our grep results indicate that some test inserts (for example, in the contraction tapfuncs data) explicitly include zero-cost edges. Please confirm whether retaining a cost of 0 for such edges is intended or if they should be adjusted (e.g. mapped to a nonzero default) so as not to inadvertently affect downstream edge traversal logic.pgtap/contraction/contractionDeadEnd/edge_cases/compare_dijkstra.pg (2)
24-25: Check potential conflicts with newly added columns.
Addingis_newandcontracted_verticescolumns toedgesandverticesmight conflict with existing columns if they already exist. Consider adding a safeguard to handle potential name collisions or to drop them first if they exist.
101-107: Assess the integration of related calls.
create_contracted_graph,check_cg, andcompare_dijkstra_contractionare chained correctly. Verify that the test suite captures all relevant states before and after calling each procedure to ensure consistency.docqueries/contraction/contractionDeadEnd.result (1)
146-172: Validate correctness of path cost data.
The returned costs in these rows are critical for verifying correct shortest-path logic. Double-check actual edge values match expected results from the data definitions.pgtap/contraction/contractionDeadEnd/edge_cases/edge_cases.pg (9)
22-22: Great approach using version-dependent test planning!The conditional test plan count (128 for version >= 3.8.0, 1 otherwise) ensures appropriate test execution based on feature availability.
24-31: Well-structured test data table design.The graphs table contains all necessary fields for testing the contraction functionality, with the dead_case field allowing for organized test case management.
33-63: Good use of descriptive comments for graph structures.The comments provide clear visualization of the graph structures being tested, which enhances code readability and maintainability.
120-244: Comprehensive test coverage for different graph scenarios.The edge_cases_sampledata function provides excellent coverage of different graph configurations with various forbidden vertices combinations. The systematic approach ensures all edge cases are properly tested.
269-280: Clear handling of directed vs undirected graph differences.The code correctly handles the differences between directed and undirected graphs with appropriate conditional logic and test expectations.
291-295: Proper version checking for feature availability.The code correctly checks if the current PostgreSQL version supports the new pgr_contractionDeadEnd function (>= 3.8.0) and skips tests appropriately if not supported.
297-305: Well-formed SQL prepared statements for function testing.The prepared statements properly call the new pgr_contractionDeadEnd function with appropriate parameters, ensuring consistent test execution.
307-310: Thorough testing with both directed and undirected graphs.Testing both directed (true) and undirected (false) scenarios ensures complete coverage of the function's behavior in all contexts.
316-319: Proper test cleanup with transaction rollback.The code properly finalizes the tests and rolls back the transaction, ensuring test data doesn't persist in the database.
pgtap/contraction/contractionDeadEnd/edge_cases/compare_dijkstra.pg
Outdated
Show resolved
Hide resolved
Fixes #2790 .
Changes proposed in this pull request:
@pgRouting/admins
Summary by CodeRabbit
pgr_contractionDeadEnd, enhancing network routing capabilities.pgr_contractionDeadEndfunction, including usage examples and updates to related sections.pgr_contractionDeadEndunder various scenarios.