Skip to content

Week 5 sloan ordering #460

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

Open
wants to merge 28 commits into
base: bipashabg-2025
Choose a base branch
from

Conversation

bipashabg
Copy link

@bipashabg bipashabg commented Jun 30, 2025

Fixes # .

Changes proposed in this pull request:

  • TBD

@pgRouting/admins

Summary by CodeRabbit

  • Documentation

    • Updated documentation to remove references to the pgr_sloanOrdering function and align function signature comments with the actual implementation.
    • Added a new SQL query example for pgr_sloanOrdering in the documentation.
  • Bug Fixes

    • Corrected function signature comments to accurately reflect the current parameter list.
  • Refactor

    • Simplified internal data structures and function signatures for ordering-related functions, improving consistency and maintainability.
  • Tests

    • Updated and expanded test cases for pgr_sloanOrdering to ensure broader version coverage and accuracy.
  • Chores

    • Adjusted build configuration files to include new and updated source and documentation files.

Copy link

coderabbitai bot commented Jun 30, 2025

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Walkthrough

References to the pgr_sloanOrdering function were removed from the ordering family documentation. The build and test system was updated to include a new example and test for sloanOrdering. Several C++ and C headers and source files were refactored to simplify function signatures and data structures, primarily replacing custom tuple types with int64_t arrays and updating related function interfaces.

Changes

File(s) Change Summary
doc/ordering/ordering-family.rst Removed all documentation references to pgr_sloanOrdering.
docqueries/ordering/CMakeLists.txt, docqueries/ordering/sloanOrdering.pg, docqueries/ordering/test.conf Added sloanOrdering example and test to documentation build and test configuration.
include/drivers/ordering_driver.hpp, include/process/ordering_process.h, src/ordering/ordering_driver.cpp, src/ordering/ordering_process.cpp Refactored function signatures and parameter types, replacing custom tuple types with int64_t arrays.
include/ordering/sloanOrdering.hpp Minor formatting and include adjustments; no logic changes.
pgtap/ordering/sloan/types_check.pg Updated test plan logic and array argument formatting.
sql/ordering/_sloanOrdering.sql, sql/ordering/sloanOrdering.sql Removed extra whitespace and corrected function signature in comments.
src/ordering/CMakeLists.txt Added ordering_process.cpp to build sources.
src/ordering/sloanOrdering.c Simplified result tuple logic and removed unused helper and type includes.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant SQL_Function (pgr_sloanOrdering)
    participant C_Layer (_pgr_sloanordering)
    participant Ordering_Process (pgr_process_ordering)
    participant Ordering_Driver (do_ordering)
    participant Result

    User->>SQL_Function: CALL pgr_sloanOrdering(edges_sql)
    SQL_Function->>C_Layer: _pgr_sloanordering(PG_FUNCTION_ARGS)
    C_Layer->>Ordering_Process: pgr_process_ordering(edges_sql, which, int64_t **, size_t *)
    Ordering_Process->>Ordering_Driver: do_ordering(edges_sql, which, int64_t **, size_t *, ...)
    Ordering_Driver-->>Ordering_Process: result_tuples (int64_t array)
    Ordering_Process-->>C_Layer: result_tuples, result_count
    C_Layer-->>SQL_Function: Construct tuples from int64_t array
    SQL_Function-->>User: Return result set
Loading

Poem

A hop, a skip, a code refactor,
Sloan hops out, the docs grow neater.
Arrays of numbers, tuples trimmed,
The build and tests now freshly primmed.
With every change, the code grows light—
Let’s order graphs from left to right!

((\
( -.-)
o_(")(")


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.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need 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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@cvvergara
Copy link
Member

cvvergara commented Jun 30, 2025

@bipashabg
It was my fault the server crash, forgot to : hint = "" further down in the driver code (had hint = nullptr).

Please write in a comment all the tasks/commits I made during the meeting, I suppose that at least you took notes, a screenshot or something so that you don't forget what to do. I just want to make sure that you do not forget.

Here are some additional TODO:

  • Instead of II_r_t type use int64_t here
    • Fix all consequences of that small change
      • Hint: easier to do with a one liner perl command to substitute perl -pi -e 's/fromwhat/towhat/' file
  • Remove the includes of the type for example: here

@bipashabg
Copy link
Author

bipashabg commented Jun 30, 2025

Commits/tasks made during the meeting:

  • fixing comment
  • process, fixing parameters
  • ordering_driver: changing parameters, fix function name
  • fix sql file
  • fix types_check.pg
  • fix ordering_driver.hpp file
  • deleting unused code and adding ordering_process.cpp to the build
  • fixed type of the hint in ordering_driver.cpp
  • new file sloanOrdering.pg in docqueries/ordering
  • created the initial files of the documentation queries
  • getting a segmentation fault
  • change ordering.hpp to sloanOrdering.hpp

@bipashabg
Copy link
Author

@coderabbitai summary

Copy link

coderabbitai bot commented Jul 4, 2025

✅ Actions performed

Summary regeneration triggered.

void
pgr_do_ordering(
void
do_ordering(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you get this kind of error:

 ERROR:  could not load library "/usr/lib/postgresql/16/lib/libpgrouting-4.0.so": 
 /usr/lib/postgresql/16/lib/libpgrouting-4.0.so: undefined symbol: _Z11do_orderingPKciPlPmPPcS4_S4_

Because the implementation does not match the definition.

https://github.com/pgRouting/GSoC-pgRouting/pull/460/files#diff-c78780c857dc6a395eed58eab37e1edbee777cbc83eca43be4e3e57971ff36ae

#include <string>


void
do_ordering(
std::string, int64_t, int64_t,
const char*, int,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I dont understand why in 100% C++ code you want to use C arrays?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants