-
-
Notifications
You must be signed in to change notification settings - Fork 377
Remove deprecated signatures of pgr maxCardinalityMatch #2900
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 deprecated signatures of pgr maxCardinalityMatch #2900
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 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:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
- sql: internal function retuns only the edge - C++ code - new class for undirected graphs without cost - Instead of a class, maxCardinalityMatch function - Moving code from header to implementation file - driver returns integer array, builds graph and makes call to maxCardinalityMatch function - Adjusting C code because of driver returning an integer array - Adding the new files to the build
- Removing deprecated internal function _pgr_maxCardinalityMatch - Removing deprecated C code to legacy and adjusting remaining C code - (pgtap) Testing only valid signatures in 4.0.0 - Keeping the deprecated C code for the duration of v4
3740d79 to
15b8709
Compare
|
@coderabbitai summary |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain@cvvergara I'll gather the information about this PR and provide both a summary and review. Let me first examine the changes in this pull request. 🏁 Script executed: Length of output: 43250 @cvvergara Here's the summary and review for PR #2900: SummaryThis PR removes deprecated signatures of Key Changes:
Files Modified:
Review✅ Strengths
class UndirectedNoCostsBG {
using G = boost::adjacency_list<boost::listS, boost::vecS, boost::undirectedS>;
// Clean interface with proper encapsulation
};
void pgr_do_maximum_cardinality_matching(
const char *edges_sql,
int64_t **return_tuples, // Simplified return type
size_t *return_count,
// ... error handling
);
🔍 Areas for Attention
if (edge.going) {
boost::tie(e, added) = boost::add_edge(v1, v2, graph);
if (added) E_to_id.insert(std::pair<E, int64_t>(e, edge.id));
}The conditional insertion looks correct, but verify edge deduplication behavior.
boost::tie(e, exists) = boost::edge(v1, v2, graph());
if (!exists) throw; // Consider more descriptive errorThe bare
values[0] = 0; // seq
values[1] = Int64GetDatum(result_tuples[funcctx->call_cntr]); // edge
values[2] = 0; // source
values[3] = 0; // targetConsider adding a deprecation warning when this function is called. 📋 Recommendations
if (!exists) {
throw std::runtime_error("Edge does not exist in matching result");
}
✅ Overall AssessmentThis is a well-executed refactoring that successfully:
The code quality is good and the approach is sound. The changes align well with the goal of cleaning up the API for the major version release. |
Fixes #2899
Changes proposed in this pull request:
@coderabbitai summary
@pgRouting/admins