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 <mrylander@gmail.com>

diff --git a/Open-ILS/src/c-apps/oils_sql.c b/Open-ILS/src/c-apps/oils_sql.c
index c4fc6aa..d8afb6d 100644 (file)
--- 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