On 07/04/2016 03:30 PM, j...@conductive.de wrote: > > Quoting wie...@porcupine.org: > >> j...@conductive.de: >>> >>> Quoting wie...@porcupine.org: >>> >>> > Wietse Venema: >>> >> Joel Linn: >>> >> > Why is it chosen to "not support stored procedures" instead of >>> adding two >>> >> > lines of code? >>> >> >>> >> The original mysql client may well have been written at a time that >>> >> stored procedures did not exist, or no API documentation existed >>> >> for how to do this correctly. >>> >> >>> >> If you feel strongly about stored procedures, then you are welcome >>> >> to contribute code. If you can demonstrate that the code uses the >>> >> MySQL API correctly (instead of "there are no error messages") then >>> >> that would help getting the code accepted. >>> > >>> > This is a pointer to MySQL documentation for multi-statement support, >>> > which seems to be needed to implement stored-procedure support in the >>> > Postfix MySQL client. >>> > >>> > http://dev.mysql.com/doc/refman/5.7/en/c-api-multiple-queries.html >>> > >>> > Wietse >>> >>> Thanks for the link. >>> An interesting fact is that the doc says CLIENT_MULTI_RESULTS is >>> default since MySQL 5.7. >>> And comparing my error message "Commands out of sync" to the one >>> in the discussion of 2008 John Fawcett dug out "can't return a >>> result set in the given context", we are in fact not using the api >>> as designed at the moment. Sure its "just" an error message for >>> both cases but the api thinks we can handle multiple results but >>> in fact we can not. Talking about multi statement support is >>> planning one step ahead already, but a smart thought whatsoever, >>> eliminating the need for stored procedures to some extend. >>> >>> I think we could at least support multi results by enumerating >>> over all but the first result (return that) and maybe throw them >>> away, at least issue a warning if they are additional result sets >>> (not for statuses of course, they are OK if they indicate success). >>> This way we can indicate to the administrator a poorly designed >>> query that leaks data. Maybe even issue an error for security >>> reasons if the results contain more than one result set. Because >>> that could lead to undefined behavior if some SELECT prior to the >>> correct SELECT accidentally gives wrong data (e.g. sending mail >>> to mars). >>> >>> Multiple statements is another discussion, however subordinate and >>> dependent on the implementation of multi results. As far is I can >>> see no prepared statements are used in the code so allowing for >>> multiple statements is easy and shouldn't tension the security >>> discussion (?), however sql injection is easier to do if you can >>> end a statement with ";". But I presume the % placeholders are >>> filtered anyway (either explicit or implicit) so that shouldn't >>> be a concern. >>> >>> Do you agree or do you see any drawbacks or problems? >> >> Unfortunately, I am not familiar with MySQL APIs, and I don't have >> cycles to spare. >> >> My first priority is to ensure that existing Postfix code keeps >> working as promised (i.e. Postfix does not break due to MySQL API >> library changes). >> >> If the MySQL library will send multiple results even if Postfix >> does not specify CLIENT_MULTI_RESULTS, it is sufficient to report >> an error when the library returns more replies than expected? >> >> I'm not opposed to stored-procedure support, but I think it should >> be implemented+documented by someone who understands the APIs, so >> that would not be me. >> >> Wietse > > At the moment no flags (0) are passed to mysql_real_connect. > But if using the mysql api version 5.7 or newer the api "forces" > CLIENT_MULTI_RESULTS. > I see no way in the api doc to disable it, e.g. using > mysql_set_server_option. > So the code should no matter what cycle over the results anyway, > because if not cleared they cause later queries to fail and > we have no way to prohibit multiple results using the api afaIk. I don't see how to disable CLIENT_MULTI_RESULTS either. However I believe this is not a problem if you use a single query or are not using stored procedures. > > So yes, looping results and generating an error for more than one > result is sufficient to prevent hiccups in following queries. > Is this necessary? An error should already be generated if you try to use stored procedures or multiple queries. > That bugfix however would be very close to my suggested change. > Is there someone maintaining the mysql portion of postfix, > I mean except you keeping everything running? > > Joel > I can propose a code submission to add stored procedure support (based on the proof of concept code from 2008), but the biggest part will be doing the testing and non regression testing not the actual coding.
I believe the best approach to adding stored procedure support is to iterate over multiple result sets and if there are exactly two and the last one is the status result of the stored procedure and it is successful then allow the lookup against the first result set. In all other cases there should be an error for multiple result sets. In the context of Postfix don't think it is useful to have more than one result set generated by multiple queries or more than two result sets (including the status one) from a stored procedure. John
