[SQLite] Should running a parameterized update statement with execute be possible?

[margaretselzer]

I have the following update statement:

auto u =
    sqlpp::update(t)
        .set(t.checksum = sqlpp::parameter(t.checksum))
        .where(sqlpp::verbatim<sqlpp::unsigned_integral>("ROWID")
               == sqlpp::parameter(sqlpp::unsigned_integral(), sqlpp::alias::i));

auto u_stmnt = db.prepare(u);

Running this query like this: db(u_stmnt); works perfect.

However, using db.execute(u_stmnt);
results in compile time error: no matching function for call to ‘serialize(const sqlpp::prepared_update_t<sqlpp::sqlite3::connection_base, sqlpp::statement_t<void, sqlpp::update_t, sqlpp::single_table_t<void, MyTable>, sqlpp::update_list_t<void, sqlpp::assignment_t<sqlpp::column_t<MyTable, MyTable_::field_checksum>, sqlpp::parameter_t<sqlpp::unsigned_integral, sqlpp::column_t<MyTable, MyTable_::field_checksum> > > >, sqlpp::where_t<void, sqlpp::binary_expression_t<sqlpp::verbatim_t<sqlpp::unsigned_integral>, sqlpp::op::equal_to, sqlpp::parameter_t<sqlpp::unsigned_integral, sqlpp::alias::i_t> > > > >&, sqlpp::sqlite3::connection_base::_context_t&)’

Now, as I am running an update I really do not need any kind of result back so my gut tells me that this should work with execute(), but it does not.

Is my gut talking nonsense or - against all odds - it does have a point?

[rbock]

Thanks for the question. As of today, the db(u_stmnt); is the correct way.

That said, your gut has a point: It should probably be allowed to do db.execute(u_stmnt);. Do you want to try preparing a PR for that?

[margaretselzer]

I'd be glad to help out with that, but then you need to tell me where I should intervene. Based on the error message I guess that a new overload of serialize() is required, but then the same query can be serialized via the "non-execute" way, so it might be execute () that needs tweaking?

If you think that even after revealing my ignorance I can be useful, let me know how and I will put that together. Then hopefully the next time I will need less hand-holding... ;)

[rbock]

You pretty much answered your questions yourself :-)

I would try to add another overload for sqlite3::connection::execute for anything that fulfills is_prepared_statement_t<T>::value == true.

[margaretselzer]

Will give it a try sir! :)

[margaretselzer]

I have the patch ready, but I seem to be unable to push my changes: do not have permissions. How is one supposed to submit a PR here?

git push -u --recurse-submodules=check --progress "origin" refs/heads/issue_596:refs/heads/issue_596
remote: Permission to rbock/sqlpp11.git denied to margaretselzer.
fatal: unable to access 'https://github.com/rbock/sqlpp11.git/': The requested URL returned error: 403
Done

Update
Tried with SSH as well, can clone the repo, but have the same permission denied issue when trying to push a new branch.

git push -u --progress "origin" refs/heads/issue_596:refs/heads/issue_596
Enter passphrase for key 'xxx':
ERROR: Permission to rbock/sqlpp11.git denied to margaretselzer.
fatal: Could not read from remote repository.

Please make sure you have the correct access rights
and the repository exists.
Done

[MeanSquaredError]

@margaretselzer

You need to :

• Fork the sqlpp11 project on GitHub, using the fork button in the upper right part of the screen.
• Setup the git credentials for your forked copy of sqlpp11. See below.
• git clone https://your-forked-sqlpp11-repository
• Make the changes to your local (cloned) copy of sqlpp11
• git push your local copy of sqlpp11. This will update your forked copy of sqlpp11 on GitHub.
• Go to the GitHub page for your forked copy of sqlpp11. Then click on the [Contribute] button to make a Pull Request.

The location of git credentials varies on different system, but on Fedora Linux they are in

In ~/.config/git/config there are a couple of relevant sections

...
[user]
	name = Your_Name
	email = The_NoReply_email_provided_by_github
