rlm_sql_mysql - sql_finish_select_query - multiples results sets
Hi FR-dev, I'm working on FR 1.1.6 and mysql 5.0 I wan't to work with stored procedure in mysql. So as I read on this list there is some tuning of mysql connection flags to do. So I back ported rlm_sql_mysql from today snapshot (19/07/07) to FR 1.1.6 It works well but I have one problem: - In mysql radreply I have an attribute which look like this : AA-NAME = "My value" (this is a sample). - In sql.conf I don't do a SELECT for authorize_reply_query but I call my stored procedure: authorize_reply_query = "CALL p_authorize_reply('%{SQL-User-Name}','%{NAS-IP-Address}','%{Called-Station-Id}')" p_authorize_reply return all attributes in radreply. AA-NAME attribute is not in dictionary so in sql.c in function sql_getvpdata when FR try sql_userparse it failed and says : Error: rlm_sql: Failed to create the pair: Unknown attribute "AA-NAME" Then FR launch: (inst->module->sql_finish_select_query)(sqlsocket, inst->config); But sql_finish_select_query doesn't handle the fact that the statement is perhaps a multiple results set (as sql_finish_query in today snapshot does). So after the sql_finish_select_query the connection throw out of sync (error 2014) on all the next queries. Debug: rlm_sql_mysql: MYSQL check_error: 2014 received => Database query error - Commands out of sync; you can't run this command now The connection never return to a synced state. I've duplicated the code of sql_finish_query in sql_finish_select_query to care of multiple results set and then it works. What do you think about this? Note: I know that this is the bad Attribut that break the mysql state, but I wonder if it's right that sql_finish_select_query should care of multiple results set as sql_finish_query or not. Thanks Jean DERAM
En réponse à Jean DERAM :
I'm working on FR 1.1.6 and mysql 5.0
I wan't to work with stored procedure in mysql. So as I read on this list there is some tuning of mysql connection flags to do. So I back ported rlm_sql_mysql from today snapshot (19/07/07) to FR 1.1.6
This is not needed. We're going to include support for MySQL stored procedure in 1.1.7. Therefore I'd suggest you get the sources like that: $ cvs -d :pserver:anoncvs@cvs.freeradius.org:/source login CVS password: anoncvs $ cvs -d :pserver:anoncvs@cvs.freeradius.org:/source checkout -r branch_1_1 radiusd
I've duplicated the code of sql_finish_query in sql_finish_select_query to care of multiple results set and then it works.
What do you think about this?
I think maybe you didn't backport the changes to sql_store_result(). It should work if you use the branch 1.1 of the CVS. As always, any feedback is really welcome. -- Nicolas Baradakis
Nicolas Baradakis a écrit :
En réponse à Jean DERAM :
I'm working on FR 1.1.6 and mysql 5.0
I wan't to work with stored procedure in mysql. So as I read on this list there is some tuning of mysql connection flags to do. So I back ported rlm_sql_mysql from today snapshot (19/07/07) to FR 1.1.6
This is not needed. We're going to include support for MySQL stored procedure in 1.1.7. Therefore I'd suggest you get the sources like that:
$ cvs -d :pserver:anoncvs@cvs.freeradius.org:/source login CVS password: anoncvs $ cvs -d :pserver:anoncvs@cvs.freeradius.org:/source checkout -r branch_1_1 radiusd
I've duplicated the code of sql_finish_query in sql_finish_select_query to care of multiple results set and then it works.
What do you think about this?
I think maybe you didn't backport the changes to sql_store_result(). It should work if you use the branch 1.1 of the CVS. As always, any feedback is really welcome.
I've backported all changed in relation with stored procedure from rlm_mysql. I've done what you recommanded to me: get cvs version and make a try. I've the same problem with cvs version. This is reproducible like this. In mysql create a test stored procedure: DROP PROCEDURE IF EXISTS p_test; DELIMITER $$ CREATE PROCEDURE p_test( IN uname VARCHAR(64) ) BEGIN SELECT id, UserName, Attribute, Value, op FROM radreply WHERE Username=uname ORDER BY id; END $$ DELIMITER ; in /etc/raddb/sql.conf: authorize_reply_query = "CALL p_test('%{SQL-User-Name}')" num_sql_socks = 1 I limit num_sql_socks to force use of the same socket for this test. In radreply, insert correct attributes: INSERT INTO `radreply` VALUES ('', 'jean', 'Framed-Routing', ':=', 'None'); INSERT INTO `radreply` VALUES ('', 'jean', 'Framed-Protocol', ':=', 'PPP'); INSERT INTO `radreply` VALUES ('', 'jean', 'Service-Type', ':=', 'Framed-User'); Now we can test with your favorite tools. All is ok, user can login, attributes are send in the reply. Then we insert a bad attribute: INSERT INTO `radreply` VALUES ('', 'jean', 'AA-TEST', ':=', 'xxxxxx'); Now if we try to login we get an Access-Reject In debug we get: Error: rlm_sql: Failed to create the pair: Unknown attribute "AA-TEST" Error: rlm_sql (sql): Error getting data from database This seems good but all next sql queries failed (even if user is different and has good attributes): I always get this in log: Debug: rlm_sql_mysql: MYSQL check_error: 2014 received Error: rlm_sql (sql) in sql_postauth: Database query error - Commands out of sync; you can't run this comand now This seems to be due to a bad free_result of a multiple result set. The mysql connection get out of sync, a never recover for a normal state (all mysql queries are rejected). I explain this by this part of code in rlm_sql.c: int sql_getvpdata(SQL_INST * inst, SQLSOCK * sqlsocket, VALUE_PAIR **pair, char *query, int mode) { SQL_ROW row; int rows = 0; /* * If there's no query, return an error. */ if (!query || !*query) { return -1; } if (rlm_sql_select_query(sqlsocket, inst, query)) { radlog(L_ERR, "rlm_sql_getvpdata: database query error"); return -1; } while (rlm_sql_fetch_row(sqlsocket, inst)==0) { row = sqlsocket->row; if (!row) break; if (sql_userparse(pair, row, mode) != 0) { radlog(L_ERR | L_CONS, "rlm_sql (%s): Error getting data from database", inst->config->xlat_name); (inst->module->sql_finish_select_query)(sqlsocket, inst->config); return -1; } rows++; } (inst->module->sql_finish_select_query)(sqlsocket, inst->config); return rows; } => If sql_userparse failed then FR call (inst->module->sql_finish_select_query)(sqlsocket, inst->config); But sql_finish_select_query is only doing this: static int sql_finish_select_query(SQLSOCK * sqlsocket, SQL_CONFIG *config) { sql_free_result(sqlsocket, config); return 0; } I think sql_finish_select_query should be care if the result is a multiple result set. I've made a patch for cvs version (joined in this mail). With this patch I've got a better behaviour: - FR says the attribut AA-TEST is unknown - FR return good attibutes and send Access-Accept (without the patch, FR send a reject) - FR doesn't make the connection out of sync Have got the same behaviour with stored procedure and bad attributes? Regards, --- freeradius-1.1.7/src/modules/rlm_sql/drivers/rlm_sql_mysql/sql_mysql.c_origin 2007-07-20 07:10:31.000000000 +0200 +++ freeradius-1.1.7/src/modules/rlm_sql/drivers/rlm_sql_mysql/sql_mysql.c 2007-07-20 07:11:10.000000000 +0200 @@ -455,7 +455,31 @@ *************************************************************************/ static int sql_finish_select_query(SQLSOCK * sqlsocket, SQL_CONFIG *config) { +#if (MYSQL_VERSION_ID >= 40100) + rlm_sql_mysql_sock *mysql_sock = sqlsocket->conn; + int status; + +skip_next_result: + status = sql_store_result(sqlsocket, config); + if (status != 0) { + return status; + } else if (mysql_sock->result != NULL) { + radlog(L_DBG, "rlm_sql_mysql: SQL statement returned unexpected result"); + sql_free_result(sqlsocket, config); + } + status = mysql_next_result(mysql_sock->sock); + if (status == 0) { + /* there are more results */ + goto skip_next_result; + } else if (status > 0) { + radlog(L_ERR, "rlm_sql_mysql: Cannot get next result"); + radlog(L_ERR, "rlm_sql_mysql: MySQL error '%s'", + mysql_error(mysql_sock->sock)); + return sql_check_error(status); + } +#else sql_free_result(sqlsocket, config); +#endif return 0; }
Jean DERAM wrote:
I think sql_finish_select_query should be care if the result is a multiple result set. I've made a patch for cvs version (joined in this mail). [...]
Thanks for the explanation. I now understand the code path that leads to the problem. In a select query all the results are to be fetched by sql_fetch_row(). However if there is nonsense in the database, we may stop fetching the rows before reaching the last result of a multiple results set. The problem with your patch is that it may call sql_store_result() twice on the same result. I've written a fix in the CVS, please run a "cvs up" and test again. -- Nicolas Baradakis
Nicolas Baradakis a écrit :
Jean DERAM wrote:
I think sql_finish_select_query should be care if the result is a multiple result set. I've made a patch for cvs version (joined in this mail). [...]
Thanks for the explanation. I now understand the code path that leads to the problem. In a select query all the results are to be fetched by sql_fetch_row(). However if there is nonsense in the database, we may stop fetching the rows before reaching the last result of a multiple results set.
The problem with your patch is that it may call sql_store_result() twice on the same result. I've written a fix in the CVS, please run a "cvs up" and test again.
I've made a test with the latest cvs. All is working now! Thanks for your work.
participants (3)
-
Alan DeKok -
Jean DERAM -
Nicolas Baradakis