[open-ils-commits] r12487 - trunk/Open-ILS/src/c-apps (scottmk)

svn at svn.open-ils.org svn at svn.open-ils.org
Wed Mar 11 14:57:20 EDT 2009


Author: scottmk
Date: 2009-03-11 14:57:16 -0400 (Wed, 11 Mar 2009)
New Revision: 12487

Modified:
   trunk/Open-ILS/src/c-apps/oils_cstore.c
Log:
When inserting a literal value into a SELECT statement:
whenever possible, leave the value unquoted if it is known to be
numeric, i.e. it is carried as a JSON_NUMBER, regardless of the
datatype as inferred from the associated column.  Reason: so that
the test_json_query utility (which currently doesn't look up the
datatypes of the columns) can generate the correct SQL most of
the time.  This approach should also be slightly faster, since it
bypasses some hashed lookups.

2. As part of the implementation of the change described above:
combine searchSimplePredicate() and searchWriteSimplePredicate()
into a single function, so that the JSON type is known when it's
time do decide whether to add quotes.  This change is benign because
the latter function was called only by the former anyway.

3. Several minor rearrangements and optimizations.


Modified: trunk/Open-ILS/src/c-apps/oils_cstore.c
===================================================================
--- trunk/Open-ILS/src/c-apps/oils_cstore.c	2009-03-11 05:06:32 UTC (rev 12486)
+++ trunk/Open-ILS/src/c-apps/oils_cstore.c	2009-03-11 18:57:16 UTC (rev 12487)
@@ -54,9 +54,8 @@
 static jsonObject* oilsMakeFieldmapperFromResult( dbi_result, osrfHash* );
 static jsonObject* oilsMakeJSONFromResult( dbi_result );
 
-static char* searchWriteSimplePredicate ( const char*, osrfHash*,
-        const char*, const char*, const char* );
-static char* searchSimplePredicate ( const char*, const char*, osrfHash*, const jsonObject* );
+static char* searchSimplePredicate ( const char* op, const char* class, 
+				osrfHash* field, const jsonObject* node );
 static char* searchFunctionPredicate ( const char*, osrfHash*, const jsonObject*, const char* );
 static char* searchFieldTransform ( const char*, osrfHash*, const jsonObject*);
 static char* searchFieldTransformPredicate ( const char*, osrfHash*, jsonObject*, const char* );
@@ -1323,7 +1322,11 @@
 
 	osrfLogDebug( OSRF_LOG_MARK, "Object seems to be of the correct type" );
 
