[Asterisk-code-review] cel, cdr: Assigned separator for column name and values. (asterisk[master])

Joshua Colp asteriskteam at digium.com
Fri May 22 05:16:30 CDT 2015


Joshua Colp has submitted this change and it was merged.

Change subject: cel, cdr: Assigned separator for column name and values.
......................................................................


cel, cdr: Assigned separator for column name and values.

Use a separator string between column names and values for SQL sentences
instead of evaluating the separator to use each time.

This change adds a space after the comma in constructing SQL sentences.
Before the SQL was created like "INSERT INTO cdr(calldate,clid,dst"
without spaces between column name and values.

The files applied this change are cdr/cdr_adaptive_odbc.c, cdr/cdr_pgsql.c,
cel/cel_odbc.c

ASTERISK-25109 #close
Reported By: Rodrigo Ramírez Norambuena <decipher.hk at gmail.com>

Change-Id: Ia5a1a161f5e26e1643703b30f8cc9cf0860cc7ea
---
M cdr/cdr_adaptive_odbc.c
M cdr/cdr_pgsql.c
M cel/cel_odbc.c
3 files changed, 66 insertions(+), 65 deletions(-)

Approvals:
  Ashley Sanders: Looks good to me, but someone else must approve
  Joshua Colp: Looks good to me, approved; Verified



diff --git a/cdr/cdr_adaptive_odbc.c b/cdr/cdr_adaptive_odbc.c
index e6b60dc..f276959 100644
--- a/cdr/cdr_adaptive_odbc.c
+++ b/cdr/cdr_adaptive_odbc.c
@@ -395,6 +395,8 @@
 	char colbuf[1024], *colptr;
 	SQLHSTMT stmt = NULL;
 	SQLLEN rows = 0;
+	char *separator;
+	int quoted = 0;
 
 	if (!sql || !sql2) {
 		if (sql)
@@ -412,8 +414,7 @@
 	}
 
 	AST_LIST_TRAVERSE(&odbc_tables, tableptr, list) {
-		int first = 1;
-		int quoted = 0;
+		separator = "";
 
 		if (tableptr->quoted_identifiers != '\0'){
 			quoted = 1;
@@ -517,7 +518,7 @@
 					LENGTHEN_BUF2(strlen(colptr));
 
 					/* Encode value, with escaping */
-					ast_str_append(&sql2, 0, "%s'", first ? "" : ",");
+					ast_str_append(&sql2, 0, "%s'", separator);
 					for (tmp = colptr; *tmp; tmp++) {
 						if (*tmp == '\'') {
 							ast_str_append(&sql2, 0, "''");
@@ -550,7 +551,7 @@
 						}
 
 						LENGTHEN_BUF2(17);
-						ast_str_append(&sql2, 0, "%s{ d '%04d-%02d-%02d' }", first ? "" : ",", year, month, day);
+						ast_str_append(&sql2, 0, "%s{ d '%04d-%02d-%02d' }", separator, year, month, day);
 					}
 					break;
 				case SQL_TYPE_TIME:
@@ -566,7 +567,7 @@
 						}
 
 						LENGTHEN_BUF2(15);
-						ast_str_append(&sql2, 0, "%s{ t '%02d:%02d:%02d' }", first ? "" : ",", hour, minute, second);
+						ast_str_append(&sql2, 0, "%s{ t '%02d:%02d:%02d' }", separator, hour, minute, second);
 					}
 					break;
 				case SQL_TYPE_TIMESTAMP:
@@ -594,7 +595,7 @@
 						}
 
 						LENGTHEN_BUF2(26);
-						ast_str_append(&sql2, 0, "%s{ ts '%04d-%02d-%02d %02d:%02d:%02d' }", first ? "" : ",", year, month, day, hour, minute, second);
+						ast_str_append(&sql2, 0, "%s{ ts '%04d-%02d-%02d %02d:%02d:%02d' }", separator, year, month, day, hour, minute, second);
 					}
 					break;
 				case SQL_INTEGER:
@@ -608,7 +609,7 @@
 						}
 
 						LENGTHEN_BUF2(12);
