[svn-commits] mjordan: branch 1.8 r411399 - in /branches/1.8: ./ configs/ include/asterisk/...

SVN commits to the Digium repositories svn-commits at lists.digium.com
Thu Mar 27 23:24:14 CDT 2014


Author: mjordan
Date: Thu Mar 27 23:24:07 2014
New Revision: 411399

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=411399
Log:
res_config_odbc/res_odbc: Fix handling of non-text columns updates with empty values.

This patch fixes setting nullable integer columns to NULL instead of an empty
string, which fails for PostgreSQL, for example. The current code is supposed
to do so, but the check is broken. The patch also allows the first column in
the list to be a nullable integer.

This patch also adds a compatibility setting in res_odbc.conf,
allow_empty_string_in_nontext. It is enabled by default. It should be disabled
for database backends (such as PostgreSQL) that require NULL instead of an
empty string for Integer columns.

Review: https://reviewboard.asterisk.org/r/3375

(issue ASTERISK-23459)
Reported by: zvision
patches:
  res_config_odbc.diff uploaded by zvision (License 5755)


Modified:
    branches/1.8/UPGRADE.txt
    branches/1.8/configs/res_odbc.conf.sample
    branches/1.8/include/asterisk/res_odbc.h
    branches/1.8/res/res_config_odbc.c
    branches/1.8/res/res_odbc.c
    branches/1.8/res/res_odbc.exports.in

Modified: branches/1.8/UPGRADE.txt
URL: http://svnview.digium.com/svn/asterisk/branches/1.8/UPGRADE.txt?view=diff&rev=411399&r1=411398&r2=411399
==============================================================================
--- branches/1.8/UPGRADE.txt (original)
+++ branches/1.8/UPGRADE.txt Thu Mar 27 23:24:07 2014
@@ -17,11 +17,21 @@
 === UPGRADE-1.6.txt -- Upgrade info for 1.4 to 1.6
 ===
 ===========================================================
+
 from 1.8.26.0 to 1.8.27.0:
 * res_fax now returns the correct rates for V.27ter (4800 or 9600 bit/s).
   Because of this the default settings would not load, so the minrate (minimum
   transmission rate) option was changed to default to 4800 since that is the
   minimum rate for v.27 which is included in the default modem options.
+
+* A compatibility setting, allow_empty_string_in_nontext, has been added to
+  res_odbc.conf. When enabled (default behavior), empty column values are
+  stored as empty strings during realtime updates. Disabling this option
+  causes empty column values to be stored as NULLs for non-text columns.
+
+  Disable it for PostgreSQL backends in order to avoid errors caused by
+  updating integer columns with an empty string instead of NULL
+  (sipppeers,sipregs)
 
 from 1.8.23.0 to 1.8.24.0:
 * res_agi will now properly indicate if there was an error in streaming an

Modified: branches/1.8/configs/res_odbc.conf.sample
URL: http://svnview.digium.com/svn/asterisk/branches/1.8/configs/res_odbc.conf.sample?view=diff&rev=411399&r1=411398&r2=411399
==============================================================================
--- branches/1.8/configs/res_odbc.conf.sample (original)
+++ branches/1.8/configs/res_odbc.conf.sample Thu Mar 27 23:24:07 2014
@@ -64,6 +64,14 @@
 ; MS SQL Server, the answer is no.
 ;backslash_is_escape => yes
 ;
+; When enabled (default behavior), empty column values are stored as empty strings
+; during realtime updates. Disabling this option causes empty column values to be
+; stored as NULLs for non-text columns.
+; Disable it for PostgreSQL backend in order to avoid errors caused by updating
+; integer columns with an empty string instead of NULL (sipppeers,sipregs)
+; NOTE: This option will be removed in asterisk 13. At that point, it will always behave as if it was set to 'no'.
+;allow_empty_string_in_nontext => yes
+;
 ; How long (in seconds) should we attempt to connect before considering the
 ; connection dead?  The default is 10 seconds, but you may wish to reduce it,
 ; to increase responsiveness.

Modified: branches/1.8/include/asterisk/res_odbc.h
URL: http://svnview.digium.com/svn/asterisk/branches/1.8/include/asterisk/res_odbc.h?view=diff&rev=411399&r1=411398&r2=411399
==============================================================================
--- branches/1.8/include/asterisk/res_odbc.h (original)
+++ branches/1.8/include/asterisk/res_odbc.h Thu Mar 27 23:24:07 2014
@@ -222,4 +222,10 @@
  */
 SQLRETURN ast_odbc_ast_str_SQLGetData(struct ast_str **buf, int pmaxlen, SQLHSTMT StatementHandle, SQLUSMALLINT ColumnNumber, SQLSMALLINT TargetType, SQLLEN *StrLen_or_Ind);
 
