Fix #3010: Optimize pgr_separateTouching performance#3081
Fix #3010: Optimize pgr_separateTouching performance#3081Mohit242-bit wants to merge 4 commits intopgRouting:developfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Optimizes pgr_separateTouching by reducing redundant PostGIS function calls in hot-path CTEs, improving performance on large edge sets while keeping results equivalent.
Changes:
- Precomputes
ST_StartPoint(geom)/ST_EndPoint(geom)once per edge inedges_table - Replaces repeated endpoint function calls with column comparisons in downstream CTEs
- Updates the dry-run expected output to match the new query shape
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| sql/utilities/separateTouching.sql | Precomputes endpoints in a CTE and reuses them to avoid repeated PostGIS calls |
| docqueries/utilities/separateTouching.result | Updates expected dry-run output to reflect the optimized query |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (4)
WalkthroughPrecompute and expose edge start/end points (startpoint, endpoint) in the edges table; propagate them as sp1/ep1 in get_touching and replace repeated ST_StartPoint/ST_EndPoint calls with direct column comparisons in touching-filter and intersection logic. Changes
Sequence Diagram(s)(omitted — changes are localized SQL optimization without multi-component sequential interactions needing visualization) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![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
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@sql/utilities/separateTouching.sql`:
- Around line 78-83: The query is recomputing ST_Intersection(geom, g2) multiple
times; compute it once in the SELECT (e.g., add ST_Intersection(geom, g2) AS
intersection or point) and then reference that alias in the WHERE predicate
instead of calling ST_Intersection again. Update the SELECT (in the get_touching
usage) to produce the computed column (intersection/point) and change the WHERE
clause conditions that compare to sp1/ep1 and the later OR branch to use that
alias (and keep id1, g1, g2 as before).
cvvergara
left a comment
cvvergara
left a comment
There was a problem hiding this comment.
@Mohit242-bit
Can you update the release notes and news please
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@NEWS.md`:
- Line 37: The NEWS.md entry was edited directly; instead add the release note
for pgr_separateTouching() / issue `#3010` into the generator's source (the
file(s) consumed by tools/release-scripts/notes2news.pl), then run the
notes2news.pl generator to regenerate NEWS.md so the change is preserved;
reference the pgr_separateTouching() / `#3010` text when adding the new source
entry so the generated line matches the intended release note.
ℹ️ Review info
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
NEWS.mddoc/src/release_notes.rstlocale/en/LC_MESSAGES/pgrouting_doc_strings.polocale/pot/pgrouting_doc_strings.pot
Mohit242-bit
force-pushed
the
fix/separateTouching-performance-3010
branch
from
391b844 to
f3fe1e8
Compare
|
I have updated the news and release notes files. I would appreciate a review when you have time. |
Fixes #3010 .
Problem
pgr_separateTouchingcallsST_StartPoint()/ST_EndPoint()inside theWHEREclause of a cross-join, resulting in up to 8 redundant PostGIS function calls per edge pair. On large networks this causes severe performance degradation:pgr_separateTouching()#3010, with a reference workaround in rustprooflabs/pgosm-flex#413.Changes proposed in this pull request:
ST_StartPoint(geom)andST_EndPoint(geom)once per edge as columns in theedges_tableCTEget_touchingCTEtouchingsCTEdryrunoutputPerformance impact
The optimization is mathematically equivalent (same input → same output) but eliminates millions of redundant function calls on large datasets.
Reported speedup (by issue reporter using this approach):
Testing
-> Manual SQL verification: returns identical results (edge 14 split into 2 segments)
->
dryrun => trueconfirms optimized query structure-> All 8 pgTAP tests pass (26 assertions):
edge_cases,types_check,issue_280,issue_623,issue_1009,issue_1074,issue_1336,issue_1882@pgRouting/admins
Summary by CodeRabbit
pgr_separateTouching()#3010).