-						ast_str_append(&sql2, 0, "%s%d", first ? "" : ",", integer);
+						ast_str_append(&sql2, 0, "%s%d", separator, integer);
 					}
 					break;
 				case SQL_BIGINT:
@@ -622,7 +623,7 @@
 						}
 
 						LENGTHEN_BUF2(24);
-						ast_str_append(&sql2, 0, "%s%lld", first ? "" : ",", integer);
+						ast_str_append(&sql2, 0, "%s%lld", separator, integer);
 					}
 					break;
 				case SQL_SMALLINT:
@@ -636,7 +637,7 @@
 						}
 
 						LENGTHEN_BUF2(6);
-						ast_str_append(&sql2, 0, "%s%d", first ? "" : ",", integer);
+						ast_str_append(&sql2, 0, "%s%d", separator, integer);
 					}
 					break;
 				case SQL_TINYINT:
@@ -650,7 +651,7 @@
 						}
 
 						LENGTHEN_BUF2(4);
-						ast_str_append(&sql2, 0, "%s%d", first ? "" : ",", integer);
+						ast_str_append(&sql2, 0, "%s%d", separator, integer);
 					}
 					break;
 				case SQL_BIT:
@@ -666,7 +667,7 @@
 							integer = 1;
 
 						LENGTHEN_BUF2(2);
-						ast_str_append(&sql2, 0, "%s%d", first ? "" : ",", integer);
+						ast_str_append(&sql2, 0, "%s%d", separator, integer);
 					}
 					break;
 				case SQL_NUMERIC:
@@ -698,7 +699,7 @@
 						}
 
 						LENGTHEN_BUF2(entry->decimals);
-						ast_str_append(&sql2, 0, "%s%*.*lf", first ? "" : ",", entry->decimals, entry->radix, number);
+						ast_str_append(&sql2, 0, "%s%*.*lf", separator, entry->decimals, entry->radix, number);
 					}
 					break;
 				case SQL_FLOAT:
@@ -731,7 +732,7 @@
 						}
 
 						LENGTHEN_BUF2(entry->decimals);
-						ast_str_append(&sql2, 0, "%s%lf", first ? "" : ",", number);
+						ast_str_append(&sql2, 0, "%s%lf", separator, number);
 					}
 					break;
 				default:
@@ -739,11 +740,11 @@
 					continue;
 				}
 				if (quoted) {
-					ast_str_append(&sql, 0, "%s%s", first ? "" : ",", entry->name);
+					ast_str_append(&sql, 0, "%s%s", separator, entry->name);
 				} else {
-					ast_str_append(&sql, 0, "%s%c%s%c", first ? "" : ",", tableptr->quoted_identifiers, entry->name, tableptr->quoted_identifiers);
+					ast_str_append(&sql, 0, "%s%c%s%c", separator, tableptr->quoted_identifiers, entry->name, tableptr->quoted_identifiers);
 				}
