Astar bdastar using new process and driver for astar like functions #3066
Conversation
WalkthroughRefactors Astar to a process/driver pattern: adds Changes
Sequence Diagram(s)sequenceDiagram
participant SQL as Client SQL (_pgr_astar / _pgr_bdAstar)
participant Proc as pgr_process_astar (C extern "C")
participant SPI as PostgreSQL SPI
participant Driver as pgrouting::drivers::do_astar (C++)
participant Algo as A*/BD-A* algorithm
SQL->>Proc: invoke pgr_process_astar(edges_sql, combinations_sql, starts, ends, flags...)
Proc->>SPI: SPI_connect() / run combination SQL (if provided)
Proc->>Driver: do_astar(..., &return_tuples, &return_count, log, notice, err)
Driver->>Algo: select & run A* or BD-A* based on Which/directedness
Algo-->>Driver: return path results
Driver-->>Proc: populate return_tuples/return_count and message streams
Proc->>SPI: SPI_finish()
Proc-->>SQL: return SRF tuples (or propagate errors/notices)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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: 7
🤖 Fix all issues with AI agents
In `@doc/src/release_notes.rst`:
- Line 47: Fix the typographical errors in the release note entry for
:issue:`3064`: replace "create an use a process & driverAstar" with "create and
use a process & driver Astar" so "an" -> "and" and insert a space between
"driver" and "Astar" (preserve the surrounding punctuation and capitalization
for the Astar reference).
In `@include/c_common/enums.h`:
- Around line 39-42: Fix the comment spacing next to the enum declarations by
adding a space before the closing comment delimiter so the inline comment reads
"with edges that have x y */" properly; update the comment adjacent to the ASTAR
and BDASTAR enum entries (ASTAR, BDASTAR) to include the missing space before
"*/".
In `@src/astar/astar_driver.cpp`:
- Around line 32-36: The file is missing the <utility> header required for
std::pair used in the catch block (e.g., the pair referenced around the catch
using std::pair at line ~167); add `#include` <utility> to the top include list in
astar_driver.cpp (alongside <vector>, <string>, etc.) so the build/ci stops
failing due to the missing declaration.
In `@src/astar/astar_process.cpp`:
- Around line 94-98: The outer condition already ensures (*result_tuples) is
non-null, so remove the redundant inner check and always free the buffer when
entering that branch: inside the existing if (!err.str().empty() &&
(*result_tuples)) branch call pfree(*result_tuples) directly, then set
(*result_tuples) = nullptr and (*result_count) = 0; reference the variables err,
result_tuples, result_count and the pfree call in astar_process.cpp when making
this simplification.
- Around line 36-37: The C++ headers <string> and <sstream> are currently placed
after the extern "C" block of project headers which violates the project's
include order; fix by moving the C++ system headers (<string>, <sstream>) so
they appear after the matching header and any C system headers but before the
project headers inside/after the extern "C" block in astar_process.cpp; ensure
the extern "C" block contains only C or project headers and that <string> and
<sstream> are outside/above that block to satisfy cpplint ordering.
In `@src/cpp_common/combinations.cpp`:
- Around line 161-162: Replace the asymmetric ternary null-checks for starts and
ends with the same symmetric guard used earlier: check both startsArr and
endsArr together before assigning; e.g., if (startsArr && endsArr) { starts =
normal ? get_intSet(startsArr) : get_intSet(endsArr); ends = normal ?
get_intSet(endsArr) : get_intSet(startsArr); } else leave starts/ends as empty
std::set<int64_t>(); — this ensures safe use of get_intSet with
startsArr/endsArr (variables: startsArr, endsArr, normal, get_intSet, starts,
ends).
In `@src/cpp_common/utilities.cpp`:
- Around line 97-115: The BDASTAR branch in get_name builds suffix using
(is_matrix? "Matrix" : "") which can produce invalid "pgr_bdAstarMatrix" when
is_only_cost is false; change the BDASTAR suffix construction to mirror ASTAR
and DIJKSTRA (only append "Matrix" when is_only_cost && is_matrix). Update the
BDASTAR case in get_name to use std::string(is_only_cost? "Cost" : "") +
(is_only_cost && is_matrix? "Matrix" : ""), and apply the same guard to the
BDDIJKSTRA/BD* overloads that currently use is_matrix alone so all mirrored
pairs use is_only_cost && is_matrix for the "Matrix" suffix.
| * :issue:`3055`: bdDijkstra: use the shortest_path process and driver | ||
| * :issue:`3056`: edwardMoore: use the shortest_path process and driver | ||
| * :issue:`3060`: dagShortestPath: use the shortest_path process and driver | ||
| * :issue:`3064`: Astar: create an use a process & driverAstar |
There was a problem hiding this comment.
Typographical errors in the release note entry.
Two issues:
- "create an use" → "create and use"
- "driverAstar" → "driver Astar" (missing space)
Compare with the surrounding entries (lines 44–46) which use clean, readable phrasing.
Proposed fix
-* :issue:`3064`: Astar: create an use a process & driverAstar
+* :issue:`3064`: Astar: create and use a process & driver Astar📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| * :issue:`3064`: Astar: create an use a process & driverAstar | |
| * :issue:`3064`: Astar: create and use a process & driver Astar |
🤖 Prompt for AI Agents
In `@doc/src/release_notes.rst` at line 47, Fix the typographical errors in the
release note entry for :issue:`3064`: replace "create an use a process &
driverAstar" with "create and use a process & driver Astar" so "an" -> "and" and
insert a space between "driver" and "Astar" (preserve the surrounding
punctuation and capitalization for the Astar reference).
| BANDWIDTH, | ||
| /* with edges that have x y*/ | ||
| ASTAR= 400, BDASTAR | ||
| }; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
LGTM — Clean enum extension with well-separated value range.
Starting ASTAR at 400 provides clear separation from existing algorithm groups.
Nit: minor spacing in the comment — missing space before */.
- /* with edges that have x y*/
+ /* with edges that have x y */📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| BANDWIDTH, | |
| /* with edges that have x y*/ | |
| ASTAR= 400, BDASTAR | |
| }; | |
| BANDWIDTH, | |
| /* with edges that have x y */ | |
| ASTAR= 400, BDASTAR | |
| }; |
🤖 Prompt for AI Agents
In `@include/c_common/enums.h` around lines 39 - 42, Fix the comment spacing next
to the enum declarations by adding a space before the closing comment delimiter
so the inline comment reads "with edges that have x y */" properly; update the
comment adjacent to the ASTAR and BDASTAR enum entries (ASTAR, BDASTAR) to
include the missing space before "*/".
| #include <string> | ||
| #include <sstream> |
There was a problem hiding this comment.
Fix include order to resolve CI failure.
The cpplint pipeline is failing because <string> and <sstream> (C++ system headers) appear after the extern "C" block containing project headers. Per the project's cpplint configuration, the expected order is: matching header, C system headers, C++ system headers, then other project headers.
🔧 Proposed fix
`#include` "process/astar_process.h"
+#include <string>
+#include <sstream>
+
extern "C" {
`#include` "c_common/postgres_connection.h"
`#include` "c_common/time_msg.h"
}
-#include <string>
-#include <sstream>
-
`#include` "c_types/path_rt.h"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| #include <string> | |
| #include <sstream> | |
| `#include` "process/astar_process.h" | |
| `#include` <string> | |
| `#include` <sstream> | |
| extern "C" { | |
| `#include` "c_common/postgres_connection.h" | |
| `#include` "c_common/time_msg.h" | |
| } | |
| `#include` "c_types/path_rt.h" |
🧰 Tools
🪛 Cppcheck (2.19.0)
[information] 36-36: Include file
(missingIncludeSystem)
[information] 37-37: Include file
(missingIncludeSystem)
[information] 36-36: Include file
(missingIncludeSystem)
[information] 37-37: Include file
(missingIncludeSystem)
[information] 36-36: Include file
(missingIncludeSystem)
[information] 36-36: Include file
(missingIncludeSystem)
[information] 37-37: Include file
(missingIncludeSystem)
🪛 GitHub Actions: Check files
[error] 36-36: cpplint: Found C++ system header after other header. Should be: astar_process.h, c system, c++ system, other. [build/include_order]
[error] 37-37: cpplint: Found C++ system header after other header. Should be: astar_process.h, c system, c++ system, other. [build/include_order]
🤖 Prompt for AI Agents
In `@src/astar/astar_process.cpp` around lines 36 - 37, The C++ headers <string>
and <sstream> are currently placed after the extern "C" block of project headers
which violates the project's include order; fix by moving the C++ system headers
(<string>, <sstream>) so they appear after the matching header and any C system
headers but before the project headers inside/after the extern "C" block in
astar_process.cpp; ensure the extern "C" block contains only C or project
headers and that <string> and <sstream> are outside/above that block to satisfy
cpplint ordering.
| if (!err.str().empty() && (*result_tuples)) { | ||
| if (*result_tuples) pfree(*result_tuples); | ||
| (*result_tuples) = nullptr; | ||
| (*result_count) = 0; | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Redundant inner null check on *result_tuples.
The outer condition on line 94 already guarantees *result_tuples is non-null, making the inner if (*result_tuples) on line 95 unnecessary.
✂️ Simplify
if (!err.str().empty() && (*result_tuples)) {
- if (*result_tuples) pfree(*result_tuples);
+ pfree(*result_tuples);
(*result_tuples) = nullptr;
(*result_count) = 0;
}🤖 Prompt for AI Agents
In `@src/astar/astar_process.cpp` around lines 94 - 98, The outer condition
already ensures (*result_tuples) is non-null, so remove the redundant inner
check and always free the buffer when entering that branch: inside the existing
if (!err.str().empty() && (*result_tuples)) branch call pfree(*result_tuples)
directly, then set (*result_tuples) = nullptr and (*result_count) = 0; reference
the variables err, result_tuples, result_count and the pfree call in
astar_process.cpp when making this simplification.
| auto starts = startsArr? (normal? get_intSet(startsArr) : get_intSet(endsArr)) : std::set<int64_t>(); | ||
| auto ends = endsArr? (normal? get_intSet(endsArr) : get_intSet(startsArr)) : std::set<int64_t>(); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
🧩 Analysis chain
🏁 Script executed:
# First, let's see the actual code around lines 161-162 and understand the context
sed -n '155,180p' src/cpp_common/combinations.cppRepository: pgRouting/pgrouting
Length of output: 1165
🏁 Script executed:
# Also look at the other overload (lines 117-120) mentioned in the review
sed -n '110,130p' src/cpp_common/combinations.cppRepository: pgRouting/pgrouting
Length of output: 858
🏁 Script executed:
# Find all function overloads and their signatures
rg -n 'get_combinations' src/cpp_common/combinations.cpp | head -20Repository: pgRouting/pgrouting
Length of output: 461
🏁 Script executed:
# Find all callers of get_combinations function
rg -n 'get_combinations\s*\(' src/ --type=cpp -A2 -B2Repository: pgRouting/pgrouting
Length of output: 8623
🏁 Script executed:
# Look at the astar_driver.cpp caller context
sed -n '90,115p' src/astar/astar_driver.cppRepository: pgRouting/pgrouting
Length of output: 938
🏁 Script executed:
# Look at the shortestPath_driver.cpp caller context
sed -n '145,175p' src/dijkstra/shortestPath_driver.cppRepository: pgRouting/pgrouting
Length of output: 1100
🏁 Script executed:
# Check how starts and ends are initialized in these drivers
rg -n 'ArrayType.*starts|ArrayType.*ends' src/astar/astar_driver.cpp src/dijkstra/shortestPath_driver.cpp -B2 -A2Repository: pgRouting/pgrouting
Length of output: 810
🏁 Script executed:
# Look at where normal parameter comes from in these drivers
rg -n 'normal.*=' src/astar/astar_driver.cpp src/dijkstra/shortestPath_driver.cpp | head -20Repository: pgRouting/pgrouting
Length of output: 45
🏁 Script executed:
# Search for where normal is computed/assigned in astar and shortestPath drivers
rg -n 'normal\s*=' src/astar/astar_driver.cpp src/dijkstra/shortestPath_driver.cppRepository: pgRouting/pgrouting
Length of output: 45
🏁 Script executed:
# Look at full context around the get_combinations call in astar_driver.cpp
sed -n '45,110p' src/astar/astar_driver.cppRepository: pgRouting/pgrouting
Length of output: 1925
🏁 Script executed:
# Look at full context around the get_combinations call in shortestPath_driver.cpp
sed -n '110,180p' src/dijkstra/shortestPath_driver.cppRepository: pgRouting/pgrouting
Length of output: 2070
🏁 Script executed:
# Find the SQL wrapper functions that call these drivers
rg -n 'do_astar|do_shortestPath' src/ --type=cpp -B5 | head -60Repository: pgRouting/pgrouting
Length of output: 1344
🏁 Script executed:
# Look for the PostgreSQL C interface to see how normal parameter is set
find src/ -name "*.c" | xargs grep -l "do_astar\|do_shortestPath"Repository: pgRouting/pgrouting
Length of output: 47
🏁 Script executed:
# Search for astar and shortestPath in .c files (PostgreSQL functions)
rg -n 'pgr_astar|pgr_shortestPath' src/ --type=c -A3 | head -100Repository: pgRouting/pgrouting
Length of output: 434
🏁 Script executed:
# Look at the full astar.c to understand parameter handling
sed -n '40,150p' src/astar/astar.cRepository: pgRouting/pgrouting
Length of output: 3554
🏁 Script executed:
# Look for shortestPath.c
find src/ -name "shortestPath.c" -o -name "shortest*.c" | head -5Repository: pgRouting/pgrouting
Length of output: 45
🏁 Script executed:
# Check what SQL signatures exist for these functions
rg -n 'CREATE FUNCTION.*astar|CREATE FUNCTION.*shortestPath' . --type=sql | head -20Repository: pgRouting/pgrouting
Length of output: 170
🏁 Script executed:
# Search for test cases that might show usage patterns
find . -name "*.sql" -path "*/test*" | head -10Repository: pgRouting/pgrouting
Length of output: 45
🏁 Script executed:
# Look at the shortestPath.c file
find src/ -name "*shortestPath*.c" | head -5Repository: pgRouting/pgrouting
Length of output: 45
🏁 Script executed:
# Search in all .c files for process functions matching our pattern
rg -n 'pgr_process.*shortestPath|pgr_process.*astar' src/ --type=cRepository: pgRouting/pgrouting
Length of output: 1390
🏁 Script executed:
# Look at astar_process.cpp to see the normal parameter handling
sed -n '50,100p' src/astar/astar_process.cppRepository: pgRouting/pgrouting
Length of output: 1357
🏁 Script executed:
# Let me verify the exact parameter mappings in astar.c more carefully
sed -n '52,68p' src/astar/astar.cRepository: pgRouting/pgrouting
Length of output: 560
🏁 Script executed:
# Also check the combinations case more closely
sed -n '74,92p' src/astar/astar.cRepository: pgRouting/pgrouting
Length of output: 549
🏁 Script executed:
# Check if there are other SQL signatures for astar that might pass different parameter patterns
cat sql/astar/_astar.sqlRepository: pgRouting/pgrouting
Length of output: 2668
🏁 Script executed:
# Check the shortestPath.c wrapper to understand the parameter flow for both overloads
rg -n 'pgr_process_shortestPath' src/dijkstra/dijkstra.c -B5 -A10 | head -80Repository: pgRouting/pgrouting
Length of output: 2891
🏁 Script executed:
# Let me look at one specific call to understand the context better
sed -n '55,70p' src/dijkstra/dijkstra.cRepository: pgRouting/pgrouting
Length of output: 492
🏁 Script executed:
# Let me verify the SQL function signatures to understand all the different call patterns
# Check what normal values are passed in each case
rg -n 'normal.*true|normal.*false|normal.*BOOL' sql/ --type=sql | grep -E '(astar|dijkstra|shortestPath)' | head -20Repository: pgRouting/pgrouting
Length of output: 433
🏁 Script executed:
# Let me verify the exact flow by examining what gets passed for normal in all cases
# In astar.c, many-to-many uses PG_GETARG_BOOL(8) - so normal can be any boolean
# In astar.c, combinations case uses hardcoded `true` for normal
# Check if there's any path where normal=false with partially null arrays
# by examining shortestPath.c more carefully
sed -n '190,245p' src/dijkstra/dijkstra.cRepository: pgRouting/pgrouting
Length of output: 1985
🏁 Script executed:
# Now I need to check if there's any scenario where mismatched null/non-null can occur
# Let me examine the astarCost.sql and astar.sql more carefully
cat sql/astar/astarCost.sql | grep -A5 "pgr_aStar"Repository: pgRouting/pgrouting
Length of output: 4004
🏁 Script executed:
# Check the actual wrapper function to see all the parameter passing patterns
cat sql/astar/astar.sql | grep -B5 -A5 "pgr_aStar"Repository: pgRouting/pgrouting
Length of output: 4952
🏁 Script executed:
# Let me also check if ANYARRAY parameters can be NULL
# and verify the issue exists by looking at test cases
find . -path "*/test*" -name "*.sql" -exec grep -l "astar.*false" {} \;Repository: pgRouting/pgrouting
Length of output: 45
🏁 Script executed:
# Let me trace through one more time to verify the actual calling patterns
# Check if there's any direct call path where mismatched nulls occur
rg -n 'get_combinations.*normal.*false' src/ --type=cppRepository: pgRouting/pgrouting
Length of output: 45
🏁 Script executed:
# Look for all calls to the string-based overload to confirm parameter patterns
ast-grep --pattern 'get_combinations($_, $_, $_, $_, $_)'Repository: pgRouting/pgrouting
Length of output: 328
🏁 Script executed:
# Final verification: Check if ArrayType can actually be NULL in practice
# by examining how PG_GETARG_ARRAYTYPE_P handles null arrays
rg -n 'PG_GETARG_ARRAYTYPE_P' src/astar/astar.c src/dijkstra/dijkstra.cRepository: pgRouting/pgrouting
Length of output: 622
🏁 Script executed:
# Let me verify if get_intSet can handle null pointers or if it would crash
rg -n 'get_intSet' src/cpp_common/ --type=cpp -A3 | head -40Repository: pgRouting/pgrouting
Length of output: 1769
🏁 Script executed:
# Check the actual definition of get_intSet
rg -n 'std::set.*get_intSet|get_intSet.*{' src/ --type=cpp -A5 | head -50Repository: pgRouting/pgrouting
Length of output: 194
🏁 Script executed:
# Let me examine the PostgreSQL macro behavior to understand if ArrayType can be NULL
rg -n 'PG_GETARG_ARRAYTYPE_P' src/astar/astar.c -B2 -A2Repository: pgRouting/pgrouting
Length of output: 276
🏁 Script executed:
# Final check: verify in all code paths whether both arrays are always both NULL or both non-NULL
# Looking for any edge case where one is NULL and other isn't
rg -n 'starts.*NULL|ends.*NULL' src/astar/ src/dijkstra/ --type=cpp --type=c | grep -v "//"Repository: pgRouting/pgrouting
Length of output: 45
🏁 Script executed:
# Verify by looking at the actual calling sequence one more time
# Check if astar.c and dijkstra.c ever call with mixed null/non-null
sed -n '52,98p' src/astar/astar.cRepository: pgRouting/pgrouting
Length of output: 1363
🏁 Script executed:
# Confirm STRICT behavior by checking function signatures
grep -r "STRICT" sql/astar/_astar.sql sql/dijkstra/ --include="*.sql" | head -10Repository: pgRouting/pgrouting
Length of output: 598
Consider adopting the symmetric null-guard pattern for consistency and robustness.
The asymmetric null guards on lines 161-162 are theoretically unsafe when normal=false, but unreachable in practice. All callers (both SQL and C) guarantee that startsArr and endsArr are either both provided or both null due to PostgreSQL's STRICT semantics and explicit NULL passing in the C layer. When both are non-null, the conditional access is always safe.
However, the other overload (lines 117-120) uses a more defensive pattern:
if (startsArr && endsArr) {
starts = normal ? get_intSet(startsArr) : get_intSet(endsArr);
ends = normal ? get_intSet(endsArr) : get_intSet(startsArr);
}Adopt this symmetric guard for consistency and to eliminate the theoretical vulnerability, making the code more maintainable if calling patterns change in the future.
🤖 Prompt for AI Agents
In `@src/cpp_common/combinations.cpp` around lines 161 - 162, Replace the
asymmetric ternary null-checks for starts and ends with the same symmetric guard
used earlier: check both startsArr and endsArr together before assigning; e.g.,
if (startsArr && endsArr) { starts = normal ? get_intSet(startsArr) :
get_intSet(endsArr); ends = normal ? get_intSet(endsArr) :
get_intSet(startsArr); } else leave starts/ends as empty std::set<int64_t>(); —
this ensures safe use of get_intSet with startsArr/endsArr (variables:
startsArr, endsArr, normal, get_intSet, starts, ends).
| std::string | ||
| get_name(Which which, bool is_only_cost, bool is_matrix) { | ||
| std::string base = ""; | ||
| std::string suffix = ""; | ||
|
|
||
| switch (which) { | ||
| case ASTAR : | ||
| base = "pgr_astar"; | ||
| suffix = std::string(is_only_cost? "Cost" : "") + (is_only_cost && is_matrix? "Matrix" : ""); | ||
| break; | ||
| case BDASTAR : | ||
| base = "pgr_bdAstar"; | ||
| suffix = std::string(is_only_cost? "Cost" : "") + (is_matrix? "Matrix" : ""); | ||
| break; | ||
| default : base = "unknown"; | ||
| } | ||
|
|
||
| return base + suffix; | ||
| } |
There was a problem hiding this comment.
Inconsistent "Matrix" suffix logic between ASTAR and BDASTAR.
For ASTAR (line 105), "Matrix" is appended only when is_only_cost && is_matrix, which correctly avoids producing the non-existent name "pgr_astarMatrix". However, for BDASTAR (line 109), "Matrix" is appended whenever is_matrix is true, regardless of is_only_cost. If a caller ever passes is_only_cost=false, is_matrix=true, this would produce the non-existent name "pgr_bdAstarMatrix".
This mirrors the same inconsistency in the 4-arg overload where BDDIJKSTRA (line 78) also uses is_matrix alone, while DIJKSTRA (line 74, newly changed) now uses is_only_cost && is_matrix.
Consider unifying the guard for consistency and defensive correctness:
Proposed fix
case BDASTAR :
base = "pgr_bdAstar";
- suffix = std::string(is_only_cost? "Cost" : "") + (is_matrix? "Matrix" : "");
+ suffix = std::string(is_only_cost? "Cost" : "") + (is_only_cost && is_matrix? "Matrix" : "");
break;🤖 Prompt for AI Agents
In `@src/cpp_common/utilities.cpp` around lines 97 - 115, The BDASTAR branch in
get_name builds suffix using (is_matrix? "Matrix" : "") which can produce
invalid "pgr_bdAstarMatrix" when is_only_cost is false; change the BDASTAR
suffix construction to mirror ASTAR and DIJKSTRA (only append "Matrix" when
is_only_cost && is_matrix). Update the BDASTAR case in get_name to use
std::string(is_only_cost? "Cost" : "") + (is_only_cost && is_matrix? "Matrix" :
""), and apply the same guard to the BDDIJKSTRA/BD* overloads that currently use
is_matrix alone so all mirrored pairs use is_only_cost && is_matrix for the
"Matrix" suffix.
cvvergara
force-pushed
the
astar-bdastar-using-new-process-and-driver-for-astar-like-functions
branch
from
9ea5a88 to
5d6e503
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/astar/astar_driver.cpp`:
- Line 107: The debug log in astar_driver.cpp uses the stream expression log <<
"is matrix" << is_matrix which concatenates the literal and boolean without
spacing; update the log expression that writes the "is matrix" message (the log
stream usage near the is_matrix variable) to include a separator (e.g., a
trailing space or colon) between the string and the is_matrix value so output
becomes "is matrix 1" or "is matrix: 1".
| hint = nullptr; | ||
| auto combinations = get_combinations(combinations_sql, starts, ends, normal, is_matrix); | ||
| hint = ""; | ||
| log << "is matrix" << is_matrix; |
There was a problem hiding this comment.
Missing space in debug log message.
The log entry concatenates "is matrix" directly with the boolean value, producing output like "is matrix1" or "is matrix0".
🔧 Proposed fix
- log << "is matrix" << is_matrix;
+ log << "is matrix " << is_matrix;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| log << "is matrix" << is_matrix; | |
| log << "is matrix " << is_matrix; |
🤖 Prompt for AI Agents
In `@src/astar/astar_driver.cpp` at line 107, The debug log in astar_driver.cpp
uses the stream expression log << "is matrix" << is_matrix which concatenates
the literal and boolean without spacing; update the log expression that writes
the "is matrix" message (the log stream usage near the is_matrix variable) to
include a separator (e.g., a trailing space or colon) between the string and the
is_matrix value so output becomes "is matrix 1" or "is matrix: 1".
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@locale/en/LC_MESSAGES/pgrouting_doc_strings.po`:
- Around line 3954-3957: The release-note msgid in
locale/en/LC_MESSAGES/pgrouting_doc_strings.po contains a typo ("create an use a
process & driverAstar"); fix the wording in the original release notes/NEWS
source (not the .po) so the corrected English string replaces that msgid (e.g.,
change to something like "create and use a process & driverAstar" or clearer
wording) and then regenerate the locale files (run the project's gettext/locale
extraction and compile steps or use the Weblate workflow) so the .po/.mo are
updated from the source rather than editing pgrouting_doc_strings.po directly.
| msgid "" | ||
| "`#3064 <https://github.com/pgRouting/pgrouting/issues/3064>`__: Astar: " | ||
| "create an use a process & driverAstar" | ||
| msgstr "" |
There was a problem hiding this comment.
Fix the release-note wording in the source, then regenerate locales.
The new msgid reads “create an use a process & driverAstar,” which looks like a grammar/spacing typo. Please correct the wording in the source release notes/NEWS entry and regenerate locale files rather than editing the .po directly.
Based on learnings: Modifications to locale files in pgRouting should be handled through an action commit or through Weblate, not via direct edits.
🤖 Prompt for AI Agents
In `@locale/en/LC_MESSAGES/pgrouting_doc_strings.po` around lines 3954 - 3957, The
release-note msgid in locale/en/LC_MESSAGES/pgrouting_doc_strings.po contains a
typo ("create an use a process & driverAstar"); fix the wording in the original
release notes/NEWS source (not the .po) so the corrected English string replaces
that msgid (e.g., change to something like "create and use a process &
driverAstar" or clearer wording) and then regenerate the locale files (run the
project's gettext/locale extraction and compile steps or use the Weblate
workflow) so the .po/.mo are updated from the source rather than editing
pgrouting_doc_strings.po directly.
2 participants
Fixes #3064
Changes proposed in this pull request:
@pgRouting/admins
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Documentation