pgRouting / pgrouting

Repository contains pgRouting library. Development branch is "develop", stable branch is "master"
https://pgrouting.org
GNU General Public License v2.0
1.14k stars 364 forks source link

To C++ pg data get, fetch and check #2504

Closed cvvergara closed 1 year ago

cvvergara commented 1 year ago

During the last 10 years, an effort to standarize code in pgRouting has been done. With the new trsp functions, finnaly, the code is standarized. (It still have the code of the old signature of pgr_trsp.)

By standarize, I mean, the C/C++ code is similar from one function to another. As a first step: Comparing coordinates_input with delauny_input (et all) They all look similar.

This are fetchers:

In general, this is code common to both functions

Except for the code

The function name and the different code is what is now part of the pgdata_getters.

The similar code is what is now a template header. include/cpp_common/get_data.hpp added in the first commit of this PR.

The fetchers:

The postgresql_connection:

The get_check_data

why this change is good

Adding a new inner query implies: Before this change:

After this change

drawback

Support for compiling with MSVC has to end.

D:/a/pgrouting/pgrouting/pgsql/pgsql/include/server\port/atomics/generic-msvc.h(83,12): message : while trying to match the argument list '(volatile uint64 *, uint64, uint64)' [D:\a\pgrouting\pgrouting\build\src
\cpp_common\cpp_common.vcxproj]
D:/a/pgrouting/pgrouting/pgsql/pgsql/include/server\port/atomics/generic-msvc.h(97,9): error C2664: 'LONG64 _InterlockedExchangeAdd64(volatile LONG64 *,LONG64)': cannot convert argument 1 from 'volatile uint64 *
' to 'volatile LONG64 *' [D:\a\pgrouting\pgrouting\build\src\cpp_common\cpp_common.vcxproj]
D:/a/pgrouting/pgrouting/pgsql/pgsql/include/server\port/atomics/generic-msvc.h(97,35): message : Types pointed to are unrelated; conversion requires reinterpret_cast, C-style cast or parenthesized function-styl
e cast [D:\a\pgrouting\pgrouting\build\src\cpp_common\cpp_common.vcxproj]

Where is that error happening?

Is pgRouting the first one to encounter this problem? No: extension plv8 also has the problem

How is it solved: By patching postgres: https://plv8.github.io/#patching-postgres

I did had a conversation with @JerrySievert from plv8, on postgres slack channel on March 27, 2023. I quote:

yeah, that's part of the problem of dealing with c++ and postgres, but as far as i've been able to tell over the years, this is the only postgres-specific msvc++ build issue. of course you also have to worry about msvc++ not actually implementing the standards it claims to support. and that is why plv8 may or may not compile on windows currently.

do pgRouting developers need MSVC?

Statistics

Less code to maintain. :+1:

Currently (in develop)

cloc 0d4985b19b
    1715 text files.
    1708 unique files.
     626 files ignored.

github.com/AlDanial/cloc v 1.90  T=0.55 s (1983.3 files/s, 426992.8 lines/s)
--------------------------------------------------------------------------------
Language                      files          blank        comment           code
--------------------------------------------------------------------------------
PO File                           8          21019            598          62727
SQL                             319           4822           6774          40403
C++                             106           3323           3307          12770
C/C++ Header                    195           4615           6149          11115
C                                74           3383           2789          10162
reStructuredText                131           9313          10946           9282
CMake                           164            653            350           2195
Perl                             41            455            151           1925
Standard ML                       6              0              0           1555
YAML                             14            225             98           1123
Bourne Shell                     20            240            540            827
JavaScript                        1             20              1            383
Markdown                          7            159              0            343
XML                               1              5              7            182
CSS                               1             10              1             45
SQL Data                          1              5              0             41
Bourne Again Shell                1              6             16             22
HTML                              2              2              0             18
--------------------------------------------------------------------------------
SUM:                           1092          48255          31727         155118
--------------------------------------------------------------------------------

With this PR:

cloc f59716308a
    1706 text files.
    1699 unique files.
     626 files ignored.

github.com/AlDanial/cloc v 1.90  T=0.49 s (2213.0 files/s, 478090.6 lines/s)
--------------------------------------------------------------------------------
Language                      files          blank        comment           code
--------------------------------------------------------------------------------
PO File                           8          21019            598          62727
SQL                             319           4822           6774          40403
C++                             108           3478           3636          13677
C/C++ Header                    195           4647           6042          11226
reStructuredText                131           9318          10944           9296
C                                64           2910           2357           8520
CMake                           164            651            350           2186
Perl                             41            455            151           1925
Standard ML                       6              0              0           1555
YAML                             14            225             98           1123
Bourne Shell                     20            240            540            827
JavaScript                        1             20              1            383
Markdown                          6            154              0            329
XML                               1              5              7            182
CSS                               1             10              1             45
SQL Data                          1              5              0             41
Bourne Again Shell                1              6             16             22
HTML                              2              2              0             18
--------------------------------------------------------------------------------
SUM:                           1083          47967          31515         154485
--------------------------------------------------------------------------------

Summary:

Files develop -> this PR change
SUM: 1092 -> 1083 -9
C 74 -> 64 -10
C++ 106 -> 108 +2
C/C++ Header 195 -> 195 0
Markdown 195 -> 195 0
Code develop -> this PR change
SUM: 155118 -> 154485 -633
C 10162 -> 8520 -1642
C++ 12770 -> 13677 +907
C/C++ Header 11115 -> 11226 +111
Other develop -> this PR change
blank 48255 -> 47967 -288
comment 31727 -> 31515 -212

TODO

todo or not todo:

@pgRouting/admins

robe2 commented 1 year ago

I tested on my new msys2/mingw64 chain running gcc.exe (Rev10, Built by MSYS2 project) 12.2.0 and it compiles fine. I still need to test the pgTap tests, but my perlTAP thing is hanging on installing the perl TAP modules.

cvvergara commented 1 year ago

I tested on my new msys2/mingw64 chain running gcc.exe (Rev10, Built by MSYS2 project) 12.2.0 and it compiles fine. I still need to test the pgTap tests, but my perlTAP thing is hanging on installing the perl TAP modules.

Cool, so now I am in the refinement process. (That is why I converted to draft)

cvvergara commented 1 year ago

Motion has passed: https://lists.osgeo.org/pipermail/pgrouting-dev/2023-May/002425.html

cvvergara commented 5 months ago

Closes #2494