-
-
Notifications
You must be signed in to change notification settings - Fork 373
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.
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
Remove partial option on pgr find close edges #2776
base: main
Are you sure you want to change the base?
Remove partial option on pgr find close edges #2776
Conversation
- New SQL without partial parameter - Deprecation of SQL with partial parameter - Update script adjustment - Documentation cleanup - Using the function on the sample data and concepts - Remove examples with partial parameter - pgTap tests - (tools) New function to pgtap function types - Added tests for the deprecated signature as is still usable - Updated release notes and NEWS
WalkthroughThis update removes the deprecated Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant pgr_findCloseEdges
Client->>pgr_findCloseEdges: Call function with (SQL, geometry[] array, tolerance, options)
pgr_findCloseEdges->>pgr_findCloseEdges: Validate tolerance and cap value
alt Invalid parameter
pgr_findCloseEdges-->>Client: Raise exception "Invalid value for 'tolerance'"
else Valid parameters
pgr_findCloseEdges->>pgr_findCloseEdges: Process geometry array, build query
pgr_findCloseEdges-->>Client: Return complete result set (all columns calculated)
end
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🪧 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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (20)
NEWS.md
(1 hunks)doc/src/pgRouting-concepts.rst
(2 hunks)doc/src/sampledata.rst
(9 hunks)doc/utilities/pgr_findCloseEdges.rst
(7 hunks)docqueries/src/concepts.pg
(3 hunks)docqueries/src/concepts.result
(3 hunks)docqueries/src/sampledata.result
(1 hunks)docqueries/src/withPoints-category.result
(1 hunks)docqueries/utilities/findCloseEdges.pg
(1 hunks)docqueries/utilities/findCloseEdges.result
(2 hunks)locale/en/LC_MESSAGES/pgrouting_doc_strings.po
(19 hunks)locale/pot/pgrouting_doc_strings.pot
(19 hunks)pgtap/topology/degree/edge_cases.pg
(1 hunks)pgtap/utilities/findCloseEdges/edge_cases.pg
(4 hunks)pgtap/utilities/findCloseEdges/types_check.pg
(1 hunks)sql/scripts/build-extension-update-files.pl
(2 hunks)sql/sigs/pgrouting--3.8.sig
(1 hunks)sql/utilities/findCloseEdges.sql
(7 hunks)tools/testers/general_pgtap_tests.sql
(1 hunks)tools/testers/sampledata.sql
(1 hunks)
⏰ 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 (84)
- GitHub Check: Ubuntu Boost (83)
- GitHub Check: Ubuntu Boost (80)
- GitHub Check: Ubuntu Boost (79)
- GitHub Check: Ubuntu Boost (78)
- GitHub Check: Ubuntu Boost (77)
- GitHub Check: Ubuntu Boost (76)
- GitHub Check: Ubuntu Boost (75)
- GitHub Check: macos (macos-14)
- GitHub Check: Ubuntu Boost (68)
- GitHub Check: Ubuntu clang
- GitHub Check: macos (macos-latest)
- GitHub Check: Check queries
- GitHub Check: Ubuntu Boost (56)
🔇 Additional comments (91)
locale/en/LC_MESSAGES/pgrouting_doc_strings.po (12)
11-11
: Timestamp update detected.The POT-Creation-Date has been updated to 2025-03-02, which is expected when regenerating the translation files.
4430-4432
: Documentation correctly notes the removal of the 'partial' option.This entry clearly states that the
partial
option has been removed frompgr_findCloseEdges
, which aligns with the PR objectives.
11607-11607
: Options list correctly updated to remove 'partial'.The options list now only includes
cap
anddryrun
, reflecting the removal of thepartial
parameter as intended in this PR.
11616-11620
: Documentation examples improved.The example documentation has been enhanced to clearly illustrate the use of the
cap
parameter, providing better guidance for users.
11714-11733
: Dry run execution documentation added.Excellent addition of documentation explaining the dry run execution behavior, including that it returns an
EMPTY SET
and generates a PostgreSQLNOTICE
with the code that would be used. This helps users understand how to use this feature for customization.
11796-11801
: Return value descriptions improved.The descriptions of return values are now more precise, clearly explaining what each field represents in the results.
11808-11820
: Enhanced result field documentation.The documentation now provides more detailed explanations of the geometry-related fields returned by the function, making it easier for users to understand the spatial relationships.
11822-11837
: New section for points of interest setup.This new section explaining how to set up points of interest provides valuable context for users working with the function examples.
17674-17675
: Documentation readability improvement.The wording has been refined to better explain the sample network used in the documentation examples.
17704-17706
: Clarified reverse prefix explanation.The text now more clearly explains the meaning of the
reverse_
prefix in relation to segment directions.
17737-17738
: Improved coordinate calculation documentation.Better explanation of how coordinate values can be calculated using PostGIS functions, providing useful reference for users.
Also applies to: 17745-17746
17756-17757
: General documentation formatting and clarity improvements.Several minor wording improvements that enhance readability and understanding of the documentation.
Also applies to: 17767-17768, 17836-17837
locale/pot/pgrouting_doc_strings.pot (14)
11-11
: Documentation timestamp has been updated.The POT file creation date has been updated to reflect the recent changes.
3952-3953
: Clear declaration of partial option removal.This concise statement explicitly documents the removal of the
partial
option frompgr_findCloseEdges
, which aligns perfectly with the PR objective to remove this parameter.
9880-9880
: Updated options listing correctly removes partial parameter.The options list has been correctly updated to only include
cap
anddryrun
parameters, removing thepartial
parameter as intended by this PR. This change ensures documentation consistency with the updated function signature.
9889-9893
: Improved example clarity.The updated examples provide more specific context about finding close edges to points of interest, with clear indication of the
cap
parameter usage. This helps users better understand the function behavior.
9937-9938
: Enhanced return value documentation.The descriptions of return values have been significantly improved with clearer explanations of:
- The relative position calculation
- Direction indicators (right/left)
- Distance calculation
- Geometry representations
These improvements make the function's behavior and output much more understandable for users.
Also applies to: 9943-9944, 9949-9962
9964-9978
: Added visual explanation for single point scenario.The addition of clear visual explanations for the single point scenario helps users understand how the geometry values are calculated and represented in the output, which is especially helpful given the removal of the partial option (as all columns are now always returned).
9982-9995
: Improved dryrun execution documentation.The enhanced documentation for the dryrun execution mode properly explains that:
- It returns an EMPTY SET
- It generates a PostgreSQL NOTICE with the code used
- The generated code can be used as a starting point for customizations
This is particularly valuable now that partial has been removed and users may need to customize their code more often.
9997-10016
: Added multi-point scenario documentation.The addition of clear explanations for the multi-point scenario and its dryrun execution helps users understand how the function behaves with multiple input points, which is especially important now that all columns are always calculated (due to partial option removal).
10069-10082
: Added documentation for points of interest setup.The new documentation sections covering how to set up and manage points of interest provide valuable context for users working with the
pgr_findCloseEdges
function, making it easier to implement the examples and understand real-world usage.
14797-14810
: Improved sample data documentation.The enhanced explanation of the sample data structure and the city example makes it easier for users to understand and work with the documentation examples, providing better context for practical applications.
14818-14819
: Clarified database design for documentation.The explanation about how the documentation's database design keeps two segments in the same row (one in each direction) helps users understand the
reverse_
prefix convention and why certain columns exist in the sample data.
14848-14849
: Added SQL equivalents for coordinate calculations.The inclusion of SQL equivalents (
ST_X(ST_StartPoint(geom))
andST_Y(ST_EndPoint(geom))
) for the coordinate columns helps users understand how these values can be calculated dynamically if not stored in the table.Also applies to: 14854-14855
14863-14873
: Added indexing recommendations.The addition of clear indexing recommendations for the sample data tables, including specific columns that should be indexed and the reasons why, provides valuable performance guidance for users.
14926-14927
: Improved context for combination storage.The explanation of how and why certain source-target combinations are stored in a table for documentation convenience helps users understand the example structure better.
NEWS.md (1)
23-23
: Documentation update for pgr_findCloseEdges function.The NEWS.md file correctly documents the removal of the
partial
option from thepgr_findCloseEdges
function in the upcoming 3.8.0 release notes.docqueries/src/withPoints-category.result (1)
11-12
: Row ordering has changed in the output.The order of rows with pid values 2 and 3 has been swapped. This is likely a side effect of removing the
partial
option from thepgr_findCloseEdges
function, which might have affected result ordering. Ensure this change is expected and doesn't affect any dependent code that relies on a specific result order.doc/src/sampledata.rst (7)
18-19
: Spelling and grammar improvements.Corrected "mayority" to "majority" and "bellow" to "below".
34-35
: Spelling correction.Fixed "cpacity" to "capacity".
43-45
: Spelling correction.Fixed "oposite" to "opposite" in multiple instances.
75-76
: Spelling correction.Fixed "convinience" to "convenience".
Also applies to: 80-81
99-100
: Spelling correction.Fixed "recomendation" to "recommendation".
207-214
: Documentation for new columns added.New column descriptions for
distance
,edge
, andnewPoint
have been added to the Points of Interest table. These appear to be related to the removal of thepartial
option and provide better information about the meaning of these columns.
219-246
: Improved documentation for Points of Interest.The documentation has been restructured to better explain the process of filling up the points of interest table, with clearer section titles and explanations. This improves the usability of the documentation.
tools/testers/general_pgtap_tests.sql (1)
53-67
: New utility function to check function argument types.Added a new
function_types
function that checks if a function's parameter types match the expected types. This will be helpful for testing that the updatedpgr_findCloseEdges
function signatures (after removing thepartial
option) are correctly registered in the PostgreSQL catalog.The implementation uses a well-structured Common Table Expression (CTE) to query the PostgreSQL system catalog and generates appropriate test output.
sql/sigs/pgrouting--3.8.sig (1)
163-164
: New function signatures for pgr_findCloseEdges.Added new signatures for
pgr_findcloseedges
that don't include thepartial
parameter:pgr_findcloseedges(text, geometry, double precision, integer, boolean) pgr_findcloseedges(text, geometry[], double precision, integer, boolean)
These signatures properly implement the PR objective to remove the
partial
option while maintaining backward compatibility by keeping the old signatures (lines 165-166).pgtap/utilities/findCloseEdges/types_check.pg (1)
45-83
:❓ Verification inconclusive
Verify the version-conditional logic handles all cases correctly.
The code introduces a version-conditional structure to handle different function signatures based on PostgreSQL version. For 3.8.0+, it tests both old (with
partial
) and new (withoutpartial
) signatures, while older versions only check the legacy signatures.Make sure this approach properly covers edge cases during version transitions.
🏁 Script executed:
#!/bin/bash # Check if there are any existing calls to pgr_findCloseEdges with the partial parameter # that might be broken by this change # Look for occurrences of pgr_findCloseEdges with partial parameter rg -A 2 -B 2 "pgr_findCloseEdges.*partial" --type sqlLength of output: 55
Action: Verify Version-Conditional Logic for Function Signatures
The version-conditioned structure in
pgtap/utilities/findCloseEdges/types_check.pg
(lines 45–83) appears to differentiate between PostgreSQL versions correctly, handling both the legacy (withpartial
) and the newer signatures appropriately. Our automated search for SQL calls referencingpgr_findCloseEdges
with thepartial
parameter did not return any results, suggesting that no use cases inadvertently rely on the legacy behavior.Please ensure the following:
- Confirm that for PostgreSQL 3.8.0+, both old (with
partial
) and new signatures are properly exercised.- Verify manually that, in operational environments, no queries or application logic breaks due to the omission of the
partial
parameter in the newer branch.- Double-check that all signature arrays in both branches match the expected function signatures and that no edge cases during version transitions have been missed.
docqueries/src/concepts.pg (4)
183-183
: Parameter removal aligns with PR objectives.The
partial => false
parameter has been removed from thepgr_findCloseEdges
function call, which aligns with the PR's objective to remove this option.
193-193
: Parameter removal aligns with PR objectives.The
partial => false
parameter has been removed from thepgr_findCloseEdges
function call, which aligns with the PR's objective to remove this option.
245-250
: Improved performance example with spatial filtering.This is a good example showing how to filter edges using spatial operators before routing calculations, which can significantly improve performance.
251-258
: Added practical performance example with pgr_dijkstra.This new example demonstrates how to combine spatial filtering with routing functions, which is a valuable addition to the documentation's performance tips section.
sql/scripts/build-extension-update-files.pl (1)
272-276
: Ensure version-specific upgrade path for deprecated functions.This code correctly handles the removal of the deprecated
pgr_findcloseedges
functions with thepartial
parameter when upgrading from version 3.7 to 3.8+. It's properly placed in the version-specific upgrade section.doc/src/pgRouting-concepts.rst (3)
1899-1901
: Good documentation addition for performance tips.This new subsection provides a practical example of defining an area of interest, which is a useful performance optimization technique for spatial routing.
1905-1909
: Enhanced documentation with practical routing example.This addition complements the previous example by showing how to use the filtered area in an actual routing query, making the performance tips section more actionable.
1928-1928
: Fixed spelling in section title.Corrected "Functionaity" to "Functionality" in the section title.
doc/utilities/pgr_findCloseEdges.rst (8)
25-26
: Documentation update for removed optionThe documentation correctly notes that the
partial
option has been removed in version 3.8.0, which aligns with the PR objectives.
41-42
: Fixed typographical errorThe text now correctly refers to "EMPTY SET" instead of "EMTPY SET".
54-56
: Simplified options parameterThe options parameter has been simplified by removing
partial
, leaving onlycap
anddryrun
.
69-71
: Consistent signature updateThe function signature has been updated to remove the
partial
option, consistent with the changes elsewhere in the documentation.
92-94
: Consistent signature update for many points overloadThe function signature for the "Many points" overload has also been updated to remove the
partial
option.
193-194
: Improved parameter description clarityThe description of the
fraction
parameter has been improved by using mathematical notation<0,1>
to indicate the valid range.
199-202
: Improved side parameter descriptionThe description of the
side
parameter has been improved with better formatting and clearer explanation.
207-212
: Clarified returned geometry descriptionsThe descriptions of the
geom
andedge
return fields have been clarified to better explain what these geometries represent.docqueries/src/concepts.result (4)
311-312
: Removed partial parameter from function callThe
partial => false
parameter has been removed from thepgr_findCloseEdges
function call, which aligns with the PR objective of removing this option.
324-327
: Removed partial parameter from second function callThe
partial => false
parameter has been removed from the second instance of thepgr_findCloseEdges
function call as well, maintaining consistency.
417-435
: Added spatial query example for performanceA new example query has been added that demonstrates how to select edges that intersect with a buffered geometry. This provides a useful performance optimization technique for pgRouting users.
436-448
: Updated query to use spatial filteringThe existing
pgr_dijkstra
example has been updated to use the new spatial filtering approach, demonstrating how to efficiently restrict the graph for routing.tools/testers/sampledata.sql (5)
96-99
: Enhanced pointsOfInterest table with additional columnsThree new columns have been added to the
pointsOfInterest
table to store additional spatial information:distance
,edge
, andnewPoint
. This allows capturing more details from thepgr_findCloseEdges
function.
101-108
: Simplified data insertion approachThe INSERT statement has been simplified to only populate the
geom
column, with other columns to be filled later using an UPDATE statement. This approach allows for a cleaner separation between initial data creation and enrichment.
109-121
: Updated approach to populate point data using pgr_findCloseEdgesAn UPDATE statement has been added that uses
pgr_findCloseEdges
to populate all the additional columns in the pointsOfInterest table. This approach better demonstrates how to use the function in a real-world scenario.
122-123
: Manual override for test scenarioA specific record (pid = 6) has its
side
value set to 'b', likely to test a special case. This is useful for testing edge cases in the application.
124-129
: Added formatted data retrievalA SELECT statement has been added to retrieve and format the data from the
pointsOfInterest
table, which provides a clear view of how the data looks after processing.docqueries/src/sampledata.result (5)
165-171
: Insertion logic looks solid
These insert statements now only populate thegeom
column inpointsOfInterest
, which aligns with the updated approach of populating additional fields later. The changes appear consistent with the revised schema.
174-186
: Ensure usage of updated function signature
ThisUPDATE
statement correctly callspgr_findCloseEdges(...)
without thepartial
parameter. Consider verifying that all expected rows and columns (especiallyfraction
anddistance
) are adequately handled for boundary conditions (e.g., fraction > 1).
188-189
: Confirm validity of 'b' side value
Settingside = 'b'
might require documentation or constraints confirming that'b'
is a valid state for this column. If'b'
stands for "both" or a similar concept, ensure that downstream logic accounts for it.
191-204
: Result set includes newly introduced columns
Selecting the columns that includedistance
,edge
, andnewPoint
accurately showcases the outcomes of the updatedUPDATE
process. Ensure tests cover scenarios where these columns could remain unset or produce unexpected values.
206-206
: No concerns on the added comment
The newly introduced comment line serves as a readable placeholder for further logic.pgtap/utilities/findCloseEdges/edge_cases.pg (6)
23-23
: Expanded test plan coverage
The updated plan count (33 or 34 tests) significantly increases coverage. Ensure that each additional test scenario is relevant and maintains test stability across different minor versions of pgRouting.
61-61
: Improved clarity in dryrun test
Including"dryrun results empty"
provides more descriptive feedback in test outputs, making debugging easier.
100-104
: Skipping partial-flag tests on version >= 4.0.0
Conditionally skipping tests aligns with the removal of the partial flag in newer versions. This logic ensures backward compatibility while respecting changes in pgRouting 4.0.0 and beyond.
105-123
: Retaining partial flag tests for older versions
These tests confirm backward compatibility for versions below 4.0.0 where the partial flag still exists. Good approach for legacy support.
124-151
: Testing partial usage with version checks
This block tests multiple partial-flag scenarios with geometry arrays and single geometry. It ensures thorough coverage.
152-176
: Comprehensive legacy partial use checks
More detailed partial-flag checks for older versions help ensure the function continues to work as intended prior to 4.0.0. Thecollect_tap
approach cleanly aggregates multiple test assertions.docqueries/utilities/findCloseEdges.pg (6)
5-7
: Explicit field selection
Listing the columns (edge_id, fraction, side, distance, ...
) instead of usingSELECT *
clarifies the returned data and helps maintain forward compatibility if additional columns are introduced in the future.
11-11
: Cap parameter usage
Includingcap => 2
indicates a tighter control over the number of returned edges. Verify this value meets user expectations and doesn't inadvertently omit needed edges.
13-16
: Comprehensive result fields
The enhancements to retrieveside
,distance
, geometry points, and edges provide a more detailed view of each match. This is helpful for debugging and analyses.
23-28
: Valid single-geometry test
Generating a single geometry line and checking it withpgr_findCloseEdges
is a solid test scenario. This approach standardizes queries while removing the partial parameter.
49-53
: Multiple geometry test coverage
Using an array of points effectively validates that the function processes multiple inputs in a single call, aligning with the removal of partial-related logic.
55-58
: Dry-run checks on multiple geometries
Combining multi-geometry inputs withdryrun => true
ensures consistent handling of no-op or minimal results, providing more robust test coverage.docqueries/utilities/findCloseEdges.result (5)
15-18
: Output contains duplicatedistance
columns.The result set shows two identical
distance
columns, which is confusing for users reading the output.This is likely caused by the duplicate field in the SELECT statement above.
42-47
: Code organization and readability improvement.The
/* -- o0 */
comment provides better organization for test cases. The explicit column selection instead ofSELECT *
improves clarity for understanding the query results.
56-61
: Backward compatibility preserved.The function call correctly preserves the ability to use the new
cap
parameter which replaces the removedpartial
option.
80-83
: Consistent code style with explicit column naming.The SELECT statement now explicitly lists columns instead of using wildcards, which is a good practice for clarity and to prevent issues if column order changes in the future.
115-123
: Accurate geometry point creation for testing.The new function call demonstrates proper usage with multiple points, correctly showing how the function handles arrays of geometries rather than single geometries.
sql/utilities/findCloseEdges.sql (6)
24-31
: Simplified function signature with removed 'partial' parameter.The function signature has been updated to remove the
partial
parameter as intended in the PR objectives. The function now accepts an array of geometries directly, improving usability.
41-44
: Clear parameter handling with direct assignment.Variables are now directly assigned from function parameters, which improves readability and reduces potential errors in parameter handling.
55-61
: Enhanced input validation.Added proper validation for both tolerance and cap parameters with clear error messages, which improves function robustness against invalid inputs.
130-161
: Well-designed backward compatibility function.This single-point signature function correctly redirects to the new array-based implementation, maintaining backward compatibility while utilizing the new code path.
163-187
: Clear deprecation marking for removed functionality.The deprecated functions (with the
partial
parameter) are properly marked with version information and redirect to the new implementation. This allows for a smooth transition for users of the older API.
205-207
: Simplified implementation through redirection.The deprecated function implementation efficiently redirects to the new function without duplicating code, which is a good practice for maintainability.
Fixes #2775 .
pgr_findCloseEdges: Remove partial option
pgtap/degree: add todo on edge_cases test
@pgRouting/admins
Summary by CodeRabbit
Functionality Updates
Documentation
Data Enhancements
Testing & Upgrade