...
[credential]
	helper = store
...

In ~/.config/git/credentials there should be one line:

https://Your_Name:Your_Auth_Token@github.com

Your_Name and Your_Auth_Token are the credentials provided by GitHub.

[margaretselzer]

@MeanSquaredError thanks for the detailed guide, really appreciated.

I am used to the [dev branch from main] --> [do your changes in dev branch] --> [create PR from dev branch] --> [merge dev branch back to main branch] approach, but one learns something new every day. Now I do not have to wonder why are there so many forks of this project. :-)

[MeanSquaredError]

@margaretselzer

Indeed the way most people commit changes to the upstream/original project is by making changes to a separate local branch, then pushing that branch to their own fork and then creating a PR to merge their branch to the upstream. This approach would work in your case too, but for brevity I omitted the step where you create a separate branch for your changes.

The critical thing which caused the push error in your case is that you are only allowed to push to github repositories for which you have the necessary permissions. In most cases these are your own projects (either the ones that you created or your forks of 3rd party projects) for which you have the corresponding permissions and your have configured the auth credentials in your git client.

That is why you can push to your forks of 3rd party projects (in this case sqlpp11) as long as you get your auth credentials and configure them in your git client. However you cannot push to the 3rd party projct (in this case sqlpp11), because your auth credentials don't have the permissions for that.

[margaretselzer]

@MeanSquaredError

I get the concept, it is the "own fork" part that was new to me as it feels a bit unnecessary. Limiting the main branch to not accept direct commits (to outsiders at least) should be enough because that way the only way in to main is via a PR. But I see now that this is the GitHub way of doing things so learning and adapting... After you spelled it out for me it was a rather straightforward process. I hope the PR looks ok. :)

[margaretselzer]

I wonder, is this gonna make it to sqlpp11 or sqlpp23 ?

[MeanSquaredError]

@margaretselzer
sqlpp23's connection classes have three overloads for operator(), that allow the connection object to be called with either of these

• Statement object
• String containing SQL statement(s).
• Prepared statement

For example the PostgreSQL connector's overload that supports prepared statements
https://github.com/rbock/sqlpp23/blob/8673f3da74896b870f66a27fda40a0f856168d9e/include/sqlpp23/postgresql/database/connection.h#L280

So probably this fulfills your needs?

(On the other hand, the execute() method is no longer available, but it is not really necessary given that the three overloads exist).

[margaretselzer]

Thanks for the quick response.
Yeah, from the little code I saw of sqlpp23 I suspected that execute() was gone. A pity as I think it is good to be able to clearly distinguish between a query() that returns a result set and execute() that returns only number of impacted records. On the other hand one can always write a wrapper... ;)

[rbock]

The library has gone through a number of iterations with the run() method, the operator(), the execute() function for strings, and finally the query and execute function depending on the statement type.

These were all reasonable steps on the way...

However, sqlpp23 offers the chance to make some breaking changes, see https://github.com/rbock/sqlpp23/blob/main/docs/differences-to-sqlpp11.md (and I just realized the I still need to write about execute). These changes overall make the library substantially simpler and will allow for new features that I just wouldn't want to add to sqlpp11 because they are complicated.

Having just one way to execute statements is simpler, too. For instance, the current logic in sqlpp11 uses is_select_t in a static_assert. That breaks for postgresql's insert...returning.... This would require a bit of logic duplication to get right.

And frankly, I don't really see the benefit of clearly distinguish between a query() that returns a result set and execute() that returns only number of impacted records? When I write something like

db(insert_into...);

Then the insert_into clearly says what it is? similar with other statements?

Sure, in sqlpp11 you could argue that dynamic or prepared statements are more difficult and you could have something like

auto x = dp.prepare(select(...)...);
...
db(x); // <--- hard to tell what this does...

But that's just bad naming?

And note: in SQL, you also don't say things like

EXECUTE INSERT INTO tab sample ...;
QUERY SELECT id FROM tab_sample;

