Skip to content
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

Request for interest: c++-20 modularization of sqlpp11 (I did some in my own project) and making sqlpp11 modularization-friendly. #617

Open
germandiagogomez opened this issue Mar 15, 2025 · 15 comments

Comments

@germandiagogomez
Copy link

germandiagogomez commented Mar 15, 2025

Dear Sqlpp11 project,

I am converting my own code into a modularized codebase, which made me convert several dependencies, among them, partial sqlpp11.

Please note that I am using sqlite3 so ignore the sqlite3 stuff, it is there just bc it made my port more convenient. That would belong to a sqlite3 module, obviously.

This is just a partial conversion and exposes a module with symbols in a per-need basis from a project I am developing. So far I have something like this (and it works):

Partial module file (so far)

module;

#include <sqlpp11/sqlpp11.h>
#include <sqlpp11/sqlite3/connection.h>
#include <sqlpp11/sqlite3/connection_config.h>
#include <sqlpp11/connection_pool.h>
#include <sqlpp11/sqlite3/connection_pool.h>

inline constexpr int SQLITE_OPEN_READONLY_MODULES = SQLITE_OPEN_READONLY;
inline constexpr int SQLITE_OPEN_READWRITE_MODULES = SQLITE_OPEN_READWRITE;
inline constexpr int SQLITE_OPEN_CREATE_MODULES = SQLITE_OPEN_CREATE;

#undef SQLITE_OPEN_READONLY
#undef SQLITE_OPEN_READWRITE
#undef SQLITE_OPEN_CREATE

inline constexpr int SQLITE_OPEN_READONLY = {SQLITE_OPEN_READONLY_MODULES};
inline constexpr int SQLITE_OPEN_READWRITE = {SQLITE_OPEN_READWRITE_MODULES};
inline constexpr int SQLITE_OPEN_CREATE = {SQLITE_OPEN_CREATE_MODULES};



export module sqlpp11;

export {

using ::SQLITE_OPEN_READONLY;
using ::SQLITE_OPEN_READWRITE;
using ::SQLITE_OPEN_CREATE;

}


export namespace sqlpp {

    using ::sqlpp::make_char_sequence;
    using ::sqlpp::make_traits;
    using ::sqlpp::char_;
    using ::sqlpp::time_point;
    using ::sqlpp::varchar;
    using ::sqlpp::integer;
    using ::sqlpp::bigint;
    using ::sqlpp::smallint;
    using ::sqlpp::smallint_unsigned;
    using ::sqlpp::blob;
    using ::sqlpp::bigint_unsigned;


    using ::sqlpp::interpret_tuple_element;
    using ::sqlpp::interpret_tuple_impl;
    using ::sqlpp::interpret_tuple;


    using ::sqlpp::all_of;
    using ::sqlpp::select;
    using ::sqlpp::insert;
    using ::sqlpp::insert_into;
    using ::sqlpp::update;
    using ::sqlpp::remove;
    using ::sqlpp::connection_pool;

    namespace chrono {
        using ::sqlpp::chrono::floor;
    }

    namespace detail {
        using ::sqlpp::detail::make_type_set;
        using ::sqlpp::detail::make_type_set_if;
        using ::sqlpp::detail::make_difference_set;
    }

    using ::sqlpp::table_t;
    using ::sqlpp::column_t;

    namespace tag {
        using ::sqlpp::tag::require_insert;
        using ::sqlpp::tag::can_be_null;
        using ::sqlpp::tag::must_not_insert;
        using ::sqlpp::tag::requires_parens;
        using ::sqlpp::tag::is_column;
    }

    namespace sqlite3 {
        using ::sqlpp::sqlite3::pooled_connection;
        using ::sqlpp::sqlite3::connection;
        using ::sqlpp::sqlite3::connection_pool;
        using ::sqlpp::sqlite3::connection_config;
    }
}

// Sure, this does not belong here, I packed it all at once for the sake of advancing in my own project.
export using ::sqlite3_exec;

About generated code (via ddl2cpp)

I am also converting my table files with ddl2cpp via a custom script that does something like this. Basically I let ddl2cpp convert the code, I get that input, I remove all lines starting with a hash, I replace by

export module my.tables;
import sqlpp11;

export namespace thetablesnamespace {
//...
}

Problems found while modularizing.

So far I found a single problem: in file interpret_tuple.h, the static function interpret_tuple_element cannot be exported, but if it is not, then the code won't compile. I patched sqlpp11 to just remove static since I could not find a workaround.

I would like to talk about contributing, in a more tidied-up and systematic way a modularized sqlpp11 version.

@rbock
Copy link
Owner

rbock commented Mar 16, 2025

Hi @germandiagogomez,

Thanks for reaching out! Yes, modularization is of interest for sure. I haven't looked into it yet, so any contribution would be greatly appreciated.

Question: I am currently working on a new branch next-gen which is using C++23 (explicit object parameters are awesome for this library). I think I will move that to it's own repository, soon. If that's of interest to you, maybe we could add modularization there?

Cheers,
Roland

@germandiagogomez
Copy link
Author

germandiagogomez commented Mar 16, 2025