+/*! \brief Checks if the database natively supports implicit conversion from an empty string to a number (0).
+ * \param obj The ODBC object
+ * \return Returns 1 if the implicit conversion is valid and non-text columns can take empty strings, 0 if the conversion is not valid and NULLs should be used instead '\'
+ */
+int ast_odbc_allow_empty_string_in_nontext(struct odbc_obj *obj);
+
 #endif /* _ASTERISK_RES_ODBC_H */

Modified: branches/1.8/res/res_config_odbc.c
URL: http://svnview.digium.com/svn/asterisk/branches/1.8/res/res_config_odbc.c?view=diff&rev=411399&r1=411398&r2=411399
==============================================================================
--- branches/1.8/res/res_config_odbc.c (original)
+++ branches/1.8/res/res_config_odbc.c Thu Mar 27 23:24:07 2014
@@ -67,6 +67,12 @@
 			memmove(chunk + 1, chunk + 3, strlen(chunk + 3) + 1);
 		}
 	}
+}
+
+static inline int is_text(const struct odbc_cache_columns *column)
+{
+	return column->type == SQL_CHAR || column->type == SQL_VARCHAR || column->type == SQL_LONGVARCHAR
+		|| column->type == SQL_WCHAR || column->type == SQL_WVARCHAR || column->type == SQL_WLONGVARCHAR;
 }
 
 static SQLHSTMT custom_prepare(struct odbc_obj *obj, void *data)
@@ -484,15 +490,15 @@
 	SQLHSTMT stmt;
 	char sql[256];
 	SQLLEN rowcount=0;
-	const char *newparam;
-	int res, count = 1;
+	const char *newparam, *newval;
+	int res, count = 0, paramcount = 0;
 	va_list aq;
 	struct custom_prepare_struct cps = { .sql = sql, .extra = lookup };
 	struct odbc_cache_tables *tableptr;
 	struct odbc_cache_columns *column = NULL;
 	struct ast_flags connected_flag = { RES_ODBC_CONNECTED };
 
-	if (!table) {
+	if (!table || !keyfield) {
 		return -1;
 	}
 
@@ -507,39 +513,31 @@
 		return -1;
 	}
 
+	if (tableptr && !ast_odbc_find_column(tableptr, keyfield)) {
+		ast_log(LOG_WARNING, "Key field '%s' does not exist in table '%s@%s'.  Update will fail\n", keyfield, table, database);
+	}
+
 	va_copy(aq, ap);
