[open-ils-commits] r18240 - trunk/Open-ILS/src/c-apps (scottmk)
svn at svn.open-ils.org
svn at svn.open-ils.org
Fri Oct 8 10:16:21 EDT 2010
Author: scottmk
Date: 2010-10-08 10:16:19 -0400 (Fri, 08 Oct 2010)
New Revision: 18240
Modified:
trunk/Open-ILS/src/c-apps/oils_sql.c
Log:
Tidied up buildSELECT() a bit:
1. Sprinkled the const qualifier here and there.
2. Moved some variable declarations to get them closer to the point of
first use, and to limit their scope.
3. Renamed some variables to better reflect their meaning.
4. Split a couple of variables into multiple variables, instead of using
them for multiple unrelated purposes.
5. Plugged a memory leak in the case of an error return.
6. Added comments, including a Doxygen-style comment at the top of the
function.
M Open-ILS/src/c-apps/oils_sql.c
Modified: trunk/Open-ILS/src/c-apps/oils_sql.c
===================================================================
--- trunk/Open-ILS/src/c-apps/oils_sql.c 2010-10-08 03:30:00 UTC (rev 18239)
+++ trunk/Open-ILS/src/c-apps/oils_sql.c 2010-10-08 14:16:19 UTC (rev 18240)
@@ -90,8 +90,9 @@
jsonObject*, const char*, osrfMethodContext* );
static char* searchPredicate ( const ClassInfo*, osrfHash*, jsonObject*, osrfMethodContext* );
static char* searchJOIN ( const jsonObject*, const ClassInfo* left_info );
-static char* searchWHERE ( const jsonObject*, const ClassInfo*, int, osrfMethodContext* );
-static char* buildSELECT ( jsonObject*, jsonObject*, osrfHash*, osrfMethodContext* );
+static char* searchWHERE ( const jsonObject* search_hash, const ClassInfo*, int, osrfMethodContext* );
+static char* buildSELECT( const jsonObject*, jsonObject* rest_of_query,
+ osrfHash* meta, osrfMethodContext* ctx );
char* buildQuery( osrfMethodContext* ctx, jsonObject* query, int flags );
char* SELECT ( osrfMethodContext*, jsonObject*, const jsonObject*, const jsonObject*,
@@ -4891,26 +4892,36 @@
} // end of SELECT()
-static char* buildSELECT ( jsonObject* search_hash, jsonObject* order_hash, osrfHash* meta, osrfMethodContext* ctx ) {
+/**
+ @brief Build a SELECT statement.
+ @param search_hash Pointer to a JSON_HASH or JSON_ARRAY encoding the WHERE clause.
+ @param rest_of_query Pointer to a JSON_HASH containing any other SQL clauses.
+ @param meta Pointer to the class metadata for the core class.
+ @param ctx Pointer to the method context.
+ @return Pointer to a character string containing the WHERE clause; or NULL upon error.
+ Within the rest_of_query hash, the meaningful keys are "join", "select", "no_i18n",
+ "order_by", "limit", and "offset".
+
+ The SELECT statements built here are distinct from those built for the json_query method.
+*/
+static char* buildSELECT ( const jsonObject* search_hash, jsonObject* rest_of_query,
+ osrfHash* meta, osrfMethodContext* ctx ) {
+
const char* locale = osrf_message_get_last_locale();
osrfHash* fields = osrfHashGet( meta, "fields" );
- char* core_class = osrfHashGet( meta, "classname" );
+ const char* core_class = osrfHashGet( meta, "classname" );
- const jsonObject* join_hash = jsonObjectGetKeyConst( order_hash, "join" );
+ const jsonObject* join_hash = jsonObjectGetKeyConst( rest_of_query, "join" );
- jsonObject* node = NULL;
- jsonObject* snode = NULL;
- jsonObject* onode = NULL;
- const jsonObject* _tmp = NULL;
jsonObject* selhash = NULL;
jsonObject* defaultselhash = NULL;
growing_buffer* sql_buf = buffer_init( 128 );
growing_buffer* select_buf = buffer_init( 128 );
- if( !(selhash = jsonObjectGetKey( order_hash, "select" )) ) {
+ if( !(selhash = jsonObjectGetKey( rest_of_query, "select" )) ) {
defaultselhash = jsonNewObjectType( JSON_HASH );
selhash = defaultselhash;
}
@@ -4932,20 +4943,24 @@
jsonObjectSetKey( selhash, core_class, field_list );
}
+ // Build a list of columns for the SELECT clause
int first = 1;
+ const jsonObject* snode = NULL;
jsonIterator* class_itr = jsonNewIterator( selhash );
- while( (snode = jsonIteratorNext( class_itr )) ) {
+ while( (snode = jsonIteratorNext( class_itr )) ) { // For each class
+ // If the class isn't in the IDL, ignore it
const char* cname = class_itr->key;
osrfHash* idlClass = osrfHashGet( oilsIDL(), cname );
if( !idlClass )
continue;
- if( strcmp(core_class,class_itr->key )) {
+ // If the class isn't the core class, and isn't in the JOIN clause, ignore it
+ if( strcmp( core_class, class_itr->key )) {
if( !join_hash )
continue;
- jsonObject* found = jsonObjectFindPath( join_hash, "//%s", class_itr->key );
+ jsonObject* found = jsonObjectFindPath( join_hash, "//%s", class_itr->key );
if( !found->size ) {
jsonObjectFree( found );
continue;
@@ -4954,6 +4969,7 @@
jsonObjectFree( found );
}
+ const jsonObject* node = NULL;
jsonIterator* select_itr = jsonNewIterator( snode );
while( (node = jsonIteratorNext( select_itr )) ) {
const char* item_str = jsonObjectGetString( node );
@@ -4971,7 +4987,7 @@
if( locale ) {
const char* i18n;
- const jsonObject* no_i18n_obj = jsonObjectGetKeyConst( order_hash, "no_i18n" );
+ const jsonObject* no_i18n_obj = jsonObjectGetKeyConst( rest_of_query, "no_i18n" );
if( obj_is_true( no_i18n_obj ) ) // Suppress internationalization?
i18n = NULL;
else
@@ -5019,9 +5035,13 @@
ctx->request,
"Unable to build query frame for core class"
);
+ buffer_free( sql_buf );
+ if( defaultselhash )
+ jsonObjectFree( defaultselhash );
return NULL;
}
+ // Add the JOIN clauses, if any
if( join_hash ) {
char* join_clause = searchJOIN( join_hash, &curr_query->core );
OSRF_BUFFER_ADD_CHAR( sql_buf, ' ' );
@@ -5030,10 +5050,11 @@
}
osrfLogDebug( OSRF_LOG_MARK, "%s pre-predicate SQL = %s",
- modulename, OSRF_BUFFER_C_STR( sql_buf ));
+ modulename, OSRF_BUFFER_C_STR( sql_buf ));
OSRF_BUFFER_ADD( sql_buf, " WHERE " );
+ // Add the conditions in the WHERE clause
char* pred = searchWHERE( search_hash, &curr_query->core, AND_OP_JOIN, ctx );
if( !pred ) {
osrfAppSessionStatus(
@@ -5053,34 +5074,46 @@
free( pred );
}
- if( order_hash ) {
- char* string = NULL;
- if( (_tmp = jsonObjectGetKeyConst( order_hash, "order_by" )) ){
+ // Add the ORDER BY and/or LIMIT clauses, if present
+ if( rest_of_query ) {
+ const jsonObject* order_by = NULL;
+ if( ( order_by = jsonObjectGetKeyConst( rest_of_query, "order_by" )) ){
growing_buffer* order_buf = buffer_init( 128 );
+ if( JSON_HASH != order_by->type )
+ osrfLogWarning( OSRF_LOG_MARK,
+ "\"order_by\" object in a query is not a JSON_HASH; no ORDER BY generated" );
+
+ // We expect order_by to be a JSON_HASH keyed on class names. Traverse it
+ // and build a list of ORDER BY expressions.
first = 1;
- jsonIterator* class_itr = jsonNewIterator( _tmp );
- while( (snode = jsonIteratorNext( class_itr )) ) {
+ jsonIterator* class_itr = jsonNewIterator( order_by );
+ while( (snode = jsonIteratorNext( class_itr )) ) { // For each class:
- if( !jsonObjectGetKeyConst( selhash,class_itr->key ))
- continue;
+ if( !jsonObjectGetKeyConst( selhash, class_itr->key ))
+ continue; // class not referenced by SELECT clause? Ignore it.
if( snode->type == JSON_HASH ) {
+ // If the data for the current class is a JSON_HASH, then it is
+ // keyed on field name.
+
+ const jsonObject* onode = NULL;
jsonIterator* order_itr = jsonNewIterator( snode );
- while( (onode = jsonIteratorNext( order_itr )) ) {
+ while( (onode = jsonIteratorNext( order_itr )) ) { // For each field
osrfHash* field_def = oilsIDLFindPath( "/%s/fields/%s",
- class_itr->key, order_itr->key );
+ class_itr->key, order_itr->key );
if( !field_def )
- continue;
+ continue; // Field not defined in IDL? Ignore it.
+ char* field_str = NULL;
char* direction = NULL;
if( onode->type == JSON_HASH ) {
if( jsonObjectGetKeyConst( onode, "transform" ) ) {
- string = searchFieldTransform( class_itr->key, field_def, onode );
- if( ! string ) {
+ field_str = searchFieldTransform( class_itr->key, field_def, onode);
+ if( ! field_str ) {
osrfAppSessionStatus(
ctx->session,
OSRF_STATUS_INTERNALSERVERERROR,
@@ -5102,17 +5135,17 @@
growing_buffer* field_buf = buffer_init( 16 );
buffer_fadd( field_buf, "\"%s\".%s",
class_itr->key, order_itr->key );
- string = buffer_release( field_buf );
+ field_str = buffer_release( field_buf );
}
- if( (_tmp = jsonObjectGetKeyConst( onode, "direction" )) ) {
- const char* dir = jsonObjectGetString( _tmp );
+ if( ( order_by = jsonObjectGetKeyConst( onode, "direction" )) ) {
+ const char* dir = jsonObjectGetString( order_by );
if(!strncasecmp( dir, "d", 1 )) {
direction = " DESC";
}
}
} else {
- string = strdup( order_itr->key );
+ field_str = strdup( order_itr->key );
const char* dir = jsonObjectGetString( onode );
if( !strncasecmp( dir, "d", 1 )) {
direction = " DESC";
@@ -5127,8 +5160,8 @@
buffer_add( order_buf, ", " );
}
- buffer_add( order_buf, string );
- free( string );
+ buffer_add( order_buf, field_str );
+ free( field_str );
if( direction ) {
buffer_add( order_buf, direction );
@@ -5138,27 +5171,30 @@
jsonIteratorFree( order_itr );
} else {
+ // The data for the current class is not a JSON_HASH, so we expect
+ // it to be a JSON_STRING with a single field name.
const char* str = jsonObjectGetString( snode );
buffer_add( order_buf, str );
break;
}
- }
+ } // end while; looping over order_by expressions
jsonIteratorFree( class_itr );
- string = buffer_release( order_buf );
+ char* order_by_list = buffer_release( order_buf );
- if( *string ) {
+ if( *order_by_list ) {
OSRF_BUFFER_ADD( sql_buf, " ORDER BY " );
- OSRF_BUFFER_ADD( sql_buf, string );
+ OSRF_BUFFER_ADD( sql_buf, order_by_list );
}
- free( string );
+ free( order_by_list );
}
- if( (_tmp = jsonObjectGetKeyConst( order_hash, "limit" )) ) {
- const char* str = jsonObjectGetString( _tmp );
+ const jsonObject* limit = jsonObjectGetKeyConst( rest_of_query, "limit" );
+ if( limit ) {
+ const char* str = jsonObjectGetString( limit );
buffer_fadd(
sql_buf,
" LIMIT %d",
@@ -5166,9 +5202,9 @@
);
}
- _tmp = jsonObjectGetKeyConst( order_hash, "offset" );
- if( _tmp ) {
- const char* str = jsonObjectGetString( _tmp );
+ const jsonObject* offset = jsonObjectGetKeyConst( rest_of_query, "offset" );
+ if( offset ) {
+ const char* str = jsonObjectGetString( offset );
buffer_fadd(
sql_buf,
" OFFSET %d",
More information about the open-ils-commits
mailing list