@rbock it would be a nice idea indeed. That new repo you mention is the way to go as long as the lib keeps working for me. I am using it actively for something I will eventually ship.

There are several ways to do modularization. One is the way I showed here. The caveat is that no symbol must be forgotten on reexport and that new symbols must be added, which always adds a maintenance burden.

Another is to add a macro that expands to export in headers and mark symbols and go with that directly. Probably that is the better way since new symbols can be declared with the macro as necessary without having to explicitly touch the module interface file.

Something like this and marking symbols in-place in the original source code: https://github.com/fmtlib/fmt/blob/master/src/fmt.cc

@rbock
Copy link
Owner

rbock commented Mar 16, 2025

OK, I plan to create the new repository in a couple of days and will ping this thread again once it is there.

@MeanSquaredError
Copy link
Contributor

@rbock I am sorry for hijacking the thread, but it seems that there are a total of 35 commits from sqlpp11 which have not been ported to sqlpp23.

I am mostly interested in the C++20 range fix ( #593 ) and the ddl2cpp fixes (#575 , #574 , #572 ), but it probably makes sense to forward-port everything from those 35 commits as these are mostly fixes that are relevant to the new sqlpp23 too.

Most of these don't seem to be related directly to the core, so hopefully they are not difficult to port (at least the range fix seems to apply easily to the new version).

What do you think?

@rbock
Copy link
Owner

rbock commented Mar 17, 2025

Thanks, @MeanSquaredError, and no worries about hijacking as this is quite relevant in the context.

Yes, those need yet to be applied.

@rbock
Copy link
Owner

rbock commented Mar 18, 2025

FYI, I think merged all relevant commits into next-gen.

@rbock
Copy link
Owner

rbock commented Mar 18, 2025

Here we go: https://github.com/rbock/sqlpp23

The documentation is not up to date, examples need work, I only tested with clang++ recently, but since I want to preserve the history anyway, it made sense to publish it in its own repository already.

@emsr
Copy link

emsr commented Mar 18, 2025

I just built and tested with g++-15 and ninja. Built and tested clean! I haven't played with it yet.

@MeanSquaredError
Copy link
Contributor

Great! In the next couple of days I am going to try porting one of my projects from sqlpp11 to sqlpp23 and will let you know how it goes.

@rbock
Copy link
Owner

rbock commented Mar 19, 2025

Thanks @emsr, this is good news!

All: Looking forward to bug reports and pull requests :-)

@MeanSquaredError
Copy link
Contributor

OK, I built and tested the new sqlpp23 and the core, PostgreSQL, MySQL and SQLite3 tests passed successfully.

g++ (GCC) 14.2.1 20250110 (Red Hat 14.2.1-7)
cmake version 3.30.8

The build was configured with

cmake -B build -DBUILD_POSTGRESQL_CONNECTOR=ON -DBUILD_SQLITE3_CONNECTOR=ON -DBUILD_MYSQL_CONNECTOR=ON -DBUILD_TESTING=ON

@MeanSquaredError
Copy link
Contributor

MeanSquaredError commented Mar 22, 2025

OK, I finished switching one of my pet projects from sqlpp11 to sqlpp23. The project has a total of

31 different SQL queries
Out of them 1 dynamic update query
14 transactions (their BEGIN, COMMIT, ROLLBACK queries are not counted in the 31 queries).

Overall the switch from sqlpp11 to sqlpp23 was relatively simple. There were a few issues that I listed on the Issues page for sqlpp23. None of these issues were serious enough to prevent me from using the new sqlpp23, though.

@germandiagogomez
Copy link
Author

germandiagogomez commented Mar 22, 2025

@rbock maybe I can try modularization across next weekend and if not enough time, the other as well (I estimate it should not take much longer than that). Could I do it directly in sqlpp11 codebase?

So that it is useful for my project without burning too much time (I want to avoid porting for now and will stick to what already works for me)probably it is a good idea to go as shown below:

  1. Add a macro SQLPP11_USE_MODULES
  2. mark with SQLPP11_EXPORT_SYMBOL conditionally exporting to the module the marked symbols, according to the configuration.
  3. SQLPP11_USE_IMPORT_STD, to use import std when available and activated.
  4. Add a sqlpp.cppm module file, which will include stuff.
  5. Use proper options to CMake for controlling the macros and compilation mode of modules.
  6. Stick to the core (not to the drivers for sqlite, postgres, etc.)
  7. Add appropriate CMake code for compiling the module.

This should be easy to port to sqlpp23 and it is also the most useful way for me to contribute it, since I am very tight on time.

After that, you could model sqlpp11.sqlite and other drivers by importing sqlpp11 module and using the same technique here.

I am planning to test it in clang 19, maybe on gcc but not in MSVC.

Would that be good enough?

Would be good to have a review on whether symbols I export are the correct ones as well once I commit something.

@rbock
Copy link
Owner

rbock commented Mar 22, 2025

@germandiagogomez That sounds good to me.

Please also add a test (conditional on the C++ version).

@rbock
Copy link
Owner

rbock commented Mar 22, 2025

Note that I am also quite tied up in something else for a couple of days and might be slow to answer.

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

No branches or pull requests

4 participants