On 12/18/2016 09:38 PM, John Fawcett wrote:
> On 12/18/2016 02:09 AM, Wietse Venema wrote:
>> What if Postfix made an old-style query? I think it should just
>> report the old-style error here.
>>
>> Wietse
> I agree. It might be as simple as changing
>
> msg_warn("dict_mysql: stored procedure did not return any result set
> containing data");
>
> to
>
> msg_warn("mysql query failed: %s", mysql_error(host->db));
>
>
> But I think I need to do some tests with unmodified postfix and then compare
> behaviour to the patched version to make sure the warnings are really the
> same.
>
> John
>
I have revised the patch and changed the logic. The error messages are
now in line with the exisiting behaviour for queries that return
results, queries that return no results and for queries that have
errors. There is one minor difference in error messages in the case of a
query that contains no SELECT. The result returned by mysql is
indistinguishable from a stored procedure that contains no SELECT and as
I am explicitly setting errno to ENOSUP, the message given for the query
would now be:
postmap -q j...@erba.tv mysql:/etc/postfix/test_update.cf
postmap: warning: mysql query failed:
postmap: fatal: table mysql:/etc/postfix/test_update.cf: query
error: Operation not supported
instead of the existing
postmap -q j...@erba.tv mysql:/etc/postfix/test_update.cf
postmap: warning: mysql query failed:
postmap: fatal: table mysql:/etc/postfix/test_update.cf: query
error: Success
Queries containing errors continue to have errno 0 as currently
postmap -q erba.tv mysql:/etc/postfix/test_query_error.cf
postmap: warning: mysql query failed: Unknown column 'domains' in
'field list'
postmap: fatal: table mysql:/etc/postfix/test_query_error.cf: query
error: Success
though I would opt to set it in this case too.
This version of the patch includes some documentation updates to clarify
that the stored procedure must execute exactly one SELECT statement. The
code has been modified for the revised error reporting and readability.
--- postfix-3.2-20161204_orig/proto/mysql_table 2016-10-01
15:44:52.000000000 +0200
+++ postfix-3.2-20161204/proto/mysql_table 2016-12-25
09:04:09.230528031 +0100
@@ -289,6 +289,52 @@
# certificate.
# .sp
# This parameter is available with Postfix 2.11 and later.
+# USING MYSQL STORED PROCEDURES
+# .ad
+# .fi
+# With Postfix 3.2 it is possible to call a stored procedure
+# instead of using a SELECT statement in the query, e.g.
+#
+# .nf
+# query = CALL lookup('%s')
+# .fi
+#
+# The preivously described '%' expansions can be used in the
+# parameter(s) to the stored procedure.
+#
+# The stored procedure must return data with a single result
+# set, that is the stored procedure must execute exactly one
+# SELECT statement on every code path. If you have complex
+# logic and for some paths you want to return no result you
+# will need to include a SELECT statement that returns no
+# rows. One example of a query that returns no rows is
+#
+# .nf
+# SELECT 1 FROM DUAL WHERE FALSE
+# .fi
+#
+# but you may use your own query.
+#
+# Stored procedures that return multiple result sets containing
+# data, that is stored procedures that execute multiple SELECT
+# statements, are not supported. Stored procedures in
+# mysql produce an additional result set without data which
+# indicates the final status of the stored procedure. If this
+# final status is an error then any previous returned data
+# will not be used.
+#
+# The following is an example of a stored procedure returning
+# a single result set containing data:
+#
+# .nf
+# CREATE [DEFINER=`user`@`host`] PROCEDURE
+# `lookup`(IN `param` VARCHAR(255))
+# READS SQL DATA
+# SQL SECURITY INVOKER
+# BEGIN
+# select goto from alias where address=param;
+# END
+# .fi
# OBSOLETE QUERY INTERFACE
# .ad
# .fi
diff -ur postfix-3.2-20161204_orig/src/global/dict_mysql.c
postfix-3.2-20161204/src/global/dict_mysql.c
--- postfix-3.2-20161204_orig/src/global/dict_mysql.c 2016-09-25
17:27:11.000000000 +0200
+++ postfix-3.2-20161204/src/global/dict_mysql.c 2016-12-24
19:56:24.967466042 +0100
@@ -187,6 +187,7 @@
#include <time.h>
#include <mysql.h>
#include <limits.h>
+#include <errno.h>
#ifdef STRCASECMP_IN_STRINGS_H
#include <strings.h>
@@ -519,6 +520,10 @@
{
HOST *host;
MYSQL_RES *res = 0;
+ MYSQL_RES *temp_res = 0;
+ int status=0;
+ int num_result_sets=0;
+ int query_error=0;
while ((host = dict_mysql_get_active(dict_mysql)) != NULL) {
#if defined(MYSQL_VERSION_ID) && MYSQL_VERSION_ID >= 40000
@@ -536,9 +541,56 @@
#endif
if (!(mysql_query(host->db, vstring_str(query)))) {
- if ((res = mysql_store_result(host->db)) == 0) {
+ query_error = 0;
+ num_result_sets = 0;
+ res = 0;
+ do {
+ temp_res = mysql_store_result(host->db);
+ /* did statement return data? */
+ if (temp_res) {
+ num_result_sets++;
+ /* only use the first result set that returns data*/
+ if (num_result_sets == 1) {
+ res = temp_res;
+ } else {
+ /* free up any additional result sets that return data in order
+ to avoid memory leaks */
+ mysql_free_result(temp_res);
+ temp_res = 0;
+ }
+ } else {
+ /* statement did not return data, check if there were errors.
+ mysql_field_count returns 0 if no data should be expected.
+ In the case of a stored procedure this is ok as the last
result has no
+ data. In Postfix we only want to treat this result as
correct if there
+ is only one previous result set with data.
+ For all other cases treat as an error, i.e.
+ - query or stored procedure without SELECT: mysql_field_count 0
+ and num_result_sets 0
+ - stored procedure with more than one result set:
num_result_sets > 1
+ - query or stored procedure that should have returned data but
+ did not: mysql_field_count >0 */
+
+ if (mysql_field_count(host->db) == 0 && num_result_sets == 1) {
+ if (msg_verbose)
+ msg_info("dict_mysql: successful final result for
stored procedure");
+ } else {
+ query_error=1;
+ }
+ }
+ /* more results? -1 = no, >0 = error, 0 = yes (keep looping) */
+ if ((status = mysql_next_result(host->db)) > 0)
+ msg_warn("mysql query failed (mysql_next_result):
%s", mysql_error(host->db));
+ } while (status == 0);
+ if (status != -1 || query_error || num_result_sets != 1) {
msg_warn("mysql query failed: %s", mysql_error(host->db));
+ if (num_result_sets > 1)
+ msg_warn("dict_mysql: multiple result sets returning data
are not supported");
plmysql_down_host(host);
+ errno = ENOTSUP;
+ if (res) {
+ mysql_free_result(res);
+ res = 0;
+ }
} else {
if (msg_verbose)
msg_info("dict_mysql: successful query from host %s",
host->hostname);
@@ -587,7 +639,7 @@
dict_mysql->dbname,
host->port,
(host->type == TYPEUNIX ? host->name : 0),
- 0)) {
+ CLIENT_MULTI_RESULTS)) {
if (msg_verbose)
msg_info("dict_mysql: successful connection to host %s",
host->hostname);
