From: scottmk Date: Sat, 4 Jul 2009 19:01:35 +0000 (+0000) Subject: Change the interface to doFieldMapperSearch(), and reap some performance X-Git-Url: https://old-git.evergreen-ils.org/?a=commitdiff_plain;h=95ca13d5cb9fe7c62945455dafbdf0cbfe6dfd53;p=evergreen%2Fbjwebb.git Change the interface to doFieldMapperSearch(), and reap some performance gains that this change makes possible. Instead of passing the query as two jsonObjects wrapped in a JSON_ARRAY, pass them as separate parameters. This change makes it possible to avoid the overhead of constructing a single package to bundle them both. The resulting code is also, to my eyes, easier to read. git-svn-id: svn://svn.open-ils.org/ILS/trunk@13499 dcc99617-32d9-48b4-a31d-7c20da2025e4 --- diff --git a/Open-ILS/src/c-apps/oils_cstore.c b/Open-ILS/src/c-apps/oils_cstore.c index d78a7137a..9b85a805c 100644 --- a/Open-ILS/src/c-apps/oils_cstore.c +++ b/Open-ILS/src/c-apps/oils_cstore.c @@ -50,8 +50,8 @@ static jsonObject* doCreate ( osrfMethodContext*, int* ); static jsonObject* doRetrieve ( osrfMethodContext*, int* ); static jsonObject* doUpdate ( osrfMethodContext*, int* ); static jsonObject* doDelete ( osrfMethodContext*, int* ); -static jsonObject* doFieldmapperSearch ( osrfMethodContext*, osrfHash*, - const jsonObject*, int* ); +static jsonObject* doFieldmapperSearch ( osrfMethodContext* ctx, osrfHash* meta, + jsonObject* where_hash, jsonObject* query_hash, int* err ); static jsonObject* oilsMakeFieldmapperFromResult( dbi_result, osrfHash* ); static jsonObject* oilsMakeJSONFromResult( dbi_result ); @@ -766,17 +766,19 @@ int dispatchCRUDMethod ( osrfMethodContext* ctx ) { } else if (!strcmp(methodtype, "search")) { - jsonObject* _p = jsonObjectClone( ctx->params ); + jsonObject* where_clause; + jsonObject* rest_of_query; + #ifdef PCRUD - jsonObjectFree(_p); - _p = jsonParseString("[]"); - jsonObjectPush(_p, jsonObjectClone(jsonObjectGetIndex(ctx->params, 1))); - jsonObjectPush(_p, jsonObjectClone(jsonObjectGetIndex(ctx->params, 2))); + where_clause = jsonObjectGetIndex( ctx->params, 1 ); + rest_of_query = jsonObjectGetIndex( ctx->params, 2 ); +#else + where_clause = jsonObjectGetIndex( ctx->params, 0 ); + rest_of_query = jsonObjectGetIndex( ctx->params, 1 ); #endif - obj = doFieldmapperSearch(ctx, class_obj, _p, &err); + obj = doFieldmapperSearch( ctx, class_obj, where_clause, rest_of_query, &err ); - jsonObjectFree(_p); if(err) return err; jsonObject* cur; @@ -792,27 +794,33 @@ int dispatchCRUDMethod ( osrfMethodContext* ctx ) { } else if (!strcmp(methodtype, "id_list")) { - jsonObject* _p = jsonObjectClone( ctx->params ); + jsonObject* where_clause; + jsonObject* rest_of_query; + + // We use the where clause without change. But we need + // to massage the rest of the query, so we work with a copy + // of it instead of modifying the original. #ifdef PCRUD - jsonObjectFree(_p); - _p = jsonParseString("[]"); - jsonObjectPush(_p, jsonObjectClone(jsonObjectGetIndex(ctx->params, 1))); - jsonObjectPush(_p, jsonObjectClone(jsonObjectGetIndex(ctx->params, 2))); + where_clause = jsonObjectGetIndex( ctx->params, 1 ); + rest_of_query = jsonObjectClone( jsonObjectGetIndex( ctx->params, 2 ) ); +#else + where_clause = jsonObjectGetIndex( ctx->params, 0 ); + rest_of_query = jsonObjectClone( jsonObjectGetIndex( ctx->params, 1 ) ); #endif - if (jsonObjectGetIndex( _p, 1 )) { - jsonObjectRemoveKey( jsonObjectGetIndex( _p, 1 ), "select" ); - jsonObjectRemoveKey( jsonObjectGetIndex( _p, 1 ), "no_i18n" ); - jsonObjectRemoveKey( jsonObjectGetIndex( _p, 1 ), "flesh" ); - jsonObjectRemoveKey( jsonObjectGetIndex( _p, 1 ), "flesh_columns" ); - } else { - jsonObjectSetIndex( _p, 1, jsonNewObjectType(JSON_HASH) ); - } + if ( rest_of_query ) { + jsonObjectRemoveKey( rest_of_query, "select" ); + jsonObjectRemoveKey( rest_of_query, "no_i18n" ); + jsonObjectRemoveKey( rest_of_query, "flesh" ); + jsonObjectRemoveKey( rest_of_query, "flesh_columns" ); + } else { + rest_of_query = jsonNewObjectType( JSON_HASH ); + } - jsonObjectSetKey( jsonObjectGetIndex( _p, 1 ), "no_i18n", jsonNewBoolObject( 1 ) ); + jsonObjectSetKey( rest_of_query, "no_i18n", jsonNewBoolObject( 1 ) ); - jsonObjectSetKey( - jsonObjectGetIndex( _p, 1 ), + jsonObjectSetKey( + rest_of_query, "select", jsonParseStringFmt( "{ \"%s\":[\"%s\"] }", @@ -821,10 +829,10 @@ int dispatchCRUDMethod ( osrfMethodContext* ctx ) { ) ); - obj = doFieldmapperSearch(ctx, class_obj, _p, &err); + obj = doFieldmapperSearch( ctx, class_obj, where_clause, rest_of_query, &err ); - jsonObjectFree(_p); - if(err) return err; + jsonObjectFree( rest_of_query ); + if(err) return err; jsonObject* cur; jsonIterator* itr = jsonNewIterator( obj ); @@ -969,8 +977,9 @@ static int verifyObjectPCRUD ( osrfMethodContext* ctx, const jsonObject* obj ) osrfLogDebug( OSRF_LOG_MARK, "global-level permissions required, fetching top of the org tree" ); // check for perm at top of org tree - jsonObject* _tmp_params = jsonParseString("[{\"parent_ou\":null}]"); - jsonObject* _list = doFieldmapperSearch(ctx, osrfHashGet( oilsIDL(), "aou" ), _tmp_params, &err); + jsonObject* _tmp_params = jsonParseString("{\"parent_ou\":null}"); + jsonObject* _list = doFieldmapperSearch(ctx, osrfHashGet( oilsIDL(), "aou" ), + _tmp_params, NULL, &err); jsonObject* _tree_top = jsonObjectGetIndex(_list, 0); @@ -1012,16 +1021,12 @@ static int verifyObjectPCRUD ( osrfMethodContext* ctx, const jsonObject* obj ) } if (fetch) { - jsonObject* _tmp_params = jsonParseStringFmt("[{\"%s\":\"%s\"}]", pkey, pkey_value); - jsonObject* _list = doFieldmapperSearch( - ctx, - class, - _tmp_params, - &err - ); - - param = jsonObjectClone(jsonObjectGetIndex(_list, 0)); - + jsonObject* _tmp_params = jsonParseStringFmt("{\"%s\":\"%s\"}", pkey, pkey_value); + jsonObject* _list = doFieldmapperSearch( + ctx, class, _tmp_params, NULL, &err ); + + param = jsonObjectClone(jsonObjectGetIndex(_list, 0)); + jsonObjectFree(_tmp_params); jsonObjectFree(_list); } @@ -1092,17 +1097,13 @@ static int verifyObjectPCRUD ( osrfMethodContext* ctx, const jsonObject* obj ) char* foreign_pkey_value = oilsFMGetString(param, osrfHashGet(fcontext, "fkey")); jsonObject* _tmp_params = jsonParseStringFmt( - "[{\"%s\":\"%s\"}]", + "{\"%s\":\"%s\"}", foreign_pkey, foreign_pkey_value ); - - jsonObject* _list = doFieldmapperSearch( - ctx, - osrfHashGet( oilsIDL(), class_name ), - _tmp_params, - &err - ); + + jsonObject* _list = doFieldmapperSearch( + ctx, osrfHashGet( oilsIDL(), class_name ), _tmp_params, NULL, &err ); jsonObject* _fparam = jsonObjectClone(jsonObjectGetIndex(_list, 0)); jsonObjectFree(_tmp_params); @@ -1122,17 +1123,18 @@ static int verifyObjectPCRUD ( osrfMethodContext* ctx, const jsonObject* obj ) foreign_pkey = osrfHashGet( foreign_link_hash, "key" ); _tmp_params = jsonParseStringFmt( - "[{\"%s\":\"%s\"}]", + "{\"%s\":\"%s\"}", foreign_pkey, foreign_pkey_value ); - _list = doFieldmapperSearch( - ctx, - osrfHashGet( oilsIDL(), osrfHashGet( foreign_link_hash, "class" ) ), - _tmp_params, - &err - ); + _list = doFieldmapperSearch( + ctx, + osrfHashGet( oilsIDL(), osrfHashGet( foreign_link_hash, "class" ) ), + _tmp_params, + NULL, + &err + ); _fparam = jsonObjectClone(jsonObjectGetIndex(_list, 0)); jsonObjectFree(_tmp_params); @@ -1512,26 +1514,20 @@ static jsonObject* doCreate(osrfMethodContext* ctx, int* err ) { } else { - jsonObject* fake_params = jsonNewObjectType(JSON_ARRAY); - jsonObjectPush(fake_params, jsonNewObjectType(JSON_HASH)); + jsonObject* where_clause = jsonNewObjectType( JSON_HASH ); + jsonObjectSetKey( where_clause, pkey, jsonNewObject(id) ); - jsonObjectSetKey( - jsonObjectGetIndex(fake_params, 0), - pkey, - jsonNewObject(id) - ); + jsonObject* list = doFieldmapperSearch( ctx, meta, where_clause, NULL, err ); - jsonObject* list = doFieldmapperSearch( ctx,meta, fake_params, err); + jsonObjectFree( where_clause ); if(*err) { - jsonObjectFree( fake_params ); obj = jsonNULL; } else { obj = jsonObjectClone( jsonObjectGetIndex(list, 0) ); } jsonObjectFree( list ); - jsonObjectFree( fake_params ); } free(id); @@ -1567,29 +1563,24 @@ static jsonObject* doRetrieve(osrfMethodContext* ctx, int* err ) { id ); - jsonObject* fake_params = jsonNewObjectType(JSON_ARRAY); - jsonObjectPush(fake_params, jsonNewObjectType(JSON_HASH)); - - jsonObjectSetKey( - jsonObjectGetIndex(fake_params, 0), - osrfHashGet(meta, "primarykey"), - jsonObjectClone(jsonObjectGetIndex(ctx->params, id_pos)) + jsonObject* key_param = jsonNewObjectType( JSON_HASH ); + jsonObjectSetKey( + key_param, + osrfHashGet( meta, "primarykey" ), + jsonObjectClone( jsonObjectGetIndex(ctx->params, id_pos) ) ); - - if (order_hash) jsonObjectPush(fake_params, jsonObjectClone(order_hash) ); - - jsonObject* list = doFieldmapperSearch( ctx,meta, fake_params, err); + jsonObject* list = doFieldmapperSearch( ctx, meta, key_param, order_hash, err ); if(*err) { - jsonObjectFree( fake_params ); + jsonObjectFree( key_param ); return jsonNULL; } jsonObject* obj = jsonObjectClone( jsonObjectGetIndex(list, 0) ); jsonObjectFree( list ); - jsonObjectFree( fake_params ); + jsonObjectFree( key_param ); #ifdef PCRUD if(!verifyObjectPCRUD(ctx, obj)) { @@ -4201,7 +4192,7 @@ int doJSONSearch ( osrfMethodContext* ctx ) { } static jsonObject* doFieldmapperSearch ( osrfMethodContext* ctx, osrfHash* meta, - const jsonObject* params, int* err ) { + jsonObject* where_hash, jsonObject* query_hash, int* err ) { // XXX for now... dbhandle = writehandle; @@ -4213,16 +4204,14 @@ static jsonObject* doFieldmapperSearch ( osrfMethodContext* ctx, osrfHash* meta, const jsonObject* _tmp; jsonObject* obj; - jsonObject* search_hash = jsonObjectGetIndex(params, 0); - jsonObject* order_hash = jsonObjectGetIndex(params, 1); - char* sql = buildSELECT( search_hash, order_hash, meta, ctx ); + char* sql = buildSELECT( where_hash, query_hash, meta, ctx ); if (!sql) { osrfLogDebug(OSRF_LOG_MARK, "Problem building query, returning NULL"); *err = -1; return NULL; } - + osrfLogDebug(OSRF_LOG_MARK, "%s SQL = %s", MODULENAME, sql); dbi_result result = dbi_conn_query(dbhandle, sql); @@ -4271,14 +4260,14 @@ static jsonObject* doFieldmapperSearch ( osrfMethodContext* ctx, osrfHash* meta, dbi_result_free(result); free(sql); - if (res_list->size && order_hash) { - _tmp = jsonObjectGetKeyConst( order_hash, "flesh" ); + if (res_list->size && query_hash) { + _tmp = jsonObjectGetKeyConst( query_hash, "flesh" ); if (_tmp) { int x = (int)jsonObjectGetNumber(_tmp); if (x == -1 || x > max_flesh_depth) x = max_flesh_depth; const jsonObject* temp_blob; - if ((temp_blob = jsonObjectGetKeyConst( order_hash, "flesh_fields" )) && x > 0) { + if ((temp_blob = jsonObjectGetKeyConst( query_hash, "flesh_fields" )) && x > 0) { jsonObject* flesh_blob = jsonObjectClone( temp_blob ); const jsonObject* flesh_fields = jsonObjectGetKeyConst( flesh_blob, core_class ); @@ -4357,12 +4346,6 @@ static jsonObject* doFieldmapperSearch ( osrfMethodContext* ctx, osrfHash* meta, osrfHashGet(kid_link, "reltype") ); - jsonObject* fake_params = jsonNewObjectType(JSON_ARRAY); - jsonObjectPush(fake_params, jsonNewObjectType(JSON_HASH)); // search hash - jsonObjectPush(fake_params, jsonNewObjectType(JSON_HASH)); // order/flesh hash - - osrfLogDebug(OSRF_LOG_MARK, "Creating dummy params object..."); - const char* search_key = jsonObjectGetString( jsonObjectGetIndex( cur, @@ -4375,41 +4358,44 @@ static jsonObject* doFieldmapperSearch ( osrfMethodContext* ctx, osrfHash* meta, continue; } + osrfLogDebug(OSRF_LOG_MARK, "Creating param objects..."); + + // construct WHERE clause + jsonObject* where_clause = jsonNewObjectType(JSON_HASH); jsonObjectSetKey( - jsonObjectGetIndex(fake_params, 0), + where_clause, osrfHashGet(kid_link, "key"), jsonNewObject( search_key ) ); - jsonObjectSetKey( - jsonObjectGetIndex(fake_params, 1), - "flesh", + // construct the rest of the query + jsonObject* rest_of_query = jsonNewObjectType(JSON_HASH); + jsonObjectSetKey( rest_of_query, "flesh", jsonNewNumberObject( (double)(x - 1 + link_map->size) ) ); if (flesh_blob) - jsonObjectSetKey( jsonObjectGetIndex(fake_params, 1), "flesh_fields", jsonObjectClone(flesh_blob) ); + jsonObjectSetKey( rest_of_query, "flesh_fields", jsonObjectClone(flesh_blob) ); - if (jsonObjectGetKeyConst(order_hash, "order_by")) { - jsonObjectSetKey( - jsonObjectGetIndex(fake_params, 1), - "order_by", - jsonObjectClone(jsonObjectGetKeyConst(order_hash, "order_by")) + if (jsonObjectGetKeyConst(query_hash, "order_by")) { + jsonObjectSetKey( rest_of_query, "order_by", + jsonObjectClone(jsonObjectGetKeyConst(query_hash, "order_by")) ); } - if (jsonObjectGetKeyConst(order_hash, "select")) { - jsonObjectSetKey( - jsonObjectGetIndex(fake_params, 1), - "select", - jsonObjectClone(jsonObjectGetKeyConst(order_hash, "select")) + if (jsonObjectGetKeyConst(query_hash, "select")) { + jsonObjectSetKey( rest_of_query, "select", + jsonObjectClone(jsonObjectGetKeyConst(query_hash, "select")) ); } - jsonObject* kids = doFieldmapperSearch(ctx, kid_idl, fake_params, err); + jsonObject* kids = doFieldmapperSearch( ctx, kid_idl, + where_clause, rest_of_query, err); + + jsonObjectFree( where_clause ); + jsonObjectFree( rest_of_query ); if(*err) { - jsonObjectFree( fake_params ); osrfStringArrayFree(link_fields); jsonIteratorFree(itr); jsonObjectFree(res_list); @@ -4478,7 +4464,6 @@ static jsonObject* doFieldmapperSearch ( osrfMethodContext* ctx, osrfHash* meta, } jsonObjectFree( kids ); - jsonObjectFree( fake_params ); osrfLogDebug(OSRF_LOG_MARK, "Fleshing of %s complete", osrfHashGet(kid_link, "field")); osrfLogDebug(OSRF_LOG_MARK, "%s", jsonObjectToJSON(cur));