From: Mike Rylander Date: Tue, 22 Oct 2019 14:09:47 +0000 (-0400) Subject: LP#1847805: Avoid using cursors for .retrieve calls X-Git-Url: https://old-git.evergreen-ils.org/?a=commitdiff_plain;h=a455f7c3e883fd8f46c12177e7732809b3267fae;p=working%2FEvergreen.git LP#1847805: Avoid using cursors for .retrieve calls There's no need to spend the overhead of cursors on pcrud requests that we know will only return one record, so we skip it in that case. Signed-off-by: Mike Rylander --- diff --git a/Open-ILS/src/c-apps/oils_sql.c b/Open-ILS/src/c-apps/oils_sql.c index c4fc6aad8c..d8afb6dd38 100644 --- a/Open-ILS/src/c-apps/oils_sql.c +++ b/Open-ILS/src/c-apps/oils_sql.c @@ -93,7 +93,7 @@ static char* searchPredicate ( const ClassInfo*, osrfHash*, jsonObject*, osrfMet static char* searchJOIN ( const jsonObject*, const ClassInfo* left_info ); 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** cursor_name ); + osrfHash* meta, osrfMethodContext* ctx, char* cursor_name ); static char* buildOrderByFromArray( osrfMethodContext* ctx, const jsonObject* order_array ); char* buildQuery( osrfMethodContext* ctx, jsonObject* query, int flags ); @@ -5492,7 +5492,7 @@ static char* buildOrderByFromArray( osrfMethodContext* ctx, const jsonObject* or 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, char** cursor_name ) { + osrfHash* meta, osrfMethodContext* ctx, char* cursor_name ) { const char* locale = osrf_message_get_last_locale(); @@ -5607,9 +5607,8 @@ static char* buildSELECT ( const jsonObject* search_hash, jsonObject* rest_of_qu if( !table ) table = strdup( "(null)" ); - *cursor_name = random_cursor_name(); - - buffer_fadd( sql_buf, "DECLARE \"%s\" NO SCROLL CURSOR WITH HOLD FOR SELECT %s FROM %s AS \"%s\"", *cursor_name, col_list, table, core_class ); + if (cursor_name) buffer_fadd( sql_buf, "DECLARE \"%s\" NO SCROLL CURSOR WITH HOLD FOR ", cursor_name ); + buffer_fadd( sql_buf, "SELECT %s FROM %s AS \"%s\"", col_list, table, core_class ); free( col_list ); free( table ); @@ -5820,6 +5819,25 @@ static char* buildSELECT ( const jsonObject* search_hash, jsonObject* rest_of_qu } free( order_by_list ); + + } + + if (!cursor_name) { + const jsonObject* limit = jsonObjectGetKeyConst( rest_of_query, "limit" ); + if( limit ) { + const char* str = jsonObjectGetString( limit ); + if (str) { + buffer_fadd( sql_buf, " LIMIT %d", atoi(str) ); + } + } + + const jsonObject* offset = jsonObjectGetKeyConst( rest_of_query, "offset" ); + if( offset ) { + const char* str = jsonObjectGetString( offset ); + if (str) { + buffer_fadd( sql_buf, " OFFSET %d", atoi(str) ); + } + } } } @@ -5945,9 +5963,11 @@ static jsonObject* doFieldmapperSearch( osrfMethodContext* ctx, osrfHash* class_ int offset = 0; int limit = 0; int cursor_page_size = 5; - char* cursor_name; + char* cursor_name = NULL; + + if (*methodtype != 'r') cursor_name = random_cursor_name(); /* Use a cursor for all but .retrieve */ - char* sql = buildSELECT( where_hash, query_hash, class_meta, ctx, &cursor_name ); + char* sql = buildSELECT( where_hash, query_hash, class_meta, ctx, cursor_name ); if( !sql ) { osrfLogDebug( OSRF_LOG_MARK, "Problem building query, returning NULL" ); *err = -1; @@ -5990,25 +6010,27 @@ static jsonObject* doFieldmapperSearch( osrfMethodContext* ctx, osrfHash* class_ } - const jsonObject* limit_o = jsonObjectGetKeyConst( query_hash, "limit" ); - if( limit_o ) { - limit = (int) jsonObjectGetNumber( limit_o ); - } else { - limit = -1; - } + if (cursor_name) { + const jsonObject* limit_o = jsonObjectGetKeyConst( query_hash, "limit" ); + if( limit_o ) { + limit = (int) jsonObjectGetNumber( limit_o ); + } else { + limit = -1; + } - const jsonObject* offset_o = jsonObjectGetKeyConst( query_hash, "offset" ); - if( offset_o ) { - offset = (int) jsonObjectGetNumber( offset_o ); - } else { - offset = -1; - } + const jsonObject* offset_o = jsonObjectGetKeyConst( query_hash, "offset" ); + if( offset_o ) { + offset = (int) jsonObjectGetNumber( offset_o ); + } else { + offset = -1; + } - // set cursor_page_size to the largest of the three - if (cursor_page_size < offset) cursor_page_size = offset; - if (cursor_page_size < limit) cursor_page_size = limit; + // set cursor_page_size to the largest of the three + if (cursor_page_size < offset) cursor_page_size = offset; + if (cursor_page_size < limit) cursor_page_size = limit; + } - // Declares a cursor + // Declares a cursor or runs a retrieve-select dbi_result result = dbi_conn_query( dbhandle, sql ); if( NULL == result ) { @@ -6058,34 +6080,38 @@ static jsonObject* doFieldmapperSearch( osrfMethodContext* ctx, osrfHash* class_ rs_size = (int *) safe_malloc( sizeof(int) ); // will be freed by sessionDataFree() unsigned long long result_count = cursor_page_size; + if (!cursor_name) { + result_count = dbi_result_get_numrows( result ); + } *rs_size = (int) result_count * (flesh_depth + 1); // yes, we could lose some bits, but come on osrfHashSet( (osrfHash *) ctx->session->userData, rs_size, "rs_size_req_%d", ctx->request ); } + if (cursor_name) { + dbi_result_free( result ); // toss the declare result + result = dbi_conn_queryf( dbhandle, "FETCH %d FROM \"%s\";", cursor_page_size, cursor_name ); + + if( NULL == result ) { + const char* msg; + int errnum = dbi_conn_error( dbhandle, &msg ); + osrfLogError(OSRF_LOG_MARK, "%s: Error retrieving %s with query [%s]: %d %s", + modulename, osrfHashGet( class_meta, "fieldmapper" ), sql, errnum, + msg ? msg : "(No description available)" ); + if( !oilsIsDBConnected( dbhandle )) + osrfAppSessionPanic( ctx->session ); + osrfAppSessionStatus( + ctx->session, + OSRF_STATUS_INTERNALSERVERERROR, + "osrfMethodException", + ctx->request, + "Severe query error -- see error log for more details" + ); + *err = -1; + free( sql ); + return NULL; - dbi_result_free( result ); // toss the declare result - result = dbi_conn_queryf( dbhandle, "FETCH %d FROM \"%s\";", cursor_page_size, cursor_name ); - - if( NULL == result ) { - const char* msg; - int errnum = dbi_conn_error( dbhandle, &msg ); - osrfLogError(OSRF_LOG_MARK, "%s: Error retrieving %s with query [%s]: %d %s", - modulename, osrfHashGet( class_meta, "fieldmapper" ), sql, errnum, - msg ? msg : "(No description available)" ); - if( !oilsIsDBConnected( dbhandle )) - osrfAppSessionPanic( ctx->session ); - osrfAppSessionStatus( - ctx->session, - OSRF_STATUS_INTERNALSERVERERROR, - "osrfMethodException", - ctx->request, - "Severe query error -- see error log for more details" - ); - *err = -1; - free( sql ); - return NULL; - - } + } + } if( dbi_result_first_row( result )) { @@ -6094,9 +6120,9 @@ static jsonObject* doFieldmapperSearch( osrfMethodContext* ctx, osrfHash* class_ // eliminate the duplicates. osrfLogDebug( OSRF_LOG_MARK, "Query returned at least one row" ); osrfHash* dedup = osrfNewHash(); - int cursor_page_remaining = cursor_page_size; + int cursor_page_remaining = cursor_page_size; do { - cursor_page_remaining--; + cursor_page_remaining--; row_obj = oilsMakeFieldmapperFromResult( result, class_meta ); char* pkey_val = oilsFMGetString( row_obj, pkey ); if( osrfHashGet( dedup, pkey_val ) ) { @@ -6105,16 +6131,21 @@ static jsonObject* doFieldmapperSearch( osrfMethodContext* ctx, osrfHash* class_ } else { if( !enforce_pcrud || !need_to_verify || verifyObjectPCRUD( ctx, class_meta, row_obj, 0 /* means check user data for rs_size */ )) { - if (offset > -1 && offset) { - offset--; - } else { - if (limit > -1 && limit) limit--; - osrfHashSet( dedup, pkey_val, pkey_val ); - jsonObjectPush( res_list, row_obj ); - } + if (cursor_name) { + if (offset > -1 && offset) { + offset--; + } else { + if (limit > -1 && limit) limit--; + osrfHashSet( dedup, pkey_val, pkey_val ); + jsonObjectPush( res_list, row_obj ); + } + } else { + osrfHashSet( dedup, pkey_val, pkey_val ); + jsonObjectPush( res_list, row_obj ); + } } } - if (!cursor_page_remaining) { // fetch next page + if (cursor_name && !cursor_page_remaining) { // fetch next page cursor_page_remaining = cursor_page_size; dbi_result_free( result ); result = dbi_conn_queryf( dbhandle, "FETCH %d FROM \"%s\";", cursor_page_size, cursor_name ); @@ -6140,9 +6171,11 @@ static jsonObject* doFieldmapperSearch( osrfMethodContext* ctx, osrfHash* class_ } } } while( - limit && ( // limit is -1 (none) or not yet decremented to 0 - (cursor_page_remaining == cursor_page_size && dbi_result_first_row(result)) || // on a new page, and there's a row - (cursor_page_remaining != cursor_page_size && dbi_result_next_row(result)) // on the same page, and there's a row + (!cursor_name && dbi_result_next_row(result)) || ( + cursor_name && limit && ( // limit is -1 (none) or not yet decremented to 0 + (cursor_page_remaining == cursor_page_size && dbi_result_first_row(result)) || // on a new page, and there's a row + (cursor_page_remaining != cursor_page_size && dbi_result_next_row(result)) // on the same page, and there's a row + ) ) ); osrfHashFree( dedup ); @@ -6154,8 +6187,11 @@ static jsonObject* doFieldmapperSearch( osrfMethodContext* ctx, osrfHash* class_ /* clean up the query */ dbi_result_free( result ); - result = dbi_conn_queryf( dbhandle, "CLOSE \"%s\";", cursor_name ); - dbi_result_free( result ); + if (cursor_name) { + result = dbi_conn_queryf( dbhandle, "CLOSE \"%s\";", cursor_name ); + dbi_result_free( result ); + } + free( sql ); // If we're asked to flesh, and there's anything to flesh, then flesh it