Re: [review] [sqlite]

From: Alexander Grund (alexander.grund_at_[hidden])
Date: 2024-11-21 09:19:00

> 1. I've found myself running into lifetime issues with code that
> looked well-formed. At one point, I had the following:
>
> sqlite::static_resultset<order_with_items>
> get_order_with_items(std::int64_t order_id)
> {
>      sqlite::statement stmt = conn.prepare("SELECT ...");
>      return conn.execute<order_with_items>({order_id});
> }
>
> This is undefined behavior (as stated in the docs), and needs to be
> rewritten as:
>
> sqlite::static_resultset<order_with_items>
> get_order_with_items(std::int64_t order_id)
> {
>      return conn.prepare("SELECT ...")
>          .execute<order_with_items>({order_id});
> }
Is the 1st example supposed to be `return stmt.execute<...`?

Answered by Klemens as:

> Do you think this could be solved with ref-qualified overloads, i.e.
> .execute() && would transfer ownership, while .execute() & would not?
> That would seem intuitive to me.
 From what the examples look like this already seems to be the case. 
Otherwise there would be no difference between the 2. Or am I missing 
anything?

I also agree that those 2 code examples look like they would be the same 
behavior and people would likely just use the first and expect it to work.
I.e. from the code alone it is not clear that there is an issue and  it 
can be easily overlooked in code reviews.
Is it possible to use shared ownership here, such that the resultset in 
example 1 remains valid after the return?

If shared_ownership is not possible, could a weak ownership we used? 
Example 1 would lead a segmentation fault right now. If the library 
could detect that the `sqlite3_stmt`was deleted at this point and report 
that it would make it easer to understand.
This is especially dangerous as the destructor of the resultset triggers 
the segfault too.