On Fri, 19 Apr 2013 10:26:12 -0300 Ricardo Fabiano de Andrade <ricardofabianodeandr...@gmail.com> wrote:
RFdA> Now let's talk about a potential solution. Hello, 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, but just in case it isn't, let me make some comments about this proposal: RFdA> 1) Backends may or may not participate in the log generation. RFdA> - I suggest the inclusion of virtual functions in *_use_type_backend RFdA> classes. In practice, right now, which backends would be doing something different from the default value-to-string conversion? 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. RFdA> - Returning [std::string] has a copying cost. I suggest returning [const RFdA> char*/std::size_t] But this supposes that you always have a buffer storing the pointer and that this buffer remains alive for long enough. Is this always a reasonable assumption to make, for all backends? E.g. Firebird backend doesn't convert values to strings at all AFAIK. RFdA> 2) use_type_base is the interface from where the core obtain the string RFdA> representation of the parameter values. 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. RFdA> - The implementation in standard_use_type only accepts index of 0. I'm not sure to follow you here, what does this mean exactly? RFdA> - Conversion probably will be based on [std::ostringstream]. 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. RFdA> 3) session class controls the parameter logging/reporting behavior. RFdA> - A new member function [set_log_params_length(std::size_t len, RFdA> bool expand)] enables parameter logging for any value greater than zero. RFdA> Second parameter means expand vectors. I unrepentantly hate boolean parameters, they're unreadable and inextensible. 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(). 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? 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. RFdA> - A new member function [set_last_params_length(std::size_t len, bool RFdA> expand)] enables parameter reporting for any value greater than zero. 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... RFdA> 4) statement_impl is the responsible for the concatenation of the parameter RFdA> values. The work happens in pre_use member function. RFdA> - It appends the parameter values obtained RFdA> from standard_use_type/vector_use_type in a [std::ostringstream] if RFdA> reporting is enabled (len ! = 0). RFdA> - it writes the parameter values to the log stream if parameter logging is RFdA> enabled (len ! = 0). RFdA> - It only iterates through vector_use_type values if expansion is enabled RFdA> for either reporting or logging (expand = true). RFdA> - [session::set_last_parameter] is called at the end passing the contents RFdA> of parameters' stringstream. This looks straightforward enough and I have no real objections to what happens in the explicit logging case (i.e. when the logging stream is set) but, again, 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? Thanks, VZ
pgpi5FTLr1nd9.pgp
Description: PGP signature
------------------------------------------------------------------------------ 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
