Skip to content

Support for named and indexed parameters #170

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Sep 10, 2018
Merged

Conversation

zauguin
Copy link
Collaborator

@zauguin zauguin commented Jun 23, 2018

This is a UDL based attempt to support binding named parameters: For example

db << "INSERT INTO t VALUES(:firstname, :lastname, :birthday);"
   << ":birthday"_sqlparam("2000-01-01")
   << ":firstname"_sqlparam("John")
   << ":lastname"_sqlparam("Doe");

I am not really happ with the name _sqlparam. Does somebody have a better idea?
Another question is how to handle mixed positional and named parameters: Currently _sqlparam paameters are ignored in the normal order, so for example

db << "SELECT :abc, ?;" << ":abc"_sqlparam(1) << 2;

would bind both parameters, 1 and 2, to :abc so the result would be 2, NULL.
This is not perfect but I do not know about a better way and using both named and ? parameters together is advised against in the SQLite manual anyway, so fixing this might not be the top priority.

@zauguin zauguin requested a review from aminroosta June 23, 2018 11:05
Copy link
Collaborator

@aminroosta aminroosta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@zauguin The implementation looks good to me.

I'm just wondering why would anyone prefer the named parameters instead of ? given our current api interface ?

@zauguin
Copy link
Collaborator Author

zauguin commented Jun 27, 2018

@aminroosta The motivation are prepared statements where the parameters are not bound immediately after creating the statement.
For example I have a function which accepts a prepared statement as a parameter. During the execution the stament is executed multiple times with different values. The ? syntax forces the caller to use the parameters inthe statement in a order implicitly defined by the callee. Also changing the implementation such that the parameter values are known and bound in a different order becomes a API breaking change.

The caller can create a work-around by using the ?1, ?2, ... syntax but at this point the number already acts like a name and no longer has a real meaning. So I think it is useful to allow descriptive names too.

This also gives more flexibility to the implementation of the callee.

Also the SQLite documentation says about ? parameters: "the use of this parameter format is discouraged". The reasons do not apply for typical uses of the library, because counting the question marks is not necessary, but especially for cases where SQL statements are shared between code in different languages some support for named types might be a good idea.

This becomes even more useful in combination with #169.
On the other hand these changes but can be seen as a too big change in the fundamental design of the current API.

@aminroosta
Copy link
Collaborator

aminroosta commented Jun 28, 2018

@zauguin You make a really good point, I also agree :-)

Let's talk about the interface, I will propose to use template character packs template<char ...c> (or template<char const *>) and have an interface like this:

db << "INSERT INTO t VALUES(:firstname, :lastname, :birthday);"
   <<  named_parameter<":birthday">("2000-01-01")
   << named_parameter<":firstname">("John")
   << named_parameter<":lastname">("Doe");

I have not thought about the details of implementation but given we have class template argument deduction it should be possible to implement in C++17.

What do you think?

@zauguin
Copy link
Collaborator Author

zauguin commented Jun 28, 2018

@aminroosta Literal strings are not valid non-type template arguments, so AFAICT syntax like named_parameter<":birthday"> can not be implemented. There is a proposal to change this for C++20, P0424, but in C++17 we have to live without it.

We could create a function with two regular parameters:

db << "INSERT INTO t VALUES(:firstname, :lastname, :birthday);"
   << named_parameter(":birthday", "2000-01-01")
   << named_parameter(":firstname", "John")
   << named_parameter(":lastname", "Doe");

@aminroosta
Copy link
Collaborator

@zauguin Thanks for the proposal link, yea makes sense.

Between the two syntaxes ":birthday"_named_parameter("2000-01-01") and named_parameter(":birthday", "2000-01-01"), i slightly prefer the function version ... but both look good to me, i leave it to you to choose.

@zauguin
Copy link
Collaborator Author

zauguin commented Jun 28, 2018

Let's use the function then, this also allows non-literals. Then I would use indexed_parameter for the numbered version.

@zauguin zauguin merged commit d6f4070 into SqliteModernCpp:dev Sep 10, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants