Re: [review] [sqlite]

From: Klemens Morgenstern (klemensdavidmorgenstern_at_[hidden])
Date: 2024-11-14 03:21:32

>
> If nothing else, this serves as a good demonstration as to exactly how
> useless a Slack to ML gateway would be.
>

Here's a summary (i.e. it's edited) of the questions I answered
regarding this review so far:

Q: explicit connection(handle_type handle, bool take_ownership = true)
that reads inscrutably at the call site.
      I've taken the habit of using e.g. enum class HasOwnership :
bool; inline constexpr HasOwnership cTakeOwnership{ true }; ... so the
call site reads well.
      Bad idea? Is that something you'd consider?
A: Well I don't know, introducing a type for a single parameter seems
a bit overkill. You usually onl nee the take_ownership = false from
plugin code
     it's used here
://github.com/klemens-morgenstern/sqlite/blob/9de8655d7e26f9b5d5cb99c05e91b213d4a607a4/include/boost/sqlite/detail/vtable.hpp#L27

Q: In a similar vein, you've decided not to make flags type-safe? E.g.
int flags=SQLITE_OPEN_READWRITE|SQLITE_OPEN_CREATE
A: right, because the sqlite flags are good enough. i didn't want to
wrap everything and duplicate a bunch of code for little gain
     the idea is more that the library is an extension of sqlite into
C++, not a 100% wrapper

Q: What about opening a connection with a custom VFS? I've seen that
in our code-base. Did you get into that? Both allowing using a custom
VFS, and wrappers for VFSs similar to your vtable one?
A: nope. I thought about it but couldn't come up with an example that
was contrived. I might add it later if someone comes with a use-case

Q:  Why does the transaction not commit in the destructor if there is
no current_exception? It's a reasonable decision, but I'm curious.
A: This can for one lead to surprises if an early return happens.
Secondly, a COMMIT might fail, where as a ROLLBACK won't. Unhandled
errors in a destructor are a problem.