-				first = 0;
+				separator = ", ";
 			} else if (entry->filtervalue
 				&& ((!entry->negatefiltervalue && entry->filtervalue[0] != '\0')
 					|| (entry->negatefiltervalue && entry->filtervalue[0] == '\0'))) {
diff --git a/cdr/cdr_pgsql.c b/cdr/cdr_pgsql.c
index 37bc084..8dc49e1 100644
--- a/cdr/cdr_pgsql.c
+++ b/cdr/cdr_pgsql.c
@@ -248,7 +248,7 @@
 		struct columns *cur;
 		struct ast_str *sql = ast_str_create(maxsize), *sql2 = ast_str_create(maxsize2);
 		char buf[257], escapebuf[513], *value;
-		int first = 1;
+		char *separator = "";
 
 		if (!sql || !sql2) {
 			ast_free(sql);
@@ -270,86 +270,86 @@
 				if (cur->notnull && !cur->hasdefault) {
 					/* Field is NOT NULL (but no default), must include it anyway */
 					LENGTHEN_BUF1(strlen(cur->name) + 2);
-					ast_str_append(&sql, 0, "%s\"%s\"", first ? "" : ",", cur->name);
+					ast_str_append(&sql, 0, "%s\"%s\"", separator, cur->name);
 					LENGTHEN_BUF2(3);
-					ast_str_append(&sql2, 0, "%s''", first ? "" : ",");
-					first = 0;
+					ast_str_append(&sql2, 0, "%s''", separator);
+					separator = ", ";
 				}
 				continue;
 			}
 
 			LENGTHEN_BUF1(strlen(cur->name) + 2);
-			ast_str_append(&sql, 0, "%s\"%s\"", first ? "" : ",", cur->name);
+			ast_str_append(&sql, 0, "%s\"%s\"", separator, cur->name);
 
 			if (strcmp(cur->name, "start") == 0 || strcmp(cur->name, "calldate") == 0) {
 				if (strncmp(cur->type, "int", 3) == 0) {
 					LENGTHEN_BUF2(13);
-					ast_str_append(&sql2, 0, "%s%ld", first ? "" : ",", (long) cdr->start.tv_sec);
+					ast_str_append(&sql2, 0, "%s%ld", separator, (long) cdr->start.tv_sec);
 				} else if (strncmp(cur->type, "float", 5) == 0) {
 					LENGTHEN_BUF2(31);
-					ast_str_append(&sql2, 0, "%s%f", first ? "" : ",", (double)cdr->start.tv_sec + (double)cdr->start.tv_usec / 1000000.0);
+					ast_str_append(&sql2, 0, "%s%f", separator, (double)cdr->start.tv_sec + (double)cdr->start.tv_usec / 1000000.0);
 				} else {
 					/* char, hopefully */
 					LENGTHEN_BUF2(31);
 					ast_localtime(&cdr->start, &tm, tz);
 					ast_strftime(buf, sizeof(buf), DATE_FORMAT, &tm);
-					ast_str_append(&sql2, 0, "%s%s", first ? "" : ",", buf);
+					ast_str_append(&sql2, 0, "%s%s", separator, buf);
 				}
 			} else if (strcmp(cur->name, "answer") == 0) {
 				if (strncmp(cur->type, "int", 3) == 0) {
 					LENGTHEN_BUF2(13);
-					ast_str_append(&sql2, 0, "%s%ld", first ? "" : ",", (long) cdr->answer.tv_sec);
+					ast_str_append(&sql2, 0, "%s%ld", separator, (long) cdr->answer.tv_sec);
 				} else if (strncmp(cur->type, "float", 5) == 0) {
 					LENGTHEN_BUF2(31);
-					ast_str_append(&sql2, 0, "%s%f", first ? "" : ",", (double)cdr->answer.tv_sec + (double)cdr->answer.tv_usec / 1000000.0);
+					ast_str_append(&sql2, 0, "%s%f", separator, (double)cdr->answer.tv_sec + (double)cdr->answer.tv_usec / 1000000.0);
 				} else {
 					/* char, hopefully */
 					LENGTHEN_BUF2(31);
 					ast_localtime(&cdr->answer, &tm, tz);
 					ast_strftime(buf, sizeof(buf), DATE_FORMAT, &tm);
-					ast_str_append(&sql2, 0, "%s%s", first ? "" : ",", buf);
+					ast_str_append(&sql2, 0, "%s%s", separator, buf);
 				}
 			} else if (strcmp(cur->name, "end") == 0) {
 				if (strncmp(cur->type, "int", 3) == 0) {
 					LENGTHEN_BUF2(13);
-					ast_str_append(&sql2, 0, "%s%ld", first ? "" : ",", (long) cdr->end.tv_sec);
+					ast_str_append(&sql2, 0, "%s%ld", separator, (long) cdr->end.tv_sec);
 				} else if (strncmp(cur->type, "float", 5) == 0) {
 					LENGTHEN_BUF2(31);
-					ast_str_append(&sql2, 0, "%s%f", first ? "" : ",", (double)cdr->end.tv_sec + (double)cdr->end.tv_usec / 1000000.0);
+					ast_str_append(&sql2, 0, "%s%f", separator, (double)cdr->end.tv_sec + (double)cdr->end.tv_usec / 1000000.0);
 				} else {
 					/* char, hopefully */
 					LENGTHEN_BUF2(31);
 					ast_localtime(&cdr->end, &tm, tz);
 					ast_strftime(buf, sizeof(buf), DATE_FORMAT, &tm);
-					ast_str_append(&sql2, 0, "%s%s", first ? "" : ",", buf);
+					ast_str_append(&sql2, 0, "%s%s", separator, buf);
 				}
 			} else if (strcmp(cur->name, "duration") == 0 || strcmp(cur->name, "billsec") == 0) {
 				if (cur->type[0] == 'i') {
 					/* Get integer, no need to escape anything */
 					ast_cdr_format_var(cdr, cur->name, &value, buf, sizeof(buf), 0);
 					LENGTHEN_BUF2(13);
-					ast_str_append(&sql2, 0, "%s%s", first ? "" : ",", value);
+					ast_str_append(&sql2, 0, "%s%s", separator, value);
 				} else if (strncmp(cur->type, "float", 5) == 0) {
 					struct timeval *when = cur->name[0] == 'd' ? &cdr->start : ast_tvzero(cdr->answer) ? &cdr->end : &cdr->answer;
 					LENGTHEN_BUF2(31);
-					ast_str_append(&sql2, 0, "%s%f", first ? "" : ",", (double) (ast_tvdiff_us(cdr->end, *when) / 1000000.0));
+					ast_str_append(&sql2, 0, "%s%f", separator, (double) (ast_tvdiff_us(cdr->end, *when) / 1000000.0));
 				} else {
 					/* Char field, probably */
 					struct timeval *when = cur->name[0] == 'd' ? &cdr->start : ast_tvzero(cdr->answer) ? &cdr->end : &cdr->answer;
 					LENGTHEN_BUF2(31);
-					ast_str_append(&sql2, 0, "%s'%f'", first ? "" : ",", (double) (ast_tvdiff_us(cdr->end, *when) / 1000000.0));
+					ast_str_append(&sql2, 0, "%s'%f'", separator, (double) (ast_tvdiff_us(cdr->end, *when) / 1000000.0));
 				}
 			} else if (strcmp(cur->name, "disposition") == 0 || strcmp(cur->name, "amaflags") == 0) {
 				if (strncmp(cur->type, "int", 3) == 0) {
 					/* Integer, no need to escape anything */
 					ast_cdr_format_var(cdr, cur->name, &value, buf, sizeof(buf), 1);
 					LENGTHEN_BUF2(13);
-					ast_str_append(&sql2, 0, "%s%s", first ? "" : ",", value);
+					ast_str_append(&sql2, 0, "%s%s", separator, value);
 				} else {
 					/* Although this is a char field, there are no special characters in the values for these fields */
 					ast_cdr_format_var(cdr, cur->name, &value, buf, sizeof(buf), 0);
 					LENGTHEN_BUF2(31);
-					ast_str_append(&sql2, 0, "%s'%s'", first ? "" : ",", value);
+					ast_str_append(&sql2, 0, "%s'%s'", separator, value);
 				}
 			} else {
 				/* Arbitrary field, could be anything */
@@ -358,19 +358,19 @@
 					long long whatever;
 					if (value && sscanf(value, "%30lld", &whatever) == 1) {
 						LENGTHEN_BUF2(26);
-						ast_str_append(&sql2, 0, "%s%lld", first ? "" : ",", whatever);
+						ast_str_append(&sql2, 0, "%s%lld", separator, whatever);
 					} else {
 						LENGTHEN_BUF2(2);
-						ast_str_append(&sql2, 0, "%s0", first ? "" : ",");
+						ast_str_append(&sql2, 0, "%s0", separator);
 					}
 				} else if (strncmp(cur->type, "float", 5) == 0) {
 					long double whatever;
 					if (value && sscanf(value, "%30Lf", &whatever) == 1) {
 						LENGTHEN_BUF2(51);
-						ast_str_append(&sql2, 0, "%s%30Lf", first ? "" : ",", whatever);
+						ast_str_append(&sql2, 0, "%s%30Lf", separator, whatever);
 					} else {
 						LENGTHEN_BUF2(2);
-						ast_str_append(&sql2, 0, "%s0", first ? "" : ",");
+						ast_str_append(&sql2, 0, "%s0", separator);
 					}
 				/* XXX Might want to handle dates, times, and other misc fields here XXX */
 				} else {
@@ -379,10 +379,10 @@
 					else
 						escapebuf[0] = '\0';
 					LENGTHEN_BUF2(strlen(escapebuf) + 3);
-					ast_str_append(&sql2, 0, "%s'%s'", first ? "" : ",", escapebuf);
+					ast_str_append(&sql2, 0, "%s'%s'", separator, escapebuf);
 				}
 			}
