Hi Vadim,
I'm not sure if this is still relevant, it seems that you've already
> implemented the proposed changes, so maybe it's too late,
Not, it isn't. After all your comments and suggestions I consider my pull
request a WIP.
> but just in case
> it isn't, let me make some comments about this proposal:
>
> Please do. :)
>
> In practice, right now, which backends would be doing something different
> from the default value-to-string conversion?
Postgres and MySQL which work over text protocols and need to convert all
parameter values to text before sending them to the server.
> This seems like something that
> could be added later if really needed and while I do understand that it
> might be needed, I'm not entirely convinced right now.
>
>
You're right. If the backend provides some form of string conversion for
any value it can used (and it don't need to be done for all supported
types, e.g. ODBC backend could provide only long long conversion to string
when available, take a look).
Otherwise, you can stick with the default implementation in core.
> But this supposes that you always have a buffer storing the pointer and
> that this buffer remains alive for long enough.
As far clean_up() doesn't get called you're good to go.
> Is this always a reasonable
> assumption to make, for all backends? E.g. Firebird backend doesn't convert
> values to strings at all AFAIK.
>
Only for backends which convert the parameter values to string.
Others use the default core implementation which don't have such lifetime
limitations.
> Yes, this looks logical to me, I had suggested adding (pure virtual)
> as_string() to it in my initial post and I still don't see any better way.
>
I named it differently but I agree. I also don't mind to change names if
they're more meaningful.
> I'm not sure to follow you here, what does this mean exactly?
>
>
[use_type_base::as_string] must be implemented by both standard and vector
variations.
vector implementation needs an index (which element of the vector do you
want to see as string?)
standard implementation (with the same signature) doesn't care about index
but calling it with any non-zero value is a bug.
> I'd avoid ostringstream in anything where performance might be a factor.
> Perhaps some implementations of it are fast enough but in general it is
> rather slow and plain old ssprintf() is generally preferable.
>
I agree and I used snprintf() in my implementation taking advantage of an
existing buffer. Less a couple of copies.
> I unrepentantly hate boolean parameters, they're unreadable and
> inextensible.
I'm glad you understood the reason of the options even using the bool
notation.
And you're right they're ugly but honestly, I wrote in the mail what came
first to my mind (what I don't do while coding).
> So I'd rather use either a separate function
> set_log_full_vector_contents(bool) (boolean arguments are fine for the
> functions taking a single parameter only where its meaning is clear) or, if
> we really want to combine everything in a single function call, use some
> logging_options helper class with methods like set_max_length() and
> enable_full_vectors().
>
I must agree. And logging_options class is a nice idea.
> But I also have more semantic concerns about this function. First, I'm not
> sure how useful is it to specify the maximal length in characters. How
> would you decide which length to use here, exactly?
As a library developers sometime we face such questions.
No, we really can't specify a valid limit for maximal length in characters.
But I still think it's valid a solution having all the parameters but
limited to a certain length.
Thinking about it again, the default should be the whole length but user
could be able to limit that (mostly applied to strings).
If you read Mateusz' last comments on this subject he suggested giving the
user the decision of how to format all this information.
Maybe he's right, I need to make too many assumptions to print a single log
line.
The idea of a pair soci::converter / soci::backend_converter just crossed
my mind.
> IMO enabling or
> disabling the logging can be done quite naturally simply by setting or
> resetting the log stream, just as your initial patch did. OTOH I do think
> it might be nice to specify the number of elements you want to be logged
> for the vectors rather than only having the choice between 0 and INT_MAX.
> E.g. 1, 2, 3 or even 10 could all be useful here.
>
I think both are interesting.
> I'd really like last parameters saving be efficient enough to never have
> any reason to disable it. Then we could get rid of this function...
>
Actually, the length thing is a usability stuff.
It can be faster because it's copying less bytes but it isn't the main goal.
> I hoped that we could postpone conversion to string until
> get_last_query() is really called in the other case. Do you see if we could
> do this?
>
> It's what you will get. I'm working on that.
Regards,
Ricardo
------------------------------------------------------------------------------
Precog is a next-generation analytics platform capable of advanced
analytics on semi-structured data. The platform includes APIs for building
apps and a phenomenal toolset for data science. Developers can use
our toolset for easy data analysis & visualization. Get a free account!
http://www2.precog.com/precogplatform/slashdotnewsletter
_______________________________________________
soci-devel mailing list
soci-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/soci-devel
