rlm_sql_mysql: SP support extended patch
Here is my work on the subject. First, I wrote a patch that iterates through multi-results in sql_store_result() and finally, takes the last result to store ( http://tifs.anarxi.st/freeradius-1.1.3-mysql5_sp.v1.patch) But later I understood that better way to do so is in sql_finish_select_query(). Besides, I wanted a possibility to use stored procedures in accounting calls. So I've also written some code to sql_finish_query(). Oher changes were made as well. While making the changes, I followed next rules: - don't break sql_mysql.c code-style; - add functionality only for MYSQL_VERSION_ID >= 50000 - use hints from rlm_sql_postgresql (like "rlm_sql_mysql: affected rows = %d" I took from there); - follow mysql version 5.0 reference; - final code should be able to handle all kinds of multi-results; Patch v2 (http://tifs.anarxi.st/freeradius-1.1.3-mysql5_sp.v2.patch) allows: - Use stored procedures in 'select' calls (used in authorization queries). First result that contains any fields is used. Any other results are ignored and properly freed, and debug message is printed:
rlm_sql_mysql: SQL query returned redundant result rlm_sql_mysql: redundant result (5 fields) ignored Any affected rows result is dumped as: rlm_sql_mysql: affected rows = 0
- Use stored procedures in 'update/insert query' calls (used in accounting). Results with fields are ignored (with correspondind message). First proper affected rows result is used for sql_affected_rows() if called. Any other affected rows result is dumped. However it seems that MySQL currently is unable to return more than one affected rows count and some code can be cutted out. But I left it for compatibility with MySQL v5.0 reference. I've performed some testing: - using SP as authorization reply query, trying to confuse rlm_sql_myql with multiple results (everything works ok, first result is used other ones ignored); - using SP as accounting start query (sql_query() -> sql_finish_query()), works ok, redundant results are ignored, affected rows dumped; - using SP as accounting update query (sql_query() -> sql_affected_rows() -> sql_finish_query()), works ok until we select any result set. affected rows = 0 comes instead of real affected rows count and freeradius tries alternative query. Though SP is still proceed ;) Hope this will be useful for Freeradius project! Please, use this code if you found it suitable. The patch itself comes below (you can also download it here: http://tifs.anarxi.st/freeradius-1.1.3-mysql5_sp.v2.patch) diff -ruN freeradius-1.1.3.orig/src/modules/rlm_sql/drivers/rlm_sql_mysql/sql_mysql.c freeradius-1.1.3/src/modules/rlm_sql/drivers/rlm_sql_mysql/sql_mysql.c --- freeradius-1.1.3.orig/src/modules/rlm_sql/drivers/rlm_sql_mysql/sql_mysql.c 2005-12-09 18:10:08.000000000 +0200 +++ freeradius-1.1.3/src/modules/rlm_sql/drivers/rlm_sql_mysql/sql_mysql.c 2007-05-17 02:10:13.000000000 +0300 @@ -47,9 +47,13 @@ MYSQL conn; MYSQL *sock; MYSQL_RES *result; + int num_fields; SQL_ROW row; } rlm_sql_mysql_sock; +/* Prototypes */ +static int sql_num_fields(SQLSOCK * sqlsocket, SQL_CONFIG *config); + /************************************************************************* * * Function: sql_create_socket @@ -82,7 +86,11 @@ config->sql_db, atoi(config->sql_port), NULL, +#if MYSQL_VERSION_ID < 50000 CLIENT_FOUND_ROWS))) { +#else + CLIENT_FOUND_ROWS|CLIENT_MULTI_STATEMENTS|CLIENT_MULTI_RESULTS))) { +#endif radlog(L_ERR, "rlm_sql_mysql: Couldn't connect socket to MySQL server %s@%s:%s", config->sql_login, config->sql_server, config->sql_db); radlog(L_ERR, "rlm_sql_mysql: Mysql error '%s'", mysql_error(&mysql_sock->conn)); mysql_sock->sock = NULL; @@ -173,18 +181,43 @@ *************************************************************************/ static int sql_store_result(SQLSOCK * sqlsocket, SQL_CONFIG *config) { +#if MYSQL_VERSION_ID >= 50000 + int status; +#endif rlm_sql_mysql_sock *mysql_sock = sqlsocket->conn; if (mysql_sock->sock == NULL) { radlog(L_ERR, "rlm_sql_mysql: Socket not connected"); return SQL_DOWN; } +#if MYSQL_VERSION_ID >= 50000 + /* + * Get every affected row count prior to result + */ + while (!(mysql_field_count(mysql_sock->sock))) { + radlog(L_DBG, "rlm_sql_mysql: affected rows = %d", + mysql_affected_rows(mysql_sock->sock)); + + if ((status = mysql_next_result(mysql_sock->sock))) { + if (status > 0) { + radlog(L_ERR, "rlm_sql_mysql: MYSQL Error: Cannot get next result"); + radlog(L_ERR, "rlm_sql_mysql: MYSQL Error: %s", + mysql_error(mysql_sock->sock)); + return sql_check_error(mysql_errno(mysql_sock->sock)); + } + break; + } + } +#endif + if (!(mysql_sock->result = mysql_store_result(mysql_sock->sock))) { radlog(L_ERR, "rlm_sql_mysql: MYSQL Error: Cannot get result"); radlog(L_ERR, "rlm_sql_mysql: MYSQL Error: %s", mysql_error(mysql_sock->sock)); return sql_check_error(mysql_errno(mysql_sock->sock)); } + mysql_sock->num_fields = sql_num_fields(sqlsocket, config); + return 0; } @@ -235,12 +268,6 @@ return ret; } - /* Why? Per http://www.mysql.com/doc/n/o/node_591.html, - * this cannot return an error. Perhaps just to complain if no - * fields are found? - */ - sql_num_fields(sqlsocket, config); - return ret; } @@ -364,6 +391,59 @@ *************************************************************************/ static int sql_finish_query(SQLSOCK * sqlsocket, SQL_CONFIG *config) { +#if MYSQL_VERSION_ID >= 50000 + int status; + int field_count; + MYSQL_RES *result; + rlm_sql_mysql_sock *mysql_sock = sqlsocket->conn; + + /* + * Free current result if we have one + */ + if ((field_count = mysql_field_count(mysql_sock->sock))) { + radlog(L_DBG, "rlm_sql_mysql: SQL query returned redundant result"); + if (!(result = mysql_store_result(mysql_sock->sock))) { + radlog(L_ERR, "rlm_sql_mysql: MYSQL Error: Cannot get result"); + radlog(L_ERR, "rlm_sql_mysql: MYSQL Error: %s", + mysql_error(mysql_sock->sock)); + return sql_check_error(mysql_errno(mysql_sock->sock)); + } + radlog(L_DBG, "rlm_sql_mysql: redundant result (%d fields) ignored", + field_count); + mysql_free_result(result); + } + + /* + * Get and free every next result or affected row count + */ + for (;;) { + if ((status = mysql_next_result(mysql_sock->sock))) { + if (status > 0) { + radlog(L_ERR, "rlm_sql_mysql: MYSQL Error: Cannot get next result"); + radlog(L_ERR, "rlm_sql_mysql: MYSQL Error: %s", + mysql_error(mysql_sock->sock)); + return sql_check_error(mysql_errno(mysql_sock->sock)); + } + break; + } + if ((field_count = mysql_field_count(mysql_sock->sock))) { + radlog(L_DBG, "rlm_sql_mysql: SQL query returned redundant result"); + if (!(result = mysql_store_result(mysql_sock->sock))) { + radlog(L_ERR, "rlm_sql_mysql: MYSQL Error: Cannot get result"); + radlog(L_ERR, "rlm_sql_mysql: MYSQL Error: %s", + mysql_error(mysql_sock->sock)); + return sql_check_error(mysql_errno(mysql_sock->sock)); + } + radlog(L_DBG, "rlm_sql_mysql: redundant result (%d fields) ignored", + field_count); + mysql_free_result(result); + } else { + radlog(L_DBG, "rlm_sql_mysql: affected rows = %d", + mysql_affected_rows(mysql_sock->sock)); + } + } +#endif + return 0; } @@ -378,7 +458,45 @@ *************************************************************************/ static int sql_finish_select_query(SQLSOCK * sqlsocket, SQL_CONFIG *config) { +#if MYSQL_VERSION_ID >= 50000 + int status; + int field_count; + MYSQL_RES *result; + rlm_sql_mysql_sock *mysql_sock = sqlsocket->conn; +#endif + sql_free_result(sqlsocket, config); +#if MYSQL_VERSION_ID >= 50000 + /* + * Get and free every next result or affected row count + */ + for (;;) { + if ((status = mysql_next_result(mysql_sock->sock))) { + if (status > 0) { + radlog(L_ERR, "rlm_sql_mysql: MYSQL Error: Cannot get next result"); + radlog(L_ERR, "rlm_sql_mysql: MYSQL Error: %s", + mysql_error(mysql_sock->sock)); + return sql_check_error(mysql_errno(mysql_sock->sock)); + } + break; + } + if ((field_count = mysql_field_count(mysql_sock->sock))) { + radlog(L_DBG, "rlm_sql_mysql: SQL query returned redundant result"); + if (!(result = mysql_store_result(mysql_sock->sock))) { + radlog(L_ERR, "rlm_sql_mysql: MYSQL Error: Cannot get result"); + radlog(L_ERR, "rlm_sql_mysql: MYSQL Error: %s", + mysql_error(mysql_sock->sock)); + return sql_check_error(mysql_errno(mysql_sock->sock)); + } + radlog(L_DBG, "rlm_sql_mysql: redundant result (%d fields) ignored", + field_count); + mysql_free_result(result); + } else { + radlog(L_DBG, "rlm_sql_mysql: affected rows = %d", + mysql_affected_rows(mysql_sock->sock)); + } + } +#endif return 0; } @@ -393,8 +511,40 @@ *************************************************************************/ static int sql_affected_rows(SQLSOCK * sqlsocket, SQL_CONFIG *config) { +#if MYSQL_VERSION_ID >= 50000 + int status; + int field_count; + MYSQL_RES *result; +#endif rlm_sql_mysql_sock *mysql_sock = sqlsocket->conn; +#if MYSQL_VERSION_ID >= 50000 + /* + * Get and free every result prior to affected row count + */ + while ((field_count = mysql_field_count(mysql_sock->sock))) { + radlog(L_DBG, "rlm_sql_mysql: SQL query returned redundant result"); + if (!(result = mysql_store_result(mysql_sock->sock))) { + radlog(L_ERR, "rlm_sql_mysql: MYSQL Error: Cannot get result"); + radlog(L_ERR, "rlm_sql_mysql: MYSQL Error: %s", + mysql_error(mysql_sock->sock)); + return -1; + } + radlog(L_DBG, "rlm_sql_mysql: redundant result (%d fields) ignored", + field_count); + mysql_free_result(result); + + if ((status = mysql_next_result(mysql_sock->sock))) { + if (status > 0) { + radlog(L_ERR, "rlm_sql_mysql: MYSQL Error: Cannot get next result"); + radlog(L_ERR, "rlm_sql_mysql: MYSQL Error: %s", + mysql_error(mysql_sock->sock)); + return -1; + } + return -1; + } + } +#endif return mysql_affected_rows(mysql_sock->sock); }
Heh. This is rather amusing in that the postgresql shim (rlm_sql_postgresql) and its wrapper module (rlm_sql) started life as MySQL specific. This means that the design doesnt't actually exactly fit the way Postgresql does things as MySQL had no concept of transactions or stored procedures etc at the time of writing, and rlm_sql_postgresql has to do some "funny things" in order to stay in that model (as do the other sql driver modules). The fact that youhave now taken some "hints" from the Postgresql driver module means that we probably should sit down and redesign rlm_sql's interface to the shims as MySQL now resembles a "real database" and does things pretty similar to other "real" databases. Any takers? (I will provide what support I can on Postgresql) Peter On Thu 17 May 2007, Stanislav Schukin wrote:
Here is my work on the subject.
First, I wrote a patch that iterates through multi-results in sql_store_result() and finally, takes the last result to store ( http://tifs.anarxi.st/freeradius-1.1.3-mysql5_sp.v1.patch)
But later I understood that better way to do so is in sql_finish_select_query(). Besides, I wanted a possibility to use stored procedures in accounting calls. So I've also written some code to sql_finish_query(). Oher changes were made as well.
While making the changes, I followed next rules: - don't break sql_mysql.c code-style; - add functionality only for MYSQL_VERSION_ID >= 50000 - use hints from rlm_sql_postgresql (like "rlm_sql_mysql: affected rows = %d" I took from there); - follow mysql version 5.0 reference; - final code should be able to handle all kinds of multi-results;
Patch v2 (http://tifs.anarxi.st/freeradius-1.1.3-mysql5_sp.v2.patch) allows:
- Use stored procedures in 'select' calls (used in authorization queries). First result that contains any fields is used. Any other results are ignored and properly freed, and debug message is
printed:
rlm_sql_mysql: SQL query returned redundant result rlm_sql_mysql: redundant result (5 fields) ignored
Any affected rows result is dumped as:
rlm_sql_mysql: affected rows = 0
- Use stored procedures in 'update/insert query' calls (used in accounting). Results with fields are ignored (with correspondind message). First proper affected rows result is used for sql_affected_rows() if called. Any other affected rows result is dumped.
However it seems that MySQL currently is unable to return more than one affected rows count and some code can be cutted out. But I left it for compatibility with MySQL v5.0 reference.
I've performed some testing: - using SP as authorization reply query, trying to confuse rlm_sql_myql with multiple results (everything works ok, first result is used other ones ignored); - using SP as accounting start query (sql_query() -> sql_finish_query()), works ok, redundant results are ignored, affected rows dumped; - using SP as accounting update query (sql_query() -> sql_affected_rows() -> sql_finish_query()), works ok until we select any result set. affected rows = 0 comes instead of real affected rows count and freeradius tries alternative query. Though SP is still proceed ;)
Hope this will be useful for Freeradius project! Please, use this code if you found it suitable.
The patch itself comes below (you can also download it here: http://tifs.anarxi.st/freeradius-1.1.3-mysql5_sp.v2.patch)
diff -ruN freeradius-1.1.3.orig/src/modules/rlm_sql/drivers/rlm_sql_mysql/sql_mysql. c freeradius-1.1.3/src/modules/rlm_sql/drivers/rlm_sql_mysql/sql_mysql.c --- freeradius-1.1.3.orig/src/modules/rlm_sql/drivers/rlm_sql_mysql/sql_mysql. c 2005-12-09 18:10:08.000000000 +0200 +++ freeradius-1.1.3/src/modules/rlm_sql/drivers/rlm_sql_mysql/sql_mysql.c 2007-05-17 02:10:13.000000000 +0300 @@ -47,9 +47,13 @@ MYSQL conn; MYSQL *sock; MYSQL_RES *result; + int num_fields; SQL_ROW row; } rlm_sql_mysql_sock;
+/* Prototypes */ +static int sql_num_fields(SQLSOCK * sqlsocket, SQL_CONFIG *config); + /************************************************************************* * * Function: sql_create_socket @@ -82,7 +86,11 @@ config->sql_db, atoi(config->sql_port), NULL, +#if MYSQL_VERSION_ID < 50000 CLIENT_FOUND_ROWS))) { +#else + CLIENT_FOUND_ROWS|CLIENT_MULTI_STATEMENTS|CLIENT_MULTI_RESULTS))) { +#endif radlog(L_ERR, "rlm_sql_mysql: Couldn't connect socket to MySQL server %s@%s:%s", config->sql_login, config->sql_server, config->sql_db); radlog(L_ERR, "rlm_sql_mysql: Mysql error '%s'", mysql_error(&mysql_sock->conn)); mysql_sock->sock = NULL; @@ -173,18 +181,43 @@
*************************************************************************/ static int sql_store_result(SQLSOCK * sqlsocket, SQL_CONFIG *config) { +#if MYSQL_VERSION_ID >= 50000 + int status; +#endif rlm_sql_mysql_sock *mysql_sock = sqlsocket->conn;
if (mysql_sock->sock == NULL) { radlog(L_ERR, "rlm_sql_mysql: Socket not connected"); return SQL_DOWN; } +#if MYSQL_VERSION_ID >= 50000 + /* + * Get every affected row count prior to result + */ + while (!(mysql_field_count(mysql_sock->sock))) { + radlog(L_DBG, "rlm_sql_mysql: affected rows = %d", + mysql_affected_rows(mysql_sock->sock)); + + if ((status = mysql_next_result(mysql_sock->sock))) { + if (status > 0) { + radlog(L_ERR, "rlm_sql_mysql: MYSQL Error: Cannot get next result"); + radlog(L_ERR, "rlm_sql_mysql: MYSQL Error: %s", + mysql_error(mysql_sock->sock)); + return sql_check_error(mysql_errno(mysql_sock->sock)); + } + break; + } + } +#endif + if (!(mysql_sock->result = mysql_store_result(mysql_sock->sock))) { radlog(L_ERR, "rlm_sql_mysql: MYSQL Error: Cannot get result"); radlog(L_ERR, "rlm_sql_mysql: MYSQL Error: %s", mysql_error(mysql_sock->sock)); return sql_check_error(mysql_errno(mysql_sock->sock)); } + mysql_sock->num_fields = sql_num_fields(sqlsocket, config); + return 0; }
@@ -235,12 +268,6 @@ return ret; }
- /* Why? Per http://www.mysql.com/doc/n/o/node_591.html, - * this cannot return an error. Perhaps just to complain if no - * fields are found? - */ - sql_num_fields(sqlsocket, config); - return ret; }
@@ -364,6 +391,59 @@
*************************************************************************/ static int sql_finish_query(SQLSOCK * sqlsocket, SQL_CONFIG *config) { +#if MYSQL_VERSION_ID >= 50000 + int status; + int field_count; + MYSQL_RES *result; + rlm_sql_mysql_sock *mysql_sock = sqlsocket->conn; + + /* + * Free current result if we have one + */ + if ((field_count = mysql_field_count(mysql_sock->sock))) { + radlog(L_DBG, "rlm_sql_mysql: SQL query returned redundant result"); + if (!(result = mysql_store_result(mysql_sock->sock))) { + radlog(L_ERR, "rlm_sql_mysql: MYSQL Error: Cannot get result"); + radlog(L_ERR, "rlm_sql_mysql: MYSQL Error: %s", + mysql_error(mysql_sock->sock)); + return sql_check_error(mysql_errno(mysql_sock->sock)); + } + radlog(L_DBG, "rlm_sql_mysql: redundant result (%d fields) ignored", + field_count); + mysql_free_result(result); + } + + /* + * Get and free every next result or affected row count + */ + for (;;) { + if ((status = mysql_next_result(mysql_sock->sock))) { + if (status > 0) { + radlog(L_ERR, "rlm_sql_mysql: MYSQL Error: Cannot get next result"); + radlog(L_ERR, "rlm_sql_mysql: MYSQL Error: %s", + mysql_error(mysql_sock->sock)); + return sql_check_error(mysql_errno(mysql_sock->sock)); + } + break; + } + if ((field_count = mysql_field_count(mysql_sock->sock))) { + radlog(L_DBG, "rlm_sql_mysql: SQL query returned redundant result"); + if (!(result = mysql_store_result(mysql_sock->sock))) { + radlog(L_ERR, "rlm_sql_mysql: MYSQL Error: Cannot get result"); + radlog(L_ERR, "rlm_sql_mysql: MYSQL Error: %s", + mysql_error(mysql_sock->sock)); + return sql_check_error(mysql_errno(mysql_sock->sock)); + } + radlog(L_DBG, "rlm_sql_mysql: redundant result (%d fields) ignored", + field_count); + mysql_free_result(result); + } else { + radlog(L_DBG, "rlm_sql_mysql: affected rows = %d", + mysql_affected_rows(mysql_sock->sock)); + } + } +#endif + return 0; }
@@ -378,7 +458,45 @@
*************************************************************************/ static int sql_finish_select_query(SQLSOCK * sqlsocket, SQL_CONFIG *config) { +#if MYSQL_VERSION_ID >= 50000 + int status; + int field_count; + MYSQL_RES *result; + rlm_sql_mysql_sock *mysql_sock = sqlsocket->conn; +#endif + sql_free_result(sqlsocket, config); +#if MYSQL_VERSION_ID >= 50000 + /* + * Get and free every next result or affected row count + */ + for (;;) { + if ((status = mysql_next_result(mysql_sock->sock))) { + if (status > 0) { + radlog(L_ERR, "rlm_sql_mysql: MYSQL Error: Cannot get next result"); + radlog(L_ERR, "rlm_sql_mysql: MYSQL Error: %s", + mysql_error(mysql_sock->sock)); + return sql_check_error(mysql_errno(mysql_sock->sock)); + } + break; + } + if ((field_count = mysql_field_count(mysql_sock->sock))) { + radlog(L_DBG, "rlm_sql_mysql: SQL query returned redundant result"); + if (!(result = mysql_store_result(mysql_sock->sock))) { + radlog(L_ERR, "rlm_sql_mysql: MYSQL Error: Cannot get result"); + radlog(L_ERR, "rlm_sql_mysql: MYSQL Error: %s", + mysql_error(mysql_sock->sock)); + return sql_check_error(mysql_errno(mysql_sock->sock)); + } + radlog(L_DBG, "rlm_sql_mysql: redundant result (%d fields) ignored", + field_count); + mysql_free_result(result); + } else { + radlog(L_DBG, "rlm_sql_mysql: affected rows = %d", + mysql_affected_rows(mysql_sock->sock)); + } + } +#endif
return 0; } @@ -393,8 +511,40 @@
*************************************************************************/ static int sql_affected_rows(SQLSOCK * sqlsocket, SQL_CONFIG *config) { +#if MYSQL_VERSION_ID >= 50000 + int status; + int field_count; + MYSQL_RES *result; +#endif rlm_sql_mysql_sock *mysql_sock = sqlsocket->conn;
+#if MYSQL_VERSION_ID >= 50000 + /* + * Get and free every result prior to affected row count + */ + while ((field_count = mysql_field_count(mysql_sock->sock))) { + radlog(L_DBG, "rlm_sql_mysql: SQL query returned redundant result"); + if (!(result = mysql_store_result(mysql_sock->sock))) { + radlog(L_ERR, "rlm_sql_mysql: MYSQL Error: Cannot get result"); + radlog(L_ERR, "rlm_sql_mysql: MYSQL Error: %s", + mysql_error(mysql_sock->sock)); + return -1; + } + radlog(L_DBG, "rlm_sql_mysql: redundant result (%d fields) ignored", + field_count); + mysql_free_result(result); + + if ((status = mysql_next_result(mysql_sock->sock))) { + if (status > 0) { + radlog(L_ERR, "rlm_sql_mysql: MYSQL Error: Cannot get next result"); + radlog(L_ERR, "rlm_sql_mysql: MYSQL Error: %s", + mysql_error(mysql_sock->sock)); + return -1; + } + return -1; + } + } +#endif return mysql_affected_rows(mysql_sock->sock); }
-- Peter Nixon http://www.peternixon.net/ PGP Key: http://www.peternixon.net/public.asc
Peter Nixon wrote:
Heh. This is rather amusing in that the postgresql shim (rlm_sql_postgresql) and its wrapper module (rlm_sql) started life as MySQL specific. This means that the design doesnt't actually exactly fit the way Postgresql does things as MySQL had no concept of transactions or stored procedures etc at the time of writing, and rlm_sql_postgresql has to do some "funny things" in order to stay in that model (as do the other sql driver modules). The fact that youhave now taken some "hints" from the Postgresql driver module means that we probably should sit down and redesign rlm_sql's interface to the shims as MySQL now resembles a "real database" and does things pretty similar to other "real" databases.
Any takers? (I will provide what support I can on Postgresql)
Let's delay this until after 2.0.0 has been released. Alan DeKok. -- http://deployingradius.com - The web site of the book http://deployingradius.com/blog/ - The blog
2007/5/17, Peter Nixon listuser@peternixon.net:
The fact that youhave now taken some "hints" from the Postgresql driver module means that we probably should sit down and redesign rlm_sql's interface to the shims as MySQL now resembles a "real database" and does things pretty similar to other "real" databases.
Any takers? (I will provide what support I can on Postgresql)
I'm not familar neither with postgresql API nor with postgresql as database. So I just don't have any ideas concerning the whole interface redesign. However I extended the current interface a bit with a new function called sql_store_next_result() in rlm_sql/shim level. And I've written a patch that makes rlm_sql_getvpdata() to use this function so it now able to process several result sets after a query. This allows queries like "SELECT ... FROM `radreply` WHERE `Username` = '%{SQL-User-Name}'; SELECT ... FROM `radreply_additional` WHERE `Username` = '%{SQL-User-Name}'" ...Or procedures that contain: SELECT `id`, `UserName`, `Attribute`, `Value`, `op` FROM `radreply` WHERE `Username` = userName ORDER BY `id`; SELECT 0, 'myuser', 'Acct-Interim-Interval', '30', ':='; SELECT 0, 'myuser', 'Other-attribute-comes-here', 'val', ':='; This will cause several results with similar schema, and I made rlm_sql_getvpdata() to be able to process them. This patch should be applied to freeradius-1.1.6 tree that is already patched with new version of my previous patch (freeradius-1.1.6-mysql5_sp.v3.patch). New version have fixes for MYSQL_VERSION_ID preprocessor conditions (version changed to 40100) and have one more error handling ("rlm_sql_mysql: MYSQL Error: No result set available"). You can get it here: http://tifs.anarxi.st/freeradius-1.1.6-mysql5_sp.v3.patch The patch goes below and can also be downloaded here: http://tifs.anarxi.st/freeradius-1.1.6-multi_results.v1.patch Have a nice day! diff -ruN freeradius-1.1.6.mysql5_sp.v3/src/modules/rlm_sql/conf.h freeradius-1.1.6.multi_results.v1/src/modules/rlm_sql/conf.h --- freeradius-1.1.6.mysql5_sp.v3/src/modules/rlm_sql/conf.h 2006-05-20 15:44:37.000000000 +0300 +++ freeradius-1.1.6.multi_results.v1/src/modules/rlm_sql/conf.h 2007-05-17 17:24:36.000000000 +0300 @@ -80,6 +80,7 @@ /* SQL Errors */ #define SQL_DOWN 1 /* for re-connect */ +#define SQL_NO_MORE_RESULTS 2 #define MAX_COMMUNITY_LEN 50 #define MAX_SQL_SOCKS 256 diff -ruN freeradius-1.1.6.mysql5_sp.v3/src/modules/rlm_sql/drivers/rlm_sql_db2/sql_db2.c freeradius-1.1.6.multi_results.v1/src/modules/rlm_sql/drivers/rlm_sql_db2/sql_db2.c --- freeradius-1.1.6.mysql5_sp.v3/src/modules/rlm_sql/drivers/rlm_sql_db2/sql_db2.c 2004-05-01 14:31:36.000000000 +0300 +++ freeradius-1.1.6.multi_results.v1/src/modules/rlm_sql/drivers/rlm_sql_db2/sql_db2.c 2007-05-17 17:45:15.000000000 +0300 @@ -163,6 +163,20 @@ /************************************************************************* * + * Function: sql_store_next_result + * + * Purpose: database specific store_next_result function. Returns + * next result set for the query. + * + *************************************************************************/ +static int sql_store_next_result(SQLSOCK * sqlsocket, SQL_CONFIG *config) +{ + return 0; +} + + +/************************************************************************* + * * Function: sql_num_fields * * Purpose: database specific num_fields function. Returns number @@ -352,6 +366,7 @@ sql_query, sql_select_query, not_implemented, /* sql_store_result */ + sql_store_next_result, sql_num_fields, not_implemented, /* sql_num_rows */ sql_fetch_row, diff -ruN freeradius-1.1.6.mysql5_sp.v3/src/modules/rlm_sql/drivers/rlm_sql_firebird/sql_firebird.c freeradius-1.1.6.multi_results.v1/src/modules/rlm_sql/drivers/rlm_sql_firebird/sql_firebird.c --- freeradius-1.1.6.mysql5_sp.v3/src/modules/rlm_sql/drivers/rlm_sql_firebird/sql_firebird.c 2007-03-09 15:34:37.000000000 +0200 +++ freeradius-1.1.6.multi_results.v1/src/modules/rlm_sql/drivers/rlm_sql_firebird/sql_firebird.c 2007-05-17 17:25:29.000000000 +0300 @@ -162,6 +162,19 @@ /************************************************************************* * + * Function: sql_store_next_result + * + * Purpose: database specific store_next_result function. Returns + * next result set for the query. + * + *************************************************************************/ +static int sql_store_next_result(SQLSOCK *sqlsocket, SQL_CONFIG *config) { + /* Not used */ + return SQL_NO_MORE_RESULTS; +} + +/************************************************************************* + * * Function: sql_num_fields * * Purpose: database specific num_fields function. Returns number @@ -305,6 +318,7 @@ sql_query, sql_select_query, sql_store_result, + sql_store_next_result, sql_num_fields, sql_num_rows, sql_fetch_row, diff -ruN freeradius-1.1.6.mysql5_sp.v3/src/modules/rlm_sql/drivers/rlm_sql_iodbc/sql_iodbc.c freeradius-1.1.6.multi_results.v1/src/modules/rlm_sql/drivers/rlm_sql_iodbc/sql_iodbc.c --- freeradius-1.1.6.mysql5_sp.v3/src/modules/rlm_sql/drivers/rlm_sql_iodbc/sql_iodbc.c 2004-02-26 21:04:36.000000000 +0200 +++ freeradius-1.1.6.multi_results.v1/src/modules/rlm_sql/drivers/rlm_sql_iodbc/sql_iodbc.c 2007-05-17 17:48:22.000000000 +0300 @@ -206,6 +206,20 @@ /************************************************************************* * + * Function: sql_store_next_result + * + * Purpose: database specific store_next_result function. Returns + * next result set for the query. + * + *************************************************************************/ +static int sql_store_next_result(SQLSOCK *sqlsocket, SQL_CONFIG *config) { + + return SQL_NO_MORE_RESULTS; +} + + +/************************************************************************* + * * Function: sql_num_fields * * Purpose: database specific num_fields function. Returns number @@ -391,6 +405,7 @@ sql_query, sql_select_query, sql_store_result, + sql_store_next_result, sql_num_fields, sql_num_rows, sql_fetch_row, diff -ruN freeradius-1.1.6.mysql5_sp.v3/src/modules/rlm_sql/drivers/rlm_sql_mysql/sql_mysql.c freeradius-1.1.6.multi_results.v1/src/modules/rlm_sql/drivers/rlm_sql_mysql/sql_mysql.c --- freeradius-1.1.6.mysql5_sp.v3/src/modules/rlm_sql/drivers/rlm_sql_mysql/sql_mysql.c 2007-05-18 00:09:28.000000000 +0300 +++ freeradius-1.1.6.multi_results.v1/src/modules/rlm_sql/drivers/rlm_sql_mysql/sql_mysql.c 2007-05-18 00:12:50.000000000 +0300 @@ -53,6 +53,7 @@ /* Prototypes */ static int sql_num_fields(SQLSOCK * sqlsocket, SQL_CONFIG *config); +static int sql_free_result(SQLSOCK * sqlsocket, SQL_CONFIG *config); /************************************************************************* * @@ -190,6 +191,7 @@ radlog(L_ERR, "rlm_sql_mysql: Socket not connected"); return SQL_DOWN; } + #if MYSQL_VERSION_ID >= 40100 /* * Get every affected row count prior to result @@ -226,6 +228,55 @@ /************************************************************************* * + * Function: sql_store_next_result + * + * Purpose: database specific store_next_result function. Returns + * next result set for the query. + * + *************************************************************************/ +static int sql_store_next_result(SQLSOCK * sqlsocket, SQL_CONFIG *config) +{ +#if MYSQL_VERSION_ID >= 40100 + int status; + rlm_sql_mysql_sock *mysql_sock = sqlsocket->conn; + + if (mysql_sock->sock == NULL) { + radlog(L_ERR, "rlm_sql_mysql: Socket not connected"); + return SQL_DOWN; + } + + sql_free_result(sqlsocket, config); + + for (;;) { + if ((status = mysql_next_result(mysql_sock->sock))) { + if (status > 0) { + radlog(L_ERR, "rlm_sql_mysql: MYSQL Error: Cannot get next result"); + radlog(L_ERR, "rlm_sql_mysql: MYSQL Error: %s", + mysql_error(mysql_sock->sock)); + return sql_check_error(mysql_errno(mysql_sock->sock)); + } + return SQL_NO_MORE_RESULTS; + } + + /* + * Get every affected row count prior to result + */ + if (!(mysql_field_count(mysql_sock->sock))) { + radlog(L_DBG, "rlm_sql_mysql: affected rows = %d", + mysql_affected_rows(mysql_sock->sock)); + } else { + break; + } + } + return sql_store_result(sqlsocket, config); +#else + return SQL_NO_MORE_RESULTS; +#endif +} + + +/************************************************************************* + * * Function: sql_num_fields * * Purpose: database specific num_fields function. Returns number @@ -559,6 +610,7 @@ sql_query, sql_select_query, sql_store_result, + sql_store_next_result, sql_num_fields, sql_num_rows, sql_fetch_row, diff -ruN freeradius-1.1.6.mysql5_sp.v3/src/modules/rlm_sql/drivers/rlm_sql_oracle/sql_oracle.c freeradius-1.1.6.multi_results.v1/src/modules/rlm_sql/drivers/rlm_sql_oracle/sql_oracle.c --- freeradius-1.1.6.mysql5_sp.v3/src/modules/rlm_sql/drivers/rlm_sql_oracle/sql_oracle.c 2004-04-16 00:19:20.000000000 +0300 +++ freeradius-1.1.6.multi_results.v1/src/modules/rlm_sql/drivers/rlm_sql_oracle/sql_oracle.c 2007-05-17 17:42:29.000000000 +0300 @@ -444,6 +444,20 @@ /************************************************************************* * + * Function: sql_store_next_result + * + * Purpose: database specific store_next_result function. Returns + * next result set for the query. + * + *************************************************************************/ +static int sql_store_next_result(SQLSOCK *sqlsocket, SQL_CONFIG *config) { + /* Not needed for Oracle */ + return SQL_NO_MORE_RESULTS; +} + + +/************************************************************************* + * * Function: sql_num_rows * * Purpose: database specific num_rows. Returns number of rows in @@ -603,6 +617,7 @@ sql_query, sql_select_query, sql_store_result, + sql_store_next_result, sql_num_fields, sql_num_rows, sql_fetch_row, diff -ruN freeradius-1.1.6.mysql5_sp.v3/src/modules/rlm_sql/drivers/rlm_sql_postgresql/sql_postgresql.c freeradius-1.1.6.multi_results.v1/src/modules/rlm_sql/drivers/rlm_sql_postgresql/sql_postgresql.c --- freeradius-1.1.6.mysql5_sp.v3/src/modules/rlm_sql/drivers/rlm_sql_postgresql/sql_postgresql.c 2007-03-21 15:02:59.000000000 +0200 +++ freeradius-1.1.6.multi_results.v1/src/modules/rlm_sql/drivers/rlm_sql_postgresql/sql_postgresql.c 2007-05-17 17:47:27.000000000 +0300 @@ -280,6 +280,20 @@ /************************************************************************* * + * Function: sql_store_next_result + * + * Purpose: database specific store_next_result function. Returns + * next result set for the query. + * + *************************************************************************/ +static int sql_store_next_result(SQLSOCK * sqlsocket, SQL_CONFIG *config) { + /* TODO: add next_result support */ + return SQL_NO_MORE_RESULTS; +} + + +/************************************************************************* + * * Function: sql_destroy_socket * * Purpose: Free socket and private connection data @@ -480,6 +494,7 @@ sql_query, sql_select_query, not_implemented, /* sql_store_result */ + sql_store_next_result, not_implemented, /* sql_num_fields */ not_implemented, /* sql_num_rows */ sql_fetch_row, diff -ruN freeradius-1.1.6.mysql5_sp.v3/src/modules/rlm_sql/drivers/rlm_sql_sybase/sql_sybase.c freeradius-1.1.6.multi_results.v1/src/modules/rlm_sql/drivers/rlm_sql_sybase/sql_sybase.c --- freeradius-1.1.6.mysql5_sp.v3/src/modules/rlm_sql/drivers/rlm_sql_sybase/sql_sybase.c 2004-02-26 21:04:36.000000000 +0200 +++ freeradius-1.1.6.multi_results.v1/src/modules/rlm_sql/drivers/rlm_sql_sybase/sql_sybase.c 2007-05-17 17:43:56.000000000 +0300 @@ -629,6 +629,23 @@ /************************************************************************* * + * Function: sql_store_next_result + * + * Purpose: database specific store_next_result function. Returns + * next result set for the query. + * + *************************************************************************/ +static int sql_store_result(SQLSOCK *sqlsocket, SQL_CONFIG *config) { + /* + ** Not needed for Sybase, code that may have gone here is + ** in sql_select_query and sql_fetch_row + */ + return SQL_NO_MORE_RESULTS; +} + + +/************************************************************************* + * * Function: sql_num_fields * * Purpose: database specific num_fields function. Returns number @@ -940,6 +957,7 @@ sql_query, sql_select_query, sql_store_result, + sql_store_next_result, sql_num_fields, sql_num_rows, sql_fetch_row, diff -ruN freeradius-1.1.6.mysql5_sp.v3/src/modules/rlm_sql/drivers/rlm_sql_unixodbc/sql_unixodbc.c freeradius-1.1.6.multi_results.v1/src/modules/rlm_sql/drivers/rlm_sql_unixodbc/sql_unixodbc.c --- freeradius-1.1.6.mysql5_sp.v3/src/modules/rlm_sql/drivers/rlm_sql_unixodbc/sql_unixodbc.c 2006-05-24 19:31:43.000000000 +0300 +++ freeradius-1.1.6.multi_results.v1/src/modules/rlm_sql/drivers/rlm_sql_unixodbc/sql_unixodbc.c 2007-05-17 17:49:12.000000000 +0300 @@ -206,6 +206,19 @@ /************************************************************************* * + * Function: sql_store_next_result + * + * Purpose: database specific store_next_result function. Returns + * next result set for the query. + * + *************************************************************************/ +static int sql_store_next_result(SQLSOCK *sqlsocket, SQL_CONFIG *config) { + return SQL_NO_MORE_RESULTS; +} + + +/************************************************************************* + * * Function: sql_num_fields * * Purpose: database specific num_fields function. Returns number @@ -455,6 +468,7 @@ sql_query, sql_select_query, sql_store_result, + sql_store_next_result, sql_num_fields, sql_num_rows, sql_fetch_row, diff -ruN freeradius-1.1.6.mysql5_sp.v3/src/modules/rlm_sql/README freeradius-1.1.6.multi_results.v1/src/modules/rlm_sql/README --- freeradius-1.1.6.mysql5_sp.v3/src/modules/rlm_sql/README 2003-07-30 22:24:17.000000000 +0300 +++ freeradius-1.1.6.multi_results.v1/src/modules/rlm_sql/README 2007-05-17 17:24:54.000000000 +0300 @@ -13,6 +13,8 @@ sql_select_query: returns -1 on failure, SQL_DOWN on 'socket not connected' sql_query: returns -1 on failure, SQL_DOWN on 'socket not connected' sql_store_result: returns -1 on failure, SQL_DOWN on 'socket not connected' +sql_store_next_result: returns -1 on failure, SQL_DOWN on 'socket not connected', + SQL_NO_MORE_RESULTS on 'no more results' sql_num_fields: cannot return an error, complains if zero fields sql_finish_select_query: returns 0 always sql_finish_query: does nothing, returns 0 diff -ruN freeradius-1.1.6.mysql5_sp.v3/src/modules/rlm_sql/rlm_sql.h freeradius-1.1.6.multi_results.v1/src/modules/rlm_sql/rlm_sql.h --- freeradius-1.1.6.mysql5_sp.v3/src/modules/rlm_sql/rlm_sql.h 2003-09-03 18:19:32.000000000 +0300 +++ freeradius-1.1.6.multi_results.v1/src/modules/rlm_sql/rlm_sql.h 2007-05-17 18:39:15.000000000 +0300 @@ -48,6 +48,7 @@ int (*sql_query)(SQLSOCK *sqlsocket, SQL_CONFIG *config, char *query); int (*sql_select_query)(SQLSOCK *sqlsocket, SQL_CONFIG *config, char *query); int (*sql_store_result)(SQLSOCK *sqlsocket, SQL_CONFIG *config); + int (*sql_store_next_result)(SQLSOCK *sqlsocket, SQL_CONFIG *config); int (*sql_num_fields)(SQLSOCK *sqlsocket, SQL_CONFIG *config); int (*sql_num_rows)(SQLSOCK *sqlsocket, SQL_CONFIG *config); int (*sql_fetch_row)(SQLSOCK *sqlsocket, SQL_CONFIG *config); @@ -85,4 +86,5 @@ int rlm_sql_select_query(SQLSOCK *sqlsocket, SQL_INST *inst, char *query); int rlm_sql_query(SQLSOCK *sqlsocket, SQL_INST *inst, char *query); int rlm_sql_fetch_row(SQLSOCK *sqlsocket, SQL_INST *inst); +int rlm_sql_store_next_result(SQLSOCK *sqlsocket, SQL_INST *inst); #endif diff -ruN freeradius-1.1.6.mysql5_sp.v3/src/modules/rlm_sql/sql.c freeradius-1.1.6.multi_results.v1/src/modules/rlm_sql/sql.c --- freeradius-1.1.6.mysql5_sp.v3/src/modules/rlm_sql/sql.c 2007-03-26 12:46:28.000000000 +0300 +++ freeradius-1.1.6.multi_results.v1/src/modules/rlm_sql/sql.c 2007-05-17 19:09:28.000000000 +0300 @@ -474,6 +474,50 @@ return ret; } + +/************************************************************************* + * + * Function: rlm_sql_store_next_result + * + * Purpose: call the module's sql_store_next_result and implement re-connect + * + *************************************************************************/ +int rlm_sql_store_next_result(SQLSOCK *sqlsocket, SQL_INST *inst) +{ + int ret; + + if (sqlsocket->conn) { + ret = (inst->module->sql_store_next_result)(sqlsocket, inst->config); + } else { + ret = SQL_DOWN; + } + + if (ret == SQL_DOWN) { + /* close the socket that failed, but only if it was open */ + if (sqlsocket->conn) { + (inst->module->sql_close)(sqlsocket, inst->config); + } + + /* reconnect the socket */ + if (connect_single_socket(sqlsocket, inst) < 0) { + radlog(L_ERR, "rlm_sql (%s): reconnect failed, database down?", inst->config->xlat_name); + return -1; + } + + /* retry the query on the newly connected socket */ + ret = (inst->module->sql_store_next_result)(sqlsocket, inst->config); + + if (ret && ret != SQL_NO_MORE_RESULTS) { + radlog(L_ERR, "rlm_sql (%s): failed after re-connect", + inst->config->xlat_name); + return -1; + } + } + + return ret; +} + + /************************************************************************* * * Function: rlm_sql_query @@ -519,6 +563,7 @@ return ret; } + /************************************************************************* * * Function: rlm_sql_select_query @@ -576,6 +621,7 @@ { SQL_ROW row; int rows = 0; + int status; /* * If there's no query, return an error. @@ -588,16 +634,24 @@ radlog(L_ERR, "rlm_sql_getvpdata: database query error"); return -1; } - while (rlm_sql_fetch_row(sqlsocket, inst)==0) { - row = sqlsocket->row; - if (!row) + for (;;) { + 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++; + } + if ((status = rlm_sql_store_next_result(sqlsocket, inst))) { + if (status != SQL_NO_MORE_RESULTS) { + /* simply ignore the error */ + } 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);
participants (3)
-
Alan DeKok -
Peter Nixon -
Stanislav Schukin