-			first = 0;
+			separator = ", ";
 		}
 
 		LENGTHEN_BUF1(ast_str_strlen(sql2) + 2);
diff --git a/cel/cel_odbc.c b/cel/cel_odbc.c
index 4803444..2d8408b 100644
--- a/cel/cel_odbc.c
+++ b/cel/cel_odbc.c
@@ -402,7 +402,7 @@
 	}
 
 	AST_LIST_TRAVERSE(&odbc_tables, tableptr, list) {
-		int first = 1;
+		char *separator = "";
 		ast_str_set(&sql, 0, "INSERT INTO %s (", tableptr->table);
 		ast_str_set(&sql2, 0, " VALUES (");
 
@@ -536,11 +536,11 @@
 						}
 					}
 
-					ast_str_append(&sql, 0, "%s%s", first ? "" : ",", entry->name);
+					ast_str_append(&sql, 0, "%s%s", separator, entry->name);
 					LENGTHEN_BUF2(strlen(colptr));
 
 					/* Encode value, with escaping */
-					ast_str_append(&sql2, 0, "%s'", first ? "" : ",");
+					ast_str_append(&sql2, 0, "%s'", separator);
 					for (tmp = colptr; *tmp; tmp++) {
 						if (*tmp == '\'') {
 							ast_str_append(&sql2, 0, "''");
@@ -580,9 +580,9 @@
 							}
 						}
 
