Re: [review] [sqlite]

From: Scott Bailey (Scott_at_[hidden])
Date: 2024-11-22 16:21:37

On Fri, Nov 22, 2024 at 4:35 AM Klemens Morgenstern <
klemensdavidmorgenstern_at_[hidden]> wrote:

>
>
> On Fri, Nov 22, 2024 at 7:40 AM Scott Bailey via Boost <
> boost_at_[hidden]> wrote:
>
>> # boost_sqlite review
>>
>> Please accept my apologies for the poor formatting. And a bit thank you to
>> everyone involved in this process. I have long wished for a sqlite wrapper
>> with critical mass.
>>
>
> Thank you for writing a review.
>
>
>>
>> ## Initial Thoughts and Use Case
>>
>> At the time I saw the announcement for this review, I was actively
>> evaluating libraries for a project I was (and continue to be) working
>> on. I successfully used the boost_sqlite library in a limited capacity
>> (create, insert, and pragmas only for on disk and in memory DBs).
>>
>> My first impression of the library is that it's extremely
>> lightweight. This simplicity has value but also leaves the end user
>> reinventing the wheel for certain repetitive operations. For example,
>> using class enums for pragma get and set moves defect finding from
>> runtime (bad) to compile time (better!).
>>
>
> That seems like a reasonable addition.
>
>
>>
>> My second thought was how painful it was to include in my professional
>> project. Our legacy software does not build Boost.json and, while it
>> was trivial to add it for my sandbox app, it is an unnecessary extra
>> dependency.
>>
>
> You are the second person to bring this up. I think making json support a
> header only addition would
> solve the dependency issue.
>

Yes, please. Or simply not adding json files and dependencies if
`BOOST_SQLITE_USE_JSON` or similar is "falsy".

>> ## Design Evaluation
>>
>> Simple, straight forward, and intuitive. I believe it meets my needs.
>>
>>
>> ### Wishlist
>>
>> Here are some wishlist items I have. They may be inappropriate for
>> this library and boost in general; however, they are still on my
>> wishlist.
>>
>> #### connection
>>
>> It would be great if connection had a simple enum so I could do this:
>>
>> ```cpp
>>    boost::sqlite::connection mydb(boost::sqlite::memory);
>> ```
>>
>> Also, given C++17 `std::fstream` takes `std::filesystem::path`, it
>> would be nice if `connection()` et all did as well. And I wouldn't
>> complain if `boost::filesystem::path` was supported as well.
>>
>
> I'll see if that can be added without including additional headers.
>

I suspect `boost::sqlite::memory` as a static member of const
some-string-type could be easily added to move defect finding for
in-memory-dbs from runtime to compile time.

Adding `xxx::filesystem::path` can always be added in a future revision.
Though `boost::iostream::file_descriptor` may have an example of how to do
it correctly. But I'm not a Boost dev, just a user.

>
>>
>> ## Implementation Evaluation
>>
>> I reviewed this library as a user.
>>
>>
>> ## Documentation Evaluation
>>
>> The readme.md file was adequate for me to quickly build a simple
>> SQLite DB application.
>>
>>
>> ## Potential Usefulness
>>
>> This is a useful library. I intend to incorporate this into
>> professional projects as soon as it becomes available in a stable
>> boost release.
>>
>>
>> ## Personal Usage
>>
>> I used the following compiler in my development:
>>
>>   gcc (Ubuntu 11.4.0-1ubuntu1~22.04) 11.4.0
>>
>> I included Boost and Boost.sqlite using CPM.cmake (fetchcontent). It
>> was a trivial operation to include and link.
>>
>>
>> ## Final Thoughts
>>
>> For the past few years I've lamented the lack of a Modern C++ SQLite
>> wrapper with a critical mass of user backing. This library has the
>> potential to fill that gap.
>>
>> I would like to see this library build without the sin dependency.
>>
>
> I assume you meant json here?
>

Yes, absolutely: json

>
>> With or without the build dependency change, this library should be
>> accepted into Boost.
>>
>
> Thank you.
>