-	if (!ctx->session || !ctx->session->userData || !osrfHashGet( (osrfHash*)ctx->session->userData, "xact_id" )) {
+	char* trans_id = NULL;
+	if( ctx->session && ctx->session->userData )
+		trans_id = osrfHashGet( (osrfHash*)ctx->session->userData, "xact_id" );
+
+	if ( !trans_id ) {
 		osrfLogError( OSRF_LOG_MARK, "No active transaction -- required for CREATE" );
 
 		osrfAppSessionStatus(
@@ -1351,10 +1354,7 @@
 		return jsonNULL;
 	}
 
-
-	char* trans_id = osrfHashGet( (osrfHash*)ctx->session->userData, "xact_id" );
-
-        // Set the last_xact_id
+	// Set the last_xact_id
 	int index = oilsIDL_ntop( target->classname, "last_xact_id" );
 	if (index > -1) {
 		osrfLogDebug(OSRF_LOG_MARK, "Setting last_xact_id to %s on %s at position %d", trans_id, target->classname, index);
@@ -1677,23 +1677,29 @@
         free(subpred);
 
     } else if (node->type == JSON_ARRAY) {
-        // litteral value list
+        // literal value list
     	int in_item_index = 0;
     	int in_item_first = 1;
-    	jsonObject* in_item;
+    	const jsonObject* in_item;
     	while ( (in_item = jsonObjectGetIndex(node, in_item_index++)) ) {
-    
-    		if (in_item_first)
-    			in_item_first = 0;
-    		else
-    			buffer_add(sql_buf, ", ");
-    
-    		if ( !strcmp( get_primitive( field ), "number") ) {
-    			char* val = jsonNumberToDBString( field, in_item );
-    			OSRF_BUFFER_ADD( sql_buf, val );
-    			free(val);
-    
-    		} else {
+
+			if (in_item_first)
+				in_item_first = 0;
+			else
+				buffer_add(sql_buf, ", ");
+
+			// Append the literal value -- quoted if not a number
+			if ( JSON_NUMBER == in_item->type ) {
+				char* val = jsonNumberToDBString( field, in_item );
+				OSRF_BUFFER_ADD( sql_buf, val );
+				free(val);
+
+			} else if ( !strcmp( get_primitive( field ), "number") ) {
+				char* val = jsonNumberToDBString( field, in_item );
+				OSRF_BUFFER_ADD( sql_buf, val );
+				free(val);
+
+			} else {
     			char* key_string = jsonObjectToSimpleString(in_item);
     			if ( dbi_conn_quote_string(dbhandle, &key_string) ) {
     				OSRF_BUFFER_ADD( sql_buf, key_string );
@@ -1848,17 +1854,20 @@
 	char* field_transform = searchFieldTransform( class, field, node );
 	char* value = NULL;
 
-	if (!jsonObjectGetKeyConst( node, "value" )) {
+	const jsonObject* value_obj = jsonObjectGetKeyConst( node, "value" );
+	if ( ! value_obj ) {
 		value = searchWHERE( node, osrfHashGet( oilsIDL(), class ), AND_OP_JOIN, NULL );
-	} else if (jsonObjectGetKeyConst( node, "value" )->type == JSON_ARRAY) {
-		value = searchValueTransform(jsonObjectGetKeyConst( node, "value" ));
-	} else if (jsonObjectGetKeyConst( node, "value" )->type == JSON_HASH) {
-		value = searchWHERE( jsonObjectGetKeyConst( node, "value" ), osrfHashGet( oilsIDL(), class ), AND_OP_JOIN, NULL );
-	} else if (jsonObjectGetKeyConst( node, "value" )->type != JSON_NULL) {
+	} else if ( value_obj->type == JSON_ARRAY ) {
+		value = searchValueTransform( value_obj );
+	} else if ( value_obj->type == JSON_HASH ) {
+		value = searchWHERE( value_obj, osrfHashGet( oilsIDL(), class ), AND_OP_JOIN, NULL );
+	} else if ( value_obj->type == JSON_NUMBER ) {
+		value = jsonNumberToDBString( field, value_obj );
+	} else if ( value_obj->type != JSON_NULL ) {
 		if ( !strcmp( get_primitive( field ), "number") ) {
-			value = jsonNumberToDBString( field, jsonObjectGetKeyConst( node, "value" ) );
+			value = jsonNumberToDBString( field, value_obj );
 		} else {
-			value = jsonObjectToSimpleString(jsonObjectGetKeyConst( node, "value" ));
+			value = jsonObjectToSimpleString( value_obj );
 			if ( !dbi_conn_quote_string(dbhandle, &value) ) {
 				osrfLogError(OSRF_LOG_MARK, "%s: Error quoting key string [%s]", MODULENAME, value);
 				free(value);
@@ -1884,59 +1893,47 @@
 	return buffer_release(sql_buf);
 }
 
-static char* searchSimplePredicate (const char* orig_op, const char* class,
+static char* searchSimplePredicate (const char* op, const char* class,
 		osrfHash* field, const jsonObject* node) {
 
 	char* val = NULL;
 
+	// Get the value to which we are comparing the specified column
 	if (node->type != JSON_NULL) {
-		if ( !strcmp( get_primitive( field ), "number") ) {
+		if ( node->type == JSON_NUMBER ) {
 			val = jsonNumberToDBString( field, node );
+		} else if ( !strcmp( get_primitive( field ), "number" ) ) {
+			val = jsonNumberToDBString( field, node );
 		} else {
 			val = jsonObjectToSimpleString(node);
 		}
 	}
 
-	char* pred = searchWriteSimplePredicate( class, field, osrfHashGet(field, "name"), orig_op, val );
-
-	if (val) free(val);
-
-	return pred;
-}
-
-static char* searchWriteSimplePredicate ( const char* class, osrfHash* field,
-	const char* left, const char* orig_op, const char* right ) {
-
-	char* val = NULL;
-	char* op = NULL;
-	if (right == NULL) {
-		val = strdup("NULL");
-
-		if (strcmp( orig_op, "=" ))
-			op = strdup("IS NOT");
+	if( val ) {
+		if( JSON_NUMBER != node->type && strcmp( get_primitive( field ), "number") ) {
+			// Value is not numeric; enclose it in quotes
+			if ( !dbi_conn_quote_string( dbhandle, &val ) ) {
+				osrfLogError( OSRF_LOG_MARK, "%s: Error quoting key string [%s]", MODULENAME, val );
+				free( val );
+				return NULL;
+			}
+		}
+	} else {
+		// Compare to a null value
+		val = strdup( "NULL" );
+		if (strcmp( op, "=" ))
+			op = "IS NOT";
 		else
-			op = strdup("IS");
+			op = "IS";
+	}
 
-	} else if ( !strcmp( get_primitive( field ), "number") ) {
-		val = strdup(right);
-		op = strdup(orig_op);
+	growing_buffer* sql_buf = buffer_init(32);
+	buffer_fadd( sql_buf, "\"%s\".%s %s %s", class, osrfHashGet(field, "name"), op, val );
+	char* pred = buffer_release( sql_buf );
 
-	} else {
-		val = strdup(right);
-		if ( !dbi_conn_quote_string(dbhandle, &val) ) {
-			osrfLogError(OSRF_LOG_MARK, "%s: Error quoting key string [%s]", MODULENAME, val);
-			free(val);
-			return NULL;
-		}
-		op = strdup(orig_op);
-	}
-
-	growing_buffer* sql_buf = buffer_init(16);
-	buffer_fadd( sql_buf, "\"%s\".%s %s %s", class, left, op, val );
 	free(val);
-	free(op);
 
-	return buffer_release(sql_buf);
+	return pred;
 }
 
 static char* searchBETWEENPredicate (const char* class, osrfHash* field, jsonObject* node) {
@@ -4044,6 +4041,7 @@
 
 		const jsonObject* field_object = oilsFMGetObject( target, field_name );
 
+		int value_is_numeric = 0;    // boolean
 		char* value;
 		if (field_object && field_object->classname) {
 			value = oilsFMGetString(
@@ -4052,6 +4050,8 @@
             );
 		} else {
 			value = jsonObjectToSimpleString( field_object );
+			if( field_object && JSON_NUMBER == field_object->type )
+				value_is_numeric = 1;
 		}
 
 		osrfLogDebug( OSRF_LOG_MARK, "Updating %s object with %s = %s", osrfHashGet(meta, "fieldmapper"), field_name, value);
@@ -4063,7 +4063,7 @@
 				buffer_fadd( sql, " %s = NULL", field_name );
 			}
 			
-		} else if ( !strcmp( get_primitive( field ), "number") ) {
+		} else if ( value_is_numeric || !strcmp( get_primitive( field ), "number") ) {
 			if (first) first = 0;
 			else OSRF_BUFFER_ADD_CHAR(sql, ',');
 



More information about the open-ils-commits mailing list