on_open is never called for in memory storage

[florianfischerx]

Hi,

assuming the standard way to obtain a sqlite3 handle in the FAQ (https://github.com/fnc12/sqlite_orm/wiki/FAQ)

storage.on_open = [](sqlite3 *db) {
    // do whatever you want except database closing
};

in combination with in-memory storage, the on_open lambda is never executed.

The reason is the following special treatment of in-memory in storage_base:

                pragma(std::bind(&storage_base::get_connection, this)),
                limit(std::bind(&storage_base::get_connection, this)),
                inMemory(filename_.empty() || filename_ == ":memory:"),
                connection(std::make_unique<connection_holder>(filename_)), cachedForeignKeysCount(foreignKeysCount) {
                if(this->inMemory) {
                    this->connection->retain();
                    this->on_open_internal(this->connection->get());
                }
            }

Basically, I see no way to have anything registered for on_open at this point already. Attaching it after the storage has been created has no effect, because, e.g.:

 void begin_transaction() {
                this->connection->retain();
                if(1 == this->connection->retain_count()) {
                    this->on_open_internal(this->connection->get());
                }
                auto db = this->connection->get();
                perform_void_exec(db, "BEGIN TRANSACTION");
            }

So, retain_count is already 2 (for the in-memory case) and on_open_internal is never called (which makes sense).

Is this on purpose / by design? One option would be to provide the on_open handler directly for make_storage, but in general, I am not sure if the on_open semantics make sense for the in-memory model (which by definition is just ... open). Thoughts?

[fnc12]

Hi. Yes what you are saying is right: things turn out a way when you never get on_open called cause it is attempted to be called in ctor. How to fix it? We need to think about. What is the purpose of getting sqlite3* handle?

[florianfischerx]

Ok, what I am doing is maybe slightly odd: I am using SQLite / in-memory sort of a rule engine for sensor data. So we capture sensor data for some time and periodically run specific queries to trigger actions.

Those queries are either predefined (for which SQLite orm is great) or are supplied as external input/user-defined. Naturally, the second case cannot be handled via statically typed queries at compile time (think a pre-validated but pretty generic WHERE clause). So, it's really this second edge-case that requires access to the sqlite3* handle.

Right now I have just worked around that by adding a utility function to storage_base.

The other option I have thought about would be to allow something like

where("User.firstname = 'Tom'")

instead of

where(c(&User::firstName) == "Tom")

And that compiles just fine, but the resulting prepared statement is wrong. I think this goes wrong somewhere in the handling of where_t / expression_type (which is just const char* in that case).

Edit: Basically, it results in something like

SELECT <stuff> FROM ''Users'  WHERE (?)

This makes sense, because i) I am really not supplying anything that fits as a prepared statement, ii) kinda breaks the typed API.

[fnc12]

And that compiles just fine, but the resulting prepared statement is wrong.

Lol yes. You pass a string, string is a valid SQL expression. It makes a query like SELECT ... WHERE 'User.firstname = \'Tom\'' which doesn't mean anything meaningful. I want to offer you a different way of creating a query. You can still use statically defined query. I mean it!

You can create a universal query with all available options chained with bool flags. Please take a look at this issue https://github.com/fnc12/sqlite_orm/issues/272. Guy there was sure that he can only use dynamic queries but I told him how to use static queries. It allows to 1) omit obtaining sqlite3* 2) using prepared statements which can be reused to increase runtime

[florianfischerx]

Hi, yes, I have seen that suggestion, and it absolutely works if you know (roughly) the structure of the WHERE clause, e.g. when you do dynamic filtering on a known max. number of conditions.

It does not work in the case of an example roughly such as:

table { Field A, Field B, Field C }

for which all the following are possible conditions:

• WHERE A = 'somevalue' AND C = 'someothervalue AND B in ('X', 'Y', 'Z)
• WHERE A = 'somevalue' AND C != 'someothervalue'
• WHERE (A = 'somevalue' AND C != 'someothervalue' ) OR B = 'someothervalue' etc.

Basically, the query is (optionally) constructed in an external query builder, validated against a shared schema, and then executed (for some cases).

I think it is fundamentally not possible to prepare for that at compile time.

[fnc12]

Yes such a situation is not designed to be handled by sqlite_orm. I'd advice you to change dynamic to static in any way. But if you want to leave it like that - it is up to you

[florianfischerx]

Ok, I will look into this :-)

Would it make any sense to add special handling for in-memory cases tho? I would be willing to prepare the code changes if you have any suggestions on how to go about this. Otherwise, we can just close this :-)

[fnc12]

I'd like to fix this issue of course. But I don't know how to do it. Actually the thing is that when you set on_open connection is already opened and will never be reopened again. If on_open setting would be implemented with a function not a member then I could add a check if database is in memory then call the argument at once. But it cannot be implemented it right now in that way. Maybe we can create a different algorithm. What do you think? Do you have any ideas?

[florianfischerx]

Hmm, the only reasonable way to do that would be to pass the std::function<void(sqlite3*)> ; to storage_base on construction. Maybe there could be a make_memory_storage function to encapsulate that. Is that not feasible?

[fnc12]

I guess we can fix it without creating make_memory_storage. make_storage accepts variadic templates pack. We can assume that the first element can be decltype(on_open) and if so we will not count it as a column but will pass it to storage_base ctor. This is how it will be called for you:

auto storage = make_storage("path.sqlite", [](sqlite3* db) {
    // handle db here
}, make_table(...), ...);

What do you think?