-						ast_str_append(&sql, 0, "%s%s", first ? "" : ",", entry->name);
+						ast_str_append(&sql, 0, "%s%s", separator, entry->name);
 						LENGTHEN_BUF2(17);
-						ast_str_append(&sql2, 0, "%s{d '%04d-%02d-%02d'}", first ? "" : ",", year, month, day);
+						ast_str_append(&sql2, 0, "%s{d '%04d-%02d-%02d'}", separator, year, month, day);
 					}
 					break;
 				case SQL_TYPE_TIME:
@@ -605,9 +605,9 @@
 							}
 						}
 
-						ast_str_append(&sql, 0, "%s%s", first ? "" : ",", entry->name);
+						ast_str_append(&sql, 0, "%s%s", separator, entry->name);
 						LENGTHEN_BUF2(15);
-						ast_str_append(&sql2, 0, "%s{t '%02d:%02d:%02d'}", first ? "" : ",", hour, minute, second);
+						ast_str_append(&sql2, 0, "%s{t '%02d:%02d:%02d'}", separator, hour, minute, second);
 					}
 					break;
 				case SQL_TYPE_TIMESTAMP:
@@ -646,9 +646,9 @@
 							}
 						}
 
-						ast_str_append(&sql, 0, "%s%s", first ? "" : ",", entry->name);
+						ast_str_append(&sql, 0, "%s%s", separator, entry->name);
 						LENGTHEN_BUF2(27);
-						ast_str_append(&sql2, 0, "%s{ts '%04d-%02d-%02d %02d:%02d:%02d.%d'}", first ? "" : ",", year, month, day, hour, minute, second, fraction);
+						ast_str_append(&sql2, 0, "%s{ts '%04d-%02d-%02d %02d:%02d:%02d.%d'}", separator, year, month, day, hour, minute, second, fraction);
 					}
 					break;
 				case SQL_INTEGER:
@@ -659,9 +659,9 @@
 							continue;
 						}
 
-						ast_str_append(&sql, 0, "%s%s", first ? "" : ",", entry->name);
+						ast_str_append(&sql, 0, "%s%s", separator, entry->name);
 						LENGTHEN_BUF2(12);
