8000 Read postgresql data on C++ by cvvergara · Pull Request #2607 · pgRouting/pgrouting · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Read postgresql data on C++ #2607

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 71 commits into from
Feb 1, 2024
Merged

Read postgresql data on C++ #2607

merged 71 commits into from
Feb 1, 2024

Conversation

cvvergara
Copy link
Member
@cvvergara cvvergara commented Jan 24, 2024 •
edited
Loading

Moving the input queries from the C code into the C++ code.

Before this change

On the C file:

  • Includes what we call a driver header is linked as C
    • The driver headers are very similar to each other
  • the static process within each C file example are very similar
    • opens the connection with PostgreSQL
    • reads the data, (edges sql, restrictions_sql, etc and any array that is on the query)
      • all of these reading is done on the C code which interacts with PostgreSQL, (which is written in C.)
      • Suppose, for simplifying this description, that the data takes x MB in memory
    • calls the pgr_do_function defined on the driver
    • gets the results
    • closes the connection with PostgreSQL

On the driver C++ file:

  • code is written in C++
  • Drivers are very similar within each other.
    • Convert C arrays to C++ containers
      • Now memory size is 2x + container overhead
  • Builds the boost graph
    • Now memory size is 3x + container overhead
  • None of this code interacts with PostgreSQL

The ideal situation:

  • Build the boost graph as the data is read.
  • Have C++ templates on the drivers
    • Being so similar that can be done

General steps to reach the ideal situation

  1. Be able to read the PostgreSQL data on the C++ code
  2. Create the templates
  3. Build the boost graphs based on the templates needs

This PR is step 1

In rough terms, moving the reading of the data to C++.

The sketch of the C & C++ driver files for the first step:

On the C file:

  • Includes what we call a driver header linked as C
    • The driver headers are very similar to each other
  • the static process within each C file will still be very similar
    • opens the connection with PostgreSQL
    • calls the pgr_do_function defined on the driver
    • gets the results
    • closes the connection with PostgreSQL

On the driver C++ file:

  • code is written in C++
  • Drivers will still be very similar within each other.
    • C++ code will interact with PostgreSQL
    • Read the data directly into C++ containers
      • Two kinds of data
        • Data that comes from the inner SQL queries
        • Data that comes from arrays
      • All of these reading will be do with C++ code
      • Now the reading into C++ container takes x MB in memory
  • Builds the boost graph
    • Now memory size is 2x + container overhead

tasks

  • Copy the files pgdata_getters and pgdata_fetchers 8000 to trsp_pgget & trsp_pgfetch
    • Verify that the only function that is including the trsp_pgget is the deprecated pgr_trsp function
    • Remove the unused code on trsp_pgget & trsp_pgfetch
  • Create workaround to postgres defines of functions that exist on the standard
  • Adjust pgdata_getters and pgdata_fetchers to be used directly from C++
    • parameter is a string containing the SQL query
    • return a structure instead of a pointer
    • return a C++ container instead of a pointer
  • Create new template get_data that works with the changes on pgdata_getters and pgdata_fetchers
  • Do general adjustments to several files
  • Per sub directory basis
    • Delete from C file code that reads the data
    • Add into C++ driver file(s) the code that reads the data
    • If necessary adjust the code that use a C array to use the C++ container
  • standardize the driver names to start with pgr_do_
  • Update release_notes & NEWS

@cvvergara cvvergara added this to the Release 3.7.0 milestone Jan 24, 2024
@cvvergara
:heavy-plus-sign: trsp_pgget copy of pgdata_getters
0ac40ef
@cvvergara
c_common pgdata_getters.h -> cpp_common/pgdata_getters.hpp
65c84de 
pgdata_getters will be called from C++ code only
@cvvergara
pgdata_getters to be used from C++
29264ef 
- parameter is the sql query
- returns a C++ container
@cvvergara
pgdata_fetchers to return a (C++) structure
8e5930a
@cvvergara
get_check_data arrays stored them in C++ containers
bbe3710
@cvvergara
cpp_common headers minor changes
76a37f4
@cvvergara
get_data: New template function to read data from C++
feefe5a
@cvvergara
interruption.h -> interruption.hpp should be a C++ header file
442d1e3
@cvvergara
(C++) Using interruption.hpp
9fcd6b2
@cvvergara
interruption.hpp undef posgres defines
22b0fdb
@cvvergara
pgr_base_graph.hpp
bf8ec68
@cvvergara
combinations.h -> combinations.hpp should be A c++ header
7f78c9f
@cvvergara
Combinations new functions to create from container
8ba1f41
@cvvergara
Using combinations.hpp
7571453
@cvvergara
On C code using trsp_pgget
4eefcab 
- This include will be removed when the reading takes place on C++
- The only expected place to remain is on old function of trsp
@cvvergara
[tools] pgtap script to not show NOTICE
a6118e2
@cvvergara
[allpairs] Reading data on C++ code part1
775ca21 
- C files: marking unused code
  - cleanup: removing comments