-	newparam = va_arg(aq, const char *);
-	if (!newparam)  {
-		va_end(aq);
-		ast_odbc_release_obj(obj);
-		ast_odbc_release_table(tableptr);
-		ast_string_field_free_memory(&cps);
-		return -1;
-	}
-	va_arg(aq, const char *);
-
-	if (tableptr && !ast_odbc_find_column(tableptr, newparam)) {
-		ast_log(LOG_WARNING, "Key field '%s' does not exist in table '%s@%s'.  Update will fail\n", newparam, table, database);
-	}
-
-	snprintf(sql, sizeof(sql), "UPDATE %s SET %s=?", table, newparam);
+
+	snprintf(sql, sizeof(sql), "UPDATE %s SET ", table);
 	while((newparam = va_arg(aq, const char *))) {
-		va_arg(aq, const char *);
-		if ((tableptr && (column = ast_odbc_find_column(tableptr, newparam))) || count > 63) {
-			/* NULL test for integer-based columns */
-			if (ast_strlen_zero(newparam) && tableptr && column && column->nullable && count < 64 &&
-				(column->type == SQL_INTEGER || column->type == SQL_BIGINT ||
-				 column->type == SQL_SMALLINT || column->type == SQL_TINYINT ||
-				 column->type == SQL_NUMERIC || column->type == SQL_DECIMAL)) {
-				snprintf(sql + strlen(sql), sizeof(sql) - strlen(sql), ", %s=NULL", newparam);
+		newval = va_arg(aq, const char *);
+		if ((tableptr && (column = ast_odbc_find_column(tableptr, newparam))) || count >= 64) {
+			if (paramcount++) {
+				snprintf(sql + strlen(sql), sizeof(sql) - strlen(sql), ", ");
+			}
+			/* NULL test for non-text columns */
+			if (count < 64 && ast_strlen_zero(newval) && column->nullable && !is_text(column) && !ast_odbc_allow_empty_string_in_nontext(obj)) {
+				snprintf(sql + strlen(sql), sizeof(sql) - strlen(sql), "%s=NULL", newparam);
 				cps.skip |= (1LL << count);
 			} else {
-				snprintf(sql + strlen(sql), sizeof(sql) - strlen(sql), ", %s=?", newparam);
+				/* Value is not an empty string, or column accepts empty strings, or we couldn't fit any more into cps.skip (count >= 64 ?!). */
+				snprintf(sql + strlen(sql), sizeof(sql) - strlen(sql), "%s=?", newparam);
 			}
 		} else { /* the column does not exist in the table */
 			cps.skip |= (1LL << count);
 		}
-		count++;
+		++count;
 	}
 	va_end(aq);
 	snprintf(sql + strlen(sql), sizeof(sql) - strlen(sql), " WHERE %s=?", keyfield);

Modified: branches/1.8/res/res_odbc.c
URL: http://svnview.digium.com/svn/asterisk/branches/1.8/res/res_odbc.c?view=diff&rev=411399&r1=411398&r2=411399
==============================================================================
--- branches/1.8/res/res_odbc.c (original)
+++ branches/1.8/res/res_odbc.c Thu Mar 27 23:24:07 2014
@@ -128,6 +128,7 @@
 	unsigned int delme:1;                /*!< Purge the class */
 	unsigned int backslash_is_escape:1;  /*!< On this database, the backslash is a native escape sequence */
 	unsigned int forcecommit:1;          /*!< Should uncommitted transactions be auto-committed on handle release? */
+	unsigned int allow_empty_strings:1;  /*!< Implicit conversion from an empty string to a number is valid for this database */
 	unsigned int isolation;              /*!< Flags for how the DB should deal with data in other, uncommitted transactions */
 	unsigned int limit;                  /*!< Maximum number of database handles we will allow */
 	int count;                           /*!< Running count of pooled connections */
@@ -773,7 +774,7 @@
 	struct ast_variable *v;
 	char *cat;
 	const char *dsn, *username, *password, *sanitysql;
-	int enabled, pooling, limit, bse, conntimeout, forcecommit, isolation;
+	int enabled, pooling, limit, bse, conntimeout, forcecommit, isolation, allow_empty_strings;
 	struct timeval ncache = { 0, 0 };
 	unsigned int idlecheck;
 	int preconnect = 0, res = 0;
@@ -802,6 +803,7 @@
 			bse = 1;
 			conntimeout = 10;
 			forcecommit = 0;
+			allow_empty_strings = 1;
 			isolation = SQL_TXN_READ_COMMITTED;
 			for (v = ast_variable_browse(config, cat); v; v = v->next) {
 				if (!strcasecmp(v->name, "pooling")) {
@@ -837,6 +839,8 @@
 					sanitysql = v->value;
 				} else if (!strcasecmp(v->name, "backslash_is_escape")) {
 					bse = ast_true(v->value);
+				} else if (!strcasecmp(v->name, "allow_empty_string_in_nontext")) {
+					allow_empty_strings = ast_true(v->value);
 				} else if (!strcasecmp(v->name, "connect_timeout")) {
 					if (sscanf(v->value, "%d", &conntimeout) != 1 || conntimeout < 1) {
 						ast_log(LOG_WARNING, "connect_timeout must be a positive integer\n");
@@ -898,6 +902,7 @@
 				new->idlecheck = idlecheck;
 				new->conntimeout = conntimeout;
 				new->negative_connection_cache = ncache;
+				new->allow_empty_strings = allow_empty_strings ? 1 : 0;
 
 				if (cat)
 					ast_copy_string(new->name, cat, sizeof(new->name));
@@ -1113,6 +1118,11 @@
 int ast_odbc_backslash_is_escape(struct odbc_obj *obj)
 {
 	return obj->parent->backslash_is_escape;
+}
+
+int ast_odbc_allow_empty_string_in_nontext(struct odbc_obj *obj)
+{
+	return obj->parent->allow_empty_strings;
 }
 
 static int commit_exec(struct ast_channel *chan, const char *data)

Modified: branches/1.8/res/res_odbc.exports.in
URL: http://svnview.digium.com/svn/asterisk/branches/1.8/res/res_odbc.exports.in?view=diff&rev=411399&r1=411398&r2=411399
==============================================================================
--- branches/1.8/res/res_odbc.exports.in (original)
+++ branches/1.8/res/res_odbc.exports.in Thu Mar 27 23:24:07 2014
@@ -2,6 +2,7 @@
 	global:
 		LINKER_SYMBOL_PREFIXast_odbc_ast_str_SQLGetData;
 		LINKER_SYMBOL_PREFIXast_odbc_backslash_is_escape;
+		LINKER_SYMBOL_PREFIXast_odbc_allow_empty_string_in_nontext;
 		LINKER_SYMBOL_PREFIXast_odbc_clear_cache;
 		LINKER_SYMBOL_PREFIXast_odbc_direct_execute;
 		LINKER_SYMBOL_PREFIXast_odbc_find_column;




More information about the svn-commits mailing list