Re: Conceptual issue in mysql++ 2.2.x (use(), store(), execute())

[Warren Young]

Michael Hanselmann wrote:
> 
>> I don't see any easy way to work around this short of rolling back the
>> null byte fixes in 2.2.0, which are far more important.
> 
> You refer to the "several Query objects from one connection" issue?

No, I didn't even realize this was part of the problem.  My previous 
response dealt solely with your use of Query::def.

> After looking into the code I realized that the C MySQL library supports
> only one query instance at a time, anyway. Shouldn't query() on a
> Connection object return the same Query instance on each call in that
> case?

How can MySQL++ "know" what use you are making of the Query objects? 
Maybe you have a legitimate need for them.  And even if not, how is it 
intuitive for the library to return the same object to two different 
requests, given that Connection::query() returns by value?  It's not 
like you can argue that it looks like a singleton factory.

This is a documentation issue, that's all.

>         mysqlpp::Query query = _conn.query();
>         query <<
> config().lookup<std::string>("global.mysql.query.jobs-by-user");
>         query.parse();
> 
>         query.def["owner"] = filter.owner();
> 
>         mysqlpp::ResUse res(query.use());

There's no reason in the world that you can't change the last two lines 
to this:

	mysqlpp::ResUse res = query.use(filter.owner());

This will work with the repository version of MySQL++, today.  The 
1-parameter limitation only affects Query::use(void), not 
Query::use(const SQLString&).  examples/tquery.cpp proves this.  The 
example works because it does not abuse Query::def.

>> The only way you can get me to relent would be to provide a patch that
>> supported your use while not breaking anything else, and I might
>> reject it for complexity reasons anyway.
> 
> The patch I provided in the first post of this thread "fixes" it by not
> allowing a single parameter to be passed as an argument. This is the way
> 2.1.x did it. 

No, the difference is that in v2.1 the call chain of Query::use() (and 
similar) had a much different organization.

These call chains used to end in an overload taking just a const char*, 
which doesn't allow embedded nulls.  Now they end in one taking both a 
const char* and an int, which does allow embedded nulls.

I could have left it at that, but if you pass your query in using a 
std::string, you'd get the old behavior again.  So, I made one more 
change, which is to overload the 1-parameter template query version of 
use() et al, which takes a const SQLString&, which derives from 
std::string.  This change was broken, and is now fixed in svn.

But, these changes also broke the single-parameter use of Query::def. 
This is a completely separate issue that I'm not going to spend any more 
effort to fix myself.  Once again, I am willing to consider patches from 
others on this issue, but won't accept them if they add significant 
complexity to this already complex area of MySQL++.  We don't need yet 
more confusion here, when the only benefit is to allow an abusive use of 
the library that isn't even necessary.

>> Ultimately, the problem is that we can't break the ABI right now.
> 
> The ABI isn't broken by the patch already provided, but the function's
> behaviour changes.

Yes, I know.  I rejected your patch because it removes functionality.

I referred to ABI breakage because if I had been free to break the ABI 
in v2.2, I wouldn't have done the changes the way I did them, thus 
preventing the confusion you're having now.  I knew this sort of problem 
could arise, but I decided it would be an acceptable consequence for an 
interim change.