New function pgr contraction linear #2794
Conversation
WalkthroughThe pull request introduces a new function, Changes
Sequence Diagram(s)sequenceDiagram
participant Client as SQL Client
participant Function as pgr_contractionLinear()
participant Processor as _pgr_contraction()
participant Database as DB
Client->>Function: Call with edges SQL and options
Function->>Processor: Delegate contraction processing
Processor->>Database: Execute query on graph edges
Database-->>Processor: Return contraction results
Processor-->>Function: Send processed record set
Function-->>Client: Return contracted graph data
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
Note 🎁 Summarized by CodeRabbit FreeYour organization has reached its limit of developer seats under the Pro Plan. For new users, CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please add seats to your subscription by visiting https://app.coderabbit.ai/login.If you believe this is a mistake and have available seats, please assign one to the pull request author through the subscription management page using the link above. 🪧 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: 14
🔭 Outside diff range comments (1)
doc/contraction/contraction-family.rst (1)
51-56: 🧹 Nitpick (assertive)Ensure the introduction section is consistent with the new function.
The introduction mentions "two algorithms" including "Linear contraction" but doesn't specifically mention the new
pgr_contractionLinearfunction. Consider updating this section to explicitly reference the new function alongside existing algorithms.This implementation gives a flexible framework for adding contraction algorithms in the future, currently, it supports two algorithms: 1. Dead end contraction 2. Linear contraction + +These are implemented in the functions `pgr_contractionDeadEnd` and +`pgr_contractionLinear` respectively, while `pgr_contraction` provides a more +general interface that can combine multiple contraction types.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (24)
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(2 hunks)doc/contraction/pgr_contractionLinear.rst(1 hunks)doc/src/release_notes.rst(1 hunks)docqueries/contraction/CMakeLists.txt(1 hunks)docqueries/contraction/contractionLinear.pg(1 hunks)docqueries/contraction/contractionLinear.result(1 hunks)docqueries/contraction/test.conf(1 hunks)locale/en/LC_MESSAGES/pgrouting_doc_strings.po(6 hunks)locale/pot/pgrouting_doc_strings.pot(6 hunks)pgtap/contraction/contraction/edge_cases/deadend/directed/id_size.pg(1 hunks)pgtap/contraction/contraction/edge_cases/deadend/undirected/id_size.pg(1 hunks)pgtap/contraction/contractionLinear/edge_cases/compare_dijkstra.pg(1 hunks)pgtap/contraction/contractionLinear/edge_cases/edge_cases.pg(1 hunks)pgtap/contraction/contractionLinear/inner_query.pg(1 hunks)pgtap/contraction/contractionLinear/no_crash_test.pg(1 hunks)pgtap/contraction/contractionLinear/types_check.pg(1 hunks)sql/contraction/CMakeLists.txt(1 hunks)sql/contraction/linearContraction.sql(1 hunks)sql/sigs/pgrouting--3.8.sig(1 hunks)tools/testers/contraction_tapfuncs.sql(0 hunks)
💤 Files with no reviewable changes (1)
- tools/testers/contraction_tapfuncs.sql
🧰 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-22T14:55:12.650Z
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-22T14:55:12.650Z
Learning: The file `sql/sigs/pgrouting--3.8.sig` is auto-generated by a command and should not be modified manually.
⏰ Context from checks skipped due to timeout of 90000ms (15)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Ubuntu Boost (86)
- GitHub Check: Ubuntu Boost (83)
- GitHub Check: Ubuntu Boost (76)
- GitHub Check: Ubuntu Boost (84)
- GitHub Check: Ubuntu Boost (80)
- GitHub Check: Ubuntu Boost (79)
- GitHub Check: Ubuntu Boost (56)
- GitHub Check: Ubuntu Boost (78)
- GitHub Check: Ubuntu Boost (77)
- GitHub Check: Ubuntu clang
- GitHub Check: Ubuntu Boost (75)
- GitHub Check: Ubuntu Boost (68)
- GitHub Check: macos (macos-14)
- GitHub Check: macos (macos-latest)
🔇 Additional comments (58)
pgtap/contraction/contraction/edge_cases/deadend/directed/id_size.pg (1)
22-22:✅ Verification successful
Check if test reduction is intentional.
The number of planned tests has been reduced from 18 to 12. This change should align with the actual number of tests being executed in this file. Ensure that this reduction is intentional and consistent with the actual test implementation.
🏁 Script executed:
#!/bin/bash # Count the number of actual test calls in the file grep -c "SELECT test_ids_size" pgtap/contraction/contraction/edge_cases/deadend/directed/id_size.pgLength of output: 103
Based on the information provided:
- The file now has a plan of 12 tests (via
SELECT plan(12);).- A grep for
"SELECT test_ids_size"returns three occurrences. If each of these calls internally runs four assertions (3 × 4 = 12), then the test plan is consistent.If this is indeed how
test_ids_size()is implemented (i.e. aggregating multiple assertions per call), the reduction from 18 to 12 planned tests now appears to be intentional and aligned with the actual tests executed.Please verify that each call to
test_ids_size()is intended to cover four individual assertions. If that is the case, the revised plan is correct.docqueries/contraction/test.conf (1)
8-8: LGTM - Test configuration correctly updated.The new
contractionLinear.pgfile has been added to the test configuration, which is necessary for testing the new functionpgr_contractionLinear.sql/contraction/CMakeLists.txt (1)
6-6: LGTM - Build configuration properly updated.The
linearContraction.sqlfile has been added to theLOCAL_FILESlist in CMakeLists.txt, ensuring it will be included in the build process. This is necessary for the newpgr_contractionLinearfunction.sql/sigs/pgrouting--3.8.sig (1)
90-90: New function signature correctly added.The new function
pgr_contractionlinear(text, boolean, bigint[])has been added to the signature file. This is consistent with the PR objectives to add this new function.Note: According to previous feedback, this file is automatically generated by a command and should not be manually modified.
pgtap/contraction/contraction/edge_cases/deadend/undirected/id_size.pg (1)
22-22:❌ Incorrect review comment
Verify test plan reduction from 18 to 12 tests.
The number of planned tests has been reduced from 18 to 12. Please confirm that this reduction is intentional and that all necessary test cases are still covered by the remaining tests.
🏁 Script executed:
#!/bin/bash # Description: Check previous version and current implementation of this test file # Look for previous version of this file to compare test counts git show HEAD~:pgtap/contraction/contraction/edge_cases/deadend/undirected/id_size.pg | grep "SELECT plan" # Examine what tests might have been changed or removed git diff HEAD~ pgtap/contraction/contraction/edge_cases/deadend/undirected/id_size.pgLength of output: 213
Test Plan Count Verification: No Change Detected
After verifying the file with the provided scripts, I confirmed that both the current and previous versions of
pgtap/contraction/contraction/edge_cases/deadend/undirected/id_size.pgspecifySELECT plan(12);. There is no evidence of a reduction from 18 to 12 tests in this file.
- The previous commit (HEAD~) already used
SELECT plan(12);.- The diff between HEAD~ and HEAD shows no changes in the test plan.
It appears that the review comment’s note on a reduction from 18 to 12 tests is based on an outdated expectation. Please confirm if the current test count is intentional or if additional adjustments are expected elsewhere.
Likely an incorrect or invalid review comment.
doc/contraction/CMakeLists.txt (1)
6-6: LGTM: Documentation file for new function properly added.The documentation file
pgr_contractionLinear.rsthas been correctly added to theLOCAL_FILESlist, ensuring it will be included in the build process.NEWS.md (1)
45-45: LGTM: New function properly documented in release notes.The new function
pgr_contractionLinearis correctly added to the release notes under "New proposed functions" with a reference to the corresponding GitHub issue #2791.docqueries/contraction/CMakeLists.txt (1)
5-5: LGTM: Query examples configuration properly updated.The
contractionLinearentry has been correctly added to theLOCAL_FILESlist, ensuring that its query examples will be properly configured and included in the documentation build process.doc/_static/page_history.js (1)
18-18: Addition of new function to documentation history is appropriate.The update to include 'pgr_contractionLinear' in the version 3.8 pages history is consistent with the PR objective of introducing this new function. This ensures proper documentation linking and version tracking.
doc/src/release_notes.rst (1)
75-75: New function entry in release notes correctly links to its issue.The addition of the new function
pgr_contractionLinearunder the "New proposed functions" section properly references the GitHub issue #2791, which aligns with the PR description. This ensures users can find more details about the function implementation by following the link to the issue.locale/en/LC_MESSAGES/pgrouting_doc_strings.po (4)
11-11: No concerns on auto-generated header information.
This line appears to be automatically updated by the .po file generation process. No issues found.
3983-3987: References to #2791 for pgr_contractionLinear look consistent.
The newly added reference cleanly matches the issue number and function name.
9163-9165: Properly references pgr_contractionLinear documentation.
No issues noted here; linking to linear contraction doc is accurate.
9543-9548: New doc placeholders for “Node” & “Adjacent nodes” are correct.
All translations appear correctly placed for future updates.locale/pot/pgrouting_doc_strings.pot (4)
11-11: No issues found in updated creation date.
The updated POT-Creation-Date reflects the latest template generation time, and this is a standard change in translation files.
3574-3576: The reference to issue #2791 for pgr_contractionLinear is correct.
This addition accurately links the new function to its corresponding GitHub issue and aligns with the proposed functionality.
7864-7864: Reference to :doc:pgr_contractionLinearappears consistent.
The doc link is properly formatted and follows existing conventions in the file.
8203-8208: No issues found in these added documentation strings.
They align well with the rest of the file’s structure and confirm standard usage.pgtap/contraction/contractionLinear/types_check.pg (3)
28-31: Version check is well implemented.The version check correctly skips tests when running on PostgreSQL versions before 3.8.0.
33-36: Function signature verification looks correct.The tests properly verify the existence of
pgr_contractionlinearand check that it accepts the expected parameters and returns the correct type.
37-42: Column names verification is thorough.The test verifies that the function returns the expected column names, which helps ensure API stability.
doc/contraction/contraction-family.rst (1)
39-39: Appropriate documentation addition.The new function
pgr_contractionLinearis correctly added to the toctree, ensuring it will be included in the documentation structure.sql/contraction/linearContraction.sql (3)
26-38: Function signature looks well-defined.The function has clear parameters with appropriate types and default values. The return type and output columns match the expected interface for contraction functions.
40-42: Implementation uses correct internal function.The function correctly calls
_pgr_contractionwithARRAY[2]::BIGINT[]to specify linear contraction (algorithm ID 2), properly passing through the other arguments.
45-54: Documentation is comprehensive.The function comment provides clear documentation about parameters and links to the full documentation.
pgtap/contraction/contractionLinear/edge_cases/compare_dijkstra.pg (3)
24-25: Table alterations look appropriate.The code adds necessary columns to store contraction information.
27-29: Comprehensive testing approach.The test correctly tests both directed and undirected graphs, which is important for graph algorithms. The approach of creating a contracted graph, checking its structure, and comparing Dijkstra results is thorough.
Also applies to: 31-33
21-21:❓ Verification inconclusive
Validate the high test count.
The file plans for 654 tests if the version is at least 3.8.0, which is a very high number. Ensure this count accurately reflects the actual number of tests being run by the called functions.
🏁 Script executed:
#!/bin/bash # Verify how many tests are actually being run by checking the underlying functions # Check the implementation of compare_dijkstra_contraction cat pgtap/utilities/compare_dijkstra_contraction.sql || fd -t f "compare_dijkstra_contraction" --full-path --exec cat {} \; # Check the implementation of check_contracted_graph_linear cat pgtap/utilities/check_contracted_graph_linear.sql || fd -t f "check_contracted_graph_linear" --full-path --exec cat {} \;Length of output: 416
Action: Validate High Test Count Manually
The review comment raises a concern that the test plan for 654 tests (for PostgreSQL versions ≥3.8.0) might be excessively high. However, our automated scripts were unable to locate the underlying implementations for
compare_dijkstra_contractionandcheck_contracted_graph_linear. This prevents us from confirming whether the high test count accurately reflects the number of tests executed by these functions.
- Manual verification is needed: Please check the implementation of the functions that contribute to the test count.
- Verify that the executed test cases indeed sum up to the planned 654 tests when
min_version('3.8.0')is true.doc/contraction/pgr_contraction.rst (1)
79-79: Reference update looks good!Great update of the reference from
:doc:contraction-familyto `:doc:`pgr_contractionLinear. This makes the documentation more specific by pointing directly to the dedicated documentation for the linear contraction function.pgtap/contraction/contractionLinear/inner_query.pg (2)
21-79: Test implementation looks good!The test file thoroughly checks various aspects of the
pgr_contractionLinearfunction, including:
- Version compatibility checks
- Style variations
- Error handling for incorrect parameter types
- Support for different integer array types
This comprehensive testing approach helps ensure the new function works correctly across various scenarios.
1-18: 🧹 Nitpick (assertive)Update copyright year
The copyright year is set to 2025, which is in the future. Update it to the current year or the year when the code was written.
-Copyright (c) 2025 pgRouting developers +Copyright (c) 2024 pgRouting developersLikely an incorrect or invalid review comment.
pgtap/contraction/contractionLinear/no_crash_test.pg (1)
21-59: No-crash test implementation looks good!The implementation for testing that the
pgr_contractionLinearfunction doesn't crash under various conditions is well-structured. It appropriately:
- Checks for version compatibility
- Sets up test parameters
- Uses the
no_crash_testfunction with proper parametersThis will help ensure the new function handles edge cases gracefully.
pgtap/contraction/contractionLinear/edge_cases/edge_cases.pg (2)
44-58: Edge cases testing looks goodThe edge cases testing approach is thorough, testing the function with both
trueandfalseboolean parameters across standard and sample data tests.
35-35:✅ Verification successful
Verify external procedure dependency
The code calls
create_linear_edge_cases(), which is not defined in this file. Ensure this procedure is properly defined elsewhere in the codebase and is accessible when this test runs.
🏁 Script executed:
#!/bin/bash # Check if the create_linear_edge_cases procedure exists in the codebase echo "Searching for create_linear_edge_cases procedure..." rg -A 3 -B 3 "CREATE OR REPLACE PROCEDURE create_linear_edge_cases" --type sqlLength of output: 564
External procedure dependency verified
The procedure
create_linear_edge_cases()has been located in the repository (defined intools/testers/contraction_tapfuncs.sql). No further action is needed regarding this dependency.docqueries/contraction/contractionLinear.pg (7)
1-42: LGTM: Comprehensive examples for the new function.The examples demonstrate different use cases of the
pgr_contractionLinearfunction, including both directed and undirected graphs, and various input formats.
46-49: LGTM: Proper schema modifications for contraction.Adding these columns to the existing tables is necessary for tracking contraction status and storing additional information about contracted vertices.
51-54: LGTM: Good approach to store contraction results.Storing results in a separate table enables further operations on the contracted graph.
56-58: LGTM: Effective update of contracted vertices.Using unnest() on the array of contracted vertices is an efficient way to mark all contracted vertices.
63-65: LGTM: Correctly adding new edges to the graph.This ensures that the contracted edges are properly inserted with the necessary metadata.
67-76: LGTM: Well-structured view definition.The view effectively filters out contracted vertices, creating a clean representation of the contracted graph.
83-92: LGTM: Good demonstration of path calculation on contracted graph.This example shows how to use Dijkstra's algorithm on the contracted graph, which is a key use case for graph contraction.
doc/contraction/pgr_contractionLinear.rst (12)
11-28: LGTM: Clear function introduction and availability information.The documentation properly introduces the function with appropriate indexing and clearly states its availability and version information.
42-48: LGTM: Clear function signature definition.The signature is well-defined, showing both the required and optional parameters.
62-91: LGTM: Excellent visual representation using graphviz.The graphviz diagram effectively illustrates the contraction process with clear node and edge styling to highlight the contracted elements.
96-113: LGTM: Well-structured parameter documentation.The parameters are clearly documented with proper types and descriptions.
156-189: LGTM: Comprehensive result column documentation.The result columns are well-documented with clear descriptions of their meanings and content.
196-401: LGTM: Excellent detailed examples with visualizations.The examples for linear edges in both directed and undirected graphs are well-explained with helpful visualizations. The section on linearity being symmetrical or not is particularly useful.
403-469: LGTM: Clear step-by-step explanation.The step-by-step explanation of linear contraction with visual aids significantly helps users understand the contraction process.
474-587: LGTM: Practical guide for creating and using contracted graphs.The section provides a comprehensive and practical guide for users to implement graph contraction in their applications.
589-622: LGTM: Important edge case handling.Addressing the case where departure/destination vertices are not in the contracted graph is essential for practical applications.
149-151:❓ Verification inconclusive
Verify the included content from pgRouting-concepts.rst.
Ensure the referenced section exists in pgRouting-concepts.rst.
🏁 Script executed:
#!/bin/bash # Check if the referenced section exists in pgRouting-concepts.rst grep -A 5 "basic_edges_sql_start" doc/pgRouting-concepts.rst || find . -name "pgRouting-concepts.rst" -exec grep -A 5 "basic_edges_sql_start" {} \;Length of output: 303
Verification Passed: Included Section Exists in
pgRouting-concepts.rstThe referenced start marker (
basic_edges_sql_start) exists in the file along with subsequent content that suggests the intended section is present. Please also verify that the end marker (basic_edges_sql_end) is defined appropriately in the same file to ensure the included block captures the complete intended content.
117-119:❓ Verification inconclusive
Verify the included content from dijkstra-family.rst.
Ensure the referenced section in dijkstra-family.rst exists and contains the appropriate content.
🏁 Script executed:
#!/bin/bash # Check if the referenced section exists in dijkstra-family.rst grep -A 5 "dijkstra_optionals_start" doc/dijkstra-family.rst || find . -name "dijkstra-family.rst" -exec grep -A 5 "dijkstra_optionals_start" {} \;Length of output: 297
Action Required: Confirm Section Markers and File Reference
I verified using the shell script that the content starting with the “dijkstra_optionals_start” marker is present in the located dijkstra-family.rst file—it now includes the expected table (list-table block). However, note that the file wasn’t found at the originally referenced path (doc/dijkstra-family.rst), and the verification output didn’t show the “dijkstra_optionals_end” marker. Please double-check that the include directive’s relative file path is correct and that the end marker exists to clearly delimit the desired content.
33-35:✅ Verification successful
Verify the included content from pgr_contraction.rst.
Make sure the referenced section in pgr_contraction.rst exists and contains the appropriate content about characteristics.
🏁 Script executed:
#!/bin/bash # Check if the referenced section exists in pgr_contraction.rst grep -A 5 "characteristics_start" doc/contraction/pgr_contraction.rstLength of output: 321
I'll now run an additional script to check that the corresponding end marker exists in the file.
🏁 Script executed:
#!/bin/bash grep -n "characteristics_end" doc/contraction/pgr_contraction.rstLength of output: 91
Verified: The referenced section in pgr_contraction.rst exists and contains the correct content.
- Confirmed that the section begins at
.. characteristics_start, containing the expected contraction details.- Verified the corresponding end marker
.. characteristics_endis present.docqueries/contraction/contractionLinear.result (5)
1-26: LGTM: Results match expected behavior for directed graph example.The results correctly show that no contraction is performed when linearity is not symmetrical in a directed graph.
27-74: LGTM: Results demonstrate correct contraction behavior for different cases.The function correctly contracts vertices in undirected graphs and in directed graphs where linearity is symmetrical, and also handles multiple contractions in sequence.
79-112: LGTM: Schema modifications apply correctly.The ALTER TABLE commands execute successfully, and the contraction results are properly stored and used to update vertex status.
113-144: LGTM: Contracted graph view works as expected.The view correctly filters out contracted vertices and shows the resulting graph structure with both original and new edges.
145-173: LGTM: Path finding on contracted graph works correctly.The examples demonstrate that Dijkstra's algorithm works correctly on the contracted graph, both when all vertices are in the contracted graph and when handling vertices that were contracted.
cvvergara
force-pushed
the
issue-2791-new-function-pgr_contractionLinear
branch
from
100f596 to
44b4bf9
Compare
Fixes #2791 .
Changes proposed in this pull request:
@pgRouting/admins
Summary by CodeRabbit
New Features
pgr_contractionLinear, to support linear contraction processes on both directed and undirected graphs.Documentation
pgr_contractionLinearfunction and its parameters.Tests