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);