- _driver.h:
  - parameter for queries is the sql char*
  - Clean up: removing parameter names & comments
- _driver.cpp:
  - c++ code read the data from the SQL queries
  - Adding a catch for input errors
@cvvergara
[allpairs] Reading data on C++ code part2 (removing unused code)
cb194b4
@cvvergara
[alpha_shape] Reading data on C++ code
1e9f2a3
@cvvergara
[alpha_shape] do_ -> pgr_do_
a5d36f8
@cvvergara
[astar] Reading data on C++ code
bf74f30
@cvvergara
[bdAstar] Reading data on C++ code
ae00930
@cvvergara
[bdDijkstra] Reading data on C++ code
3c084eb
@cvvergara
[bellman_ford] Reading data on C++ code
9eedf0a
@cvvergara
[breadthFirstSearch] Reading data on C++ code
f368453
@cvvergara cvvergara force-pushed the read-data-on-cpp-try3 branch from 64bba51 to 3be0767 Compare January 25, 2024 16:16
@cvvergara cvvergara force-pushed the read-data-on-cpp-try3 branch from 47eef4e to d6585aa Compare January 31, 2024 14:13
@cvvergara cvvergara marked this pull request as ready for review January 31, 2024 20:22
@cvvergara cvvergara requested review from sanak and robe2 January 31, 2024 20:23
@cvvergara
Copy link
Member Author

Something happened on the cpplint that is not working anymore on the CI.
But locally it is ok

@cvvergara
Copy link
Member Author

Opened an issue here

@sanak
Copy link
Member
sanak commented Feb 1, 2024

@cvvergara I encountered the following build error on my M1 MacBook Pro environment.

:
[ 60%] Built target bellman_ford
[ 61%] Building CXX object src/cpp_common/CMakeFiles/cpp_common.dir/Dmatrix.cpp.o
[ 61%] Building CXX object src/cpp_common/CMakeFiles/cpp_common.dir/compPaths.cpp.o
[ 62%] Building CXX object src/cpp_common/CMakeFiles/cpp_common.dir/rule.cpp.o
[ 62%] Building CXX object src/cpp_common/CMakeFiles/cpp_common.dir/bpoint.cpp.o
[ 63%] Building CXX object src/cpp_common/CMakeFiles/cpp_common.dir/pgr_messages.cpp.o
[ 63%] Building CXX object src/cpp_common/CMakeFiles/cpp_common.dir/combinations.cpp.o
In file included from /Users/sanak/Projects/pgRouting/git/pgrouting/src/cpp_common/combinations.cpp:27:
In file included from /Users/sanak/Projects/pgRouting/git/pgrouting/include/cpp_common/combinations.hpp:46:
In file included from /Users/sanak/Projects/pgRouting/git/pgrouting/include/cpp_common/basePath_SSEC.hpp:44:
In file included from /opt/homebrew/include/boost/graph/adjacency_list.hpp:20:
In file included from /opt/homebrew/include/boost/unordered_set.hpp:17:
In file included from /opt/homebrew/include/boost/unordered/unordered_set.hpp:17:
In file included from /opt/homebrew/include/boost/unordered/detail/serialize_fca_container.hpp:12:
In file included from /opt/homebrew/include/boost/unordered/detail/serialize_container.hpp:13:
In file included from /opt/homebrew/include/boost/throw_exception.hpp:21:
In file included from /opt/homebrew/include/boost/exception/exception.hpp:9:
/opt/homebrew/include/boost/assert/source_location.hpp:95:9: error: no member named 'snprintf' in namespace 'std'; did you mean simply 'snprintf'?
        BOOST_ASSERT_SNPRINTF( buffer, ":%lu", ln );
        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/opt/homebrew/include/boost/assert/source_location.hpp:79:53: note: expanded from macro 'BOOST_ASSERT_SNPRINTF'