And it is still clear from the statement what to expect.

Anyways, that's my current thinking and yes, you could totally write a wrapper :-)

[MeanSquaredError]

@margaretselzer

A pity as I think it is good to be able to clearly distinguish between a query() that returns a result set and execute() that returns only number of impacted records.

The three overloads of operator() are in fact three different functions that just happen to share the same name :-)

(well, in reality it is one string overload + two templated overloads, so it would be more precisely to say that each instance of operator() can be put into one of the three big groups based on the type of the function parameter, string, statement, or prepared statement)

But in reality the compiler easily distinguishes them by the type of their first (and only) parameter. So in the place where you call operator() (most likely it will be some function), you probably already know the type of your parameter, so you know the type of the operator(), that you will call. If you calling operator() from a function template, then you could use if constexpr to distinguish between the three types of operator(). For example

template <typename dbc_type, typename param_type>
void myfunc(dbc_type& dbc, param_type& param)
{
    ...
    if constexpr(std::is_same_v<std::remove_cvref_t<param_type>, std::string_view>) {
        // Calling  the string overload
        dbc(param);
    } elseif constexpr(sqlpp::is_statement_v<param_type>) {
        // Calling  the statement overload
        dbc(param);
    } elseif constexpr(sqlpp::is_prepared_statement_v<param_type>) {
        // Calling  the prepared statement overload
        dbc(param);
    } else {
        // Something unexpected happened!
        throw std::runtime_error{"Unexpected type"};
    }
}

(I am not expert in template metaprogramming, but I think I got it mostly right)

However, in the couple of small projects that I ported to sqlpp23, I never actually needed to know the type of the operator() that I am calling. But most cases it was either

dbc("SOME_UNSUPPORTED_SQL_COMMAND");

or some statement that I built in-place, e.g.

dbc(select(all_of(my_tbl)).from(...).where(...));

and these are really easy to distinguish visually just by looking at them. And I never had the need to distinguish them programmatically in my code, using if constexpr, or some other similar trick.

[margaretselzer]

Thank you both for the quick feedback. It is really great to be able to count on getting a reply. :)

Judging from the differences table it seems that we have a philosophical difference in how we approach coding and specifically overloading operator(). You seem to put a lot of emphasis on brevity, while I need to make an effort to not comment my code too much. You do not document the classes, functions in the library (meaning comments in the code), I do. :)

So from my viewpoint using operator() in the interface is already something that is too much simplification because it does not clearly communicate the intent. But now we even have overloads for the same so it is even more ambiguous what they do. While I fully understand your thinking behind this choice, I just simply prefer the code to "speak" for itself as much as it can.

One of the primary goals when overloading operator() is to create a functor, e.g. to create an object that behaves like a function. For me a "database" as a concept is much more than just a function that merely executes statements. I know that in all examples the db really represents a connection to a database so thinking about it as a functor is probably easier that way, but still, for most who uses this library that connection IS the database. And so execute() and query() just makes the code more readable. It also communicates that db is an object and not a function.

So distinguishing the two is not about limiting for example query() to not be able to run an insert (thinking of postgresql's insert...returning... here), but the opposite, to make it clear that although our SQL starts with an insert it will return records.

Most of the sqlpp template magic still represents a mystery to me so I am unable to go more deep, but for me an ideal solution would be if running query() could check what the return type of the statement is. If it is not some kind of record structure (including a single field of a record) then it would not compile. Similarly, execute() would not compile if it was returning a record structure. But even if these kind of compile time checks are not possible, using these two functions would make the code more readable, the intent clearer resulting in code that is easier to understand and maintain in the long run.

I would probably just drop operator() altogether or at least make query() and execute() available for the "chatterbox types" like myself. But whatever you decide, the library is already GREAT as is. :)

P.S.: One thing that I failed to mention is that I use the library most with prepared statements so the structure of the queries I run is mostly hidden when I run them. So that probably also gives a bias towards what I see as the ideal solution.