-						ast_str_append(&sql2, 0, "%s%d", first ? "" : ",", integer);
+						ast_str_append(&sql2, 0, "%s%d", separator, integer);
 					}
 					break;
 				case SQL_BIGINT:
@@ -673,9 +673,9 @@
 							continue;
 						}
 
-						ast_str_append(&sql, 0, "%s%s", first ? "" : ",", entry->name);
+						ast_str_append(&sql, 0, "%s%s", separator, entry->name);
 						LENGTHEN_BUF2(24);
-						ast_str_append(&sql2, 0, "%s%lld", first ? "" : ",", integer);
+						ast_str_append(&sql2, 0, "%s%lld", separator, integer);
 					}
 					break;
 				case SQL_SMALLINT:
@@ -686,9 +686,9 @@
 							continue;
 						}
 
-						ast_str_append(&sql, 0, "%s%s", first ? "" : ",", entry->name);
+						ast_str_append(&sql, 0, "%s%s", separator, entry->name);
 						LENGTHEN_BUF2(7);
-						ast_str_append(&sql2, 0, "%s%d", first ? "" : ",", integer);
+						ast_str_append(&sql2, 0, "%s%d", separator, integer);
 					}
 					break;
 				case SQL_TINYINT:
@@ -699,9 +699,9 @@
 							continue;
 						}
 
-						ast_str_append(&sql, 0, "%s%s", first ? "" : ",", entry->name);
+						ast_str_append(&sql, 0, "%s%s", separator, entry->name);
 						LENGTHEN_BUF2(4);
-						ast_str_append(&sql2, 0, "%s%d", first ? "" : ",", integer);
+						ast_str_append(&sql2, 0, "%s%d", separator, integer);
 					}
 					break;
 				case SQL_BIT:
@@ -714,9 +714,9 @@
 						if (integer != 0)
 							integer = 1;
 
-						ast_str_append(&sql, 0, "%s%s", first ? "" : ",", entry->name);
+						ast_str_append(&sql, 0, "%s%s", separator, entry->name);
 						LENGTHEN_BUF2(2);
-						ast_str_append(&sql2, 0, "%s%d", first ? "" : ",", integer);
+						ast_str_append(&sql2, 0, "%s%d", separator, integer);
 					}
 					break;
 				case SQL_NUMERIC:
@@ -728,9 +728,9 @@
 							continue;
 						}
 
-						ast_str_append(&sql, 0, "%s%s", first ? "" : ",", entry->name);
+						ast_str_append(&sql, 0, "%s%s", separator, entry->name);
 						LENGTHEN_BUF2(entry->decimals + 2);
-						ast_str_append(&sql2, 0, "%s%*.*lf", first ? "" : ",", entry->decimals, entry->radix, number);
+						ast_str_append(&sql2, 0, "%s%*.*lf", separator, entry->decimals, entry->radix, number);
 					}
 					break;
 				case SQL_FLOAT:
@@ -743,16 +743,16 @@
 							continue;
 						}
 
-						ast_str_append(&sql, 0, "%s%s", first ? "" : ",", entry->name);
+						ast_str_append(&sql, 0, "%s%s", separator, entry->name);
 						LENGTHEN_BUF2(entry->decimals);
-						ast_str_append(&sql2, 0, "%s%lf", first ? "" : ",", number);
+						ast_str_append(&sql2, 0, "%s%lf", separator, number);
 					}
 					break;
 				default:
 					ast_log(LOG_WARNING, "Column type %d (field '%s:%s:%s') is unsupported at this time.\n", entry->type, tableptr->connection, tableptr->table, entry->name);
 					continue;
 				}
-				first = 0;
+				separator = ", ";
 			}
 		}
 

-- 
To view, visit https://gerrit.asterisk.org/469
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: Ia5a1a161f5e26e1643703b30f8cc9cf0860cc7ea
Gerrit-PatchSet: 4
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Rodrigo Ramirez Norambuena <decipher.hk at gmail.com>
Gerrit-Reviewer: Ashley Sanders <asanders at digium.com>
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
Gerrit-Reviewer: Rodrigo Ramirez Norambuena <decipher.hk at gmail.com>



More information about the asterisk-code-review mailing list