# define BOOST_ASSERT_SNPRINTF(buffer, format, arg) std::snprintf(buffer, sizeof(buffer)/sizeof(buffer[0]), format, arg)
                                                    ^~~~~
/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX14.2.sdk/usr/include/stdio.h:337:6: note: 'snprintf' declared here
int      snprintf(char * __restrict __str, size_t __size, const char * __restrict __format, ...) __printflike(3, 4);
         ^
In file included from /Users/sanak/Projects/pgRouting/git/pgrouting/src/cpp_common/combinations.cpp:27:
In file included from /Users/sanak/Projects/pgRouting/git/pgrouting/include/cpp_common/combinations.hpp:46:
In file included from /Users/sanak/Projects/pgRouting/git/pgrouting/include/cpp_common/basePath_SSEC.hpp:44:
In file included from /opt/homebrew/include/boost/graph/adjacency_list.hpp:20:
In file included from /opt/homebrew/include/boost/unordered_set.hpp:17:
In file included from /opt/homebrew/include/boost/unordered/unordered_set.hpp:17:
In file included from /opt/homebrew/include/boost/unordered/detail/serialize_fca_container.hpp:12:
In file included from /opt/homebrew/include/boost/unordered/detail/serialize_container.hpp:13:
In file included from /opt/homebrew/include/boost/throw_exception.hpp:21:
In file included from /opt/homebrew/include/boost/exception/exception.hpp:9:
/opt/homebrew/include/boost/assert/source_location.hpp:102:13: error: no member named 'snprintf' in namespace 'std'; did you mean simply 'snprintf'?
            BOOST_ASSERT_SNPRINTF( buffer, ":%lu", co );
            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/opt/homebrew/include/boost/assert/source_location.hpp:79:53: note: expanded from macro 'BOOST_ASSERT_SNPRINTF'
# define BOOST_ASSERT_SNPRINTF(buffer, format, arg) std::snprintf(buffer, sizeof(buffer)/sizeof(buffer[0]), format, arg)
                                                    ^~~~~
/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX14.2.sdk/usr/include/stdio.h:337:6: note: 'snprintf' declared here
int      snprintf(char * __restrict __str, size_t __size, const char * __restrict __format, ...) __printflike(3, 4);
         ^
2 errors generated.
make[2]: *** [src/cpp_common/CMakeFiles/cpp_common.dir/combinations.cpp.o] Error 1
make[1]: *** [src/cpp_common/CMakeFiles/cpp_common.dir/all] Error 2
make: *** [all] Error 2

And changing the following was necessary to pass the build.

--- a/src/cpp_common/combinations.cpp
+++ b/src/cpp_common/combinations.cpp
@@ -24,12 +24,11 @@ Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
 
  ********************************************************************PGR-GNU*/
 
-#include "cpp_common/combinations.hpp"
-
 #include <map>
 #include <set>
 #include <deque>
 #include <vector>
+#include "cpp_common/combinations.hpp"
 #include "cpp_common/pgdata_getters.hpp"
 #include "cpp_common/basePath_SSEC.hpp"

After the build, it seems to be fine, but I will check more at tomorrow.

@cvvergara cvvergara merged commit e15a0c5 into pgRouting:develop Feb 1, 2024
@cvvergara cvvergara deleted the read-data-on-cpp-try3 branch February 1, 2024 17:50
@cvvergara cvvergara mentioned this pull request Feb 8, 2024
Closed
This was referenced Feb 13, 2024
Merged
Closed
@coderabbitai coderabbitai
Copy link
Contributor
coderabbitai bot commented Mar 26, 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.

🪧 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.
    • Generate unit testing code for this file.
    • 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 generate unit testing code for this file.
    • @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 generate unit testing code.
    • @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.

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 resolve resolve all the CodeRabbit review comments.
  • @coderabbitai plan to trigger planning for file edits and PR creation.
  • @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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@robe2 robe2 robe2 approved these changes

@sanak sanak Awaiting requested review from sanak

Assignees
No one assigned
Labels
C/C++ Enhancement
Projects
None yet
Milestone
Release 3.7.0
Development

Successfully merging this pull request may close these issues.
3 participants
@cvvergara @sanak @robe2
0