From: scottmk Date: Fri, 8 Oct 2010 14:16:19 +0000 (+0000) Subject: Tidied up buildSELECT() a bit: X-Git-Url: https://old-git.evergreen-ils.org/?a=commitdiff_plain;h=c7ca54667d02e9ed6f6ae1e7384d00142b63fe32;p=contrib%2FConifer.git 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 git-svn-id: svn://svn.open-ils.org/ILS/trunk@18240 dcc99617-32d9-48b4-a31d-7c20da2025e4 --- diff --git a/Open-ILS/src/c-apps/oils_sql.c b/Open-ILS/src/c-apps/oils_sql.c index 7a5cea87ca..b9f524e394 100644 --- a/Open-ILS/src/c-apps/oils_sql.c +++ b/Open-ILS/src/c-apps/oils_sql.c @@ -90,8 +90,9 @@ static char* searchINPredicate ( const char*, osrfHash*, 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 @@ char* SELECT ( } // 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 @@ static char* buildSELECT ( jsonObject* search_hash, jsonObject* order_hash, osrf 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 @@ static char* buildSELECT ( jsonObject* search_hash, jsonObject* order_hash, osrf 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 @@ static char* buildSELECT ( jsonObject* search_hash, jsonObject* order_hash, osrf 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 @@ static char* buildSELECT ( jsonObject* search_hash, jsonObject* order_hash, osrf 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 @@ static char* buildSELECT ( jsonObject* search_hash, jsonObject* order_hash, osrf } 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 @@ static char* buildSELECT ( jsonObject* search_hash, jsonObject* order_hash, osrf 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 @@ static char* buildSELECT ( jsonObject* search_hash, jsonObject* order_hash, osrf 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 @@ static char* buildSELECT ( jsonObject* search_hash, jsonObject* order_hash, osrf 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 @@ static char* buildSELECT ( jsonObject* search_hash, jsonObject* order_hash, osrf 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 @@ static char* buildSELECT ( jsonObject* search_hash, jsonObject* order_hash, osrf ); } - _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",