[open-ils-commits] r16450 - in trunk/Open-ILS: include/openils src/c-apps (scottmk)

svn at svn.open-ils.org svn at svn.open-ils.org
Wed May 19 08:46:19 EDT 2010


Author: scottmk
Date: 2010-05-19 08:46:18 -0400 (Wed, 19 May 2010)
New Revision: 16450

Modified:
   trunk/Open-ILS/include/openils/oils_buildq.h
   trunk/Open-ILS/src/c-apps/buildSQL.c
   trunk/Open-ILS/src/c-apps/oils_buildq.c
   trunk/Open-ILS/src/c-apps/oils_qstore.c
   trunk/Open-ILS/src/c-apps/oils_storedq.c
Log:
Add partial support for bind variables: load them from the
database, and use default values if available.  There is no
mechanism yet to override the defaults.



Modified: trunk/Open-ILS/include/openils/oils_buildq.h
===================================================================
--- trunk/Open-ILS/include/openils/oils_buildq.h	2010-05-18 20:12:38 UTC (rev 16449)
+++ trunk/Open-ILS/include/openils/oils_buildq.h	2010-05-19 12:46:18 UTC (rev 16450)
@@ -21,6 +21,9 @@
 struct SelectItem_;
 typedef struct SelectItem_ SelectItem;
 
+struct BindVar_;
+typedef struct BindVar_ BindVar;
+
 struct Expression_;
 typedef struct Expression_ Expression;
 
@@ -50,10 +53,15 @@
 	int error;                    /**< Boolean; true if an error has occurred */
 	osrfStringArray* error_msgs;  /**< Descriptions of errors, if any */
 	growing_buffer* sql;          /**< To hold the constructed query */
+	osrfHash* bindvar_list;       /**< List of bind variables used by this query, each with
+	                                   a pointer to the corresponding BindVar. */
 	IdNode* query_stack;          /**< For avoiding infinite recursion of nested queries */
 	IdNode* expr_stack;           /**< For avoiding infinite recursion of nested expressions */
 	IdNode* from_stack;           /**< For avoiding infinite recursion of from clauses */
 	int indent;                   /**< For prettifying SQL output: level of indentation */
+	int defaults_usable;          /**< Boolean; if true, we can use unconfirmed default
+	                                   values for bind variables */
+	int values_required;          /**< Boolean: if true, we need values for a bind variables */
 };
 
 typedef enum {
@@ -119,7 +127,25 @@
 };
 
 typedef enum {
+	BIND_STR,
+	BIND_NUM,
+	BIND_STR_LIST,
+	BIND_NUM_LIST
+} BindVarType;
+
+struct BindVar_ {
+	BindVar*    next;
+	char*       name;
+	char*       label;
+	BindVarType type;
+	char*       description;
+	jsonObject* default_value;
+	jsonObject* actual_value;
+};
+
+typedef enum {
 	EXP_BETWEEN,
+	EXP_BIND,
 	EXP_BOOL,
 	EXP_CASE,
 	EXP_CAST,
@@ -153,6 +179,7 @@
 	StoredQ*    subquery;
 	int         cast_type_id;
 	int         negate;             // Boolean
+	BindVar*    bind;
 };
 
 struct QSeq_ {

Modified: trunk/Open-ILS/src/c-apps/buildSQL.c
===================================================================
--- trunk/Open-ILS/src/c-apps/buildSQL.c	2010-05-18 20:12:38 UTC (rev 16449)
+++ trunk/Open-ILS/src/c-apps/buildSQL.c	2010-05-19 12:46:18 UTC (rev 16450)
@@ -9,6 +9,7 @@
 #include <dbi/dbi.h>
 #include "opensrf/utils.h"
 #include "opensrf/string_array.h"
+#include "opensrf/osrf_hash.h"
 #include "opensrf/osrf_application.h"
 #include "openils/oils_idl.h"
 #include "openils/oils_sql.h"
@@ -22,6 +23,8 @@
 static void buildSelectList( BuildSQLState* state, SelectItem* item );
 static void buildOrderBy( BuildSQLState* state, OrderItem* ord_list );
 static void buildExpression( BuildSQLState* state, Expression* expr );
+static void buildBindVar( BuildSQLState* state, BindVar* bind );
+static void buildScalar( BuildSQLState* state, int numeric, const jsonObject* obj );
 
 static void add_newline( BuildSQLState* state );
 static inline void incr_indent( BuildSQLState* state );
@@ -171,9 +174,9 @@
 		decr_indent( state );
 	}
 
-	// To do: build GROUP BY clause, if there is one
+    // To do: build GROUP BY clause, if there is one
 
-	// Build HAVING clause, if there is one
+    // Build HAVING clause, if there is one
 	if( query->having_clause ) {
 		add_newline( state );
 		buffer_add( state->sql, "HAVING" );
@@ -187,7 +190,7 @@
 		}
 		decr_indent( state );
 	}
-
+	
 	// Build ORDER BY clause, if there is one
 	if( query->order_by_list ) {
 		buildOrderBy( state, query->order_by_list );
@@ -453,6 +456,14 @@
 			sqlAddMsg( state, "BETWEEN expressions not yet supported" );
 			state->error = 1;
 			break;
+		case EXP_BIND :
+			if( !expr->bind ) {     // Sanity check
+				osrfLogError( OSRF_LOG_MARK, sqlAddMsg( state,
+					"Internal error: no variable for bind variable expression" ));
+				state->error = 1;
+			} else
+				buildBindVar( state, expr->bind );
+			break;
 		case EXP_BOOL :
 			if( expr->negate )
 				buffer_add( state->sql, "NOT " );
@@ -471,6 +482,9 @@
 			state->error = 1;
 			break;
 		case EXP_CAST :                   // Type cast
+			if( expr->negate )
+				buffer_add( state->sql, "NOT " );
+
 			sqlAddMsg( state, "Cast expressions not yet supported" );
 			state->error = 1;
 			break;
@@ -509,6 +523,9 @@
 			}
 			break;
 		case EXP_FIELD :
+			if( expr->negate )
+				buffer_add( state->sql, "NOT " );
+
 			sqlAddMsg( state, "Field expressions not yet supported" );
 			state->error = 1;
 			break;
@@ -589,6 +606,7 @@
 					"Internal error: No string value in string expression # %d", expr->id ));
 					state->error = 1;
 			} else {
+				// To do: escape special characters in the string
 				buffer_add_char( state->sql, '\'' );
 				buffer_add( state->sql, expr->literal );
 				buffer_add_char( state->sql, '\'' );
@@ -617,6 +635,141 @@
 		buffer_add_char( state->sql, ')' );
 }
 
+/**
+	@brief Add the value of a bind variable to an SQL statement.
+	@param state Pointer to the query-building context.
+	@param bind Pointer to the bind variable whose value is to be added to the SQL.
+
+	The value may be a null, a scalar, or an array of nulls and/or scalars, depending on
+	the type of the bind variable.
+*/
+static void buildBindVar( BuildSQLState* state, BindVar* bind ) {
+
+	// Decide where to get the value, if any
+	const jsonObject* value = NULL;
+	if( bind->actual_value )
+		value = bind->actual_value;
+	else if( bind->default_value ) {
+		if( state->defaults_usable )
+			value = bind->default_value;
+		else {
+			sqlAddMsg( state, "No confirmed value available for bind variable \"%s\"",
+				bind->name );
+			state->error = 1;
+			return;
+		}
+	} else if( state->values_required ) {
+		sqlAddMsg( state, "No value available for bind variable \"%s\"", bind->name );
+		state->error = 1;
+		return;
+	} else {
+		// No value available, and that's okay.  Emit the name of the bind variable.
+		buffer_add_char( state->sql, ':' );
+		buffer_add( state->sql, bind->name );
+		return;
+	}
+
+	// If we get to this point, we know that a value is available.  Carry on.
+
+	int numeric = 0;       // Boolean
+	if( BIND_NUM == bind->type || BIND_NUM_LIST == bind->type )
+		numeric = 1;
+
+	// Emit the value
+	switch( bind->type ) {
+		case BIND_STR :
+		case BIND_NUM :
+			buildScalar( state, numeric, value );
+			break;
+		case BIND_STR_LIST :
+		case BIND_NUM_LIST :
+			if( JSON_ARRAY == value->type ) {
+				// Iterate over array, emit each value
+				int first = 1;   // Boolean
+				unsigned long max = value->size;
+				unsigned long i = 0;
+				while( i < max ) {
+					if( first )
+						first = 0;
+					else
+						buffer_add( state->sql, ", " );
+
+					buildScalar( state, numeric, jsonObjectGetIndex( value, i ));
+					++i;
+				}
+			} else {
+				osrfLogError( OSRF_LOG_MARK, sqlAddMsg( state,
+					"Invalid value for bind variable; expected a list of values" ));
+				state->error = 1;
+			}
+			break;
+		default :
+			osrfLogError( OSRF_LOG_MARK, sqlAddMsg( state,
+				"Internal error: invalid type for bind variable" ));
+			state->error = 1;
+			break;
+	}
+
+	if( state->error )
+		osrfLogWarning( OSRF_LOG_MARK, sqlAddMsg( state,
+			"Unable to emit value of bind variable \"%s\"", bind->name ));
+}
+
+/**
+	@brief Add a number or quoted string to an SQL statement.
+	@param state Pointer to the query-building context.
+	@param numeric Boolean; true if the value is expected to be a number
+	@param obj Pointer to the jsonObject whose value is to be added to the SQL.
+*/
+static void buildScalar( BuildSQLState* state, int numeric, const jsonObject* obj ) {
+	switch( obj->type ) {
+		case JSON_HASH :
+			osrfLogError( OSRF_LOG_MARK, sqlAddMsg( state,
+				"Internal error: hash value for bind variable" ));
+			state->error = 1;
+			break;
+		case JSON_ARRAY :
+			osrfLogError( OSRF_LOG_MARK, sqlAddMsg( state,
+				"Internal error: array value for bind variable" ));
+			state->error = 1;
+			break;
+		case JSON_STRING :
+			if( numeric ) {
+				sqlAddMsg( state,
+					"Invalid value for bind variable: expected a string, found a number" );
+				state->error = 1;
+			} else {
+				// To do: escape special characters in the string
+				buffer_add_char( state->sql, '\'' );
+				buffer_add( state->sql, jsonObjectGetString( obj ));
+				buffer_add_char( state->sql, '\'' );
+			}
+			break;
+		case JSON_NUMBER :
+			if( numeric ) {
+				buffer_add( state->sql, jsonObjectGetString( obj ));
+			} else {
+				sqlAddMsg( state,
+					"Invalid value for bind variable: expected a number, found a string" );
+				state->error = 1;
+			}
+			break;
+		case JSON_NULL :
+			buffer_add( state->sql, "NULL" );
+			break;
+		case JSON_BOOL :
+			osrfLogError( OSRF_LOG_MARK, sqlAddMsg( state,
+				"Internal error: boolean value for bind variable" ));
+			state->error = 1;
+			break;
+		default :
+			osrfLogError( OSRF_LOG_MARK, sqlAddMsg( state,
+				"Internal error: corrupted value for bind variable" ));
+			state->error = 1;
+			break;
+	}
+}
+
 static void add_newline( BuildSQLState* state ) {
 	buffer_add_char( state->sql, '\n' );
 

Modified: trunk/Open-ILS/src/c-apps/oils_buildq.c
===================================================================
--- trunk/Open-ILS/src/c-apps/oils_buildq.c	2010-05-18 20:12:38 UTC (rev 16449)
+++ trunk/Open-ILS/src/c-apps/oils_buildq.c	2010-05-19 12:46:18 UTC (rev 16450)
@@ -23,14 +23,18 @@
 */
 BuildSQLState* buildSQLStateNew( dbi_conn dbhandle ) {
 
-	BuildSQLState* state = safe_malloc( sizeof( BuildSQLState ) );
-	state->dbhandle    = dbhandle;
-	state->error       = 0;
-	state->error_msgs  = osrfNewStringArray( 16 );
-	state->sql         = buffer_init( 128 );
-	state->query_stack = NULL;
-	state->expr_stack  = NULL;
-	state->indent      = 0;
+	BuildSQLState* state   = safe_malloc( sizeof( BuildSQLState ) );
+	state->dbhandle        = dbhandle;
+	state->result          = NULL;
+	state->error           = 0;
+	state->error_msgs      = osrfNewStringArray( 16 );
+	state->sql             = buffer_init( 128 );
+	state->bindvar_list    = NULL;  // Don't build it until we need it
+	state->query_stack     = NULL;
+	state->expr_stack      = NULL;
+	state->indent          = 0;
+	state->defaults_usable = 0;
+	state->values_required = 0;
 
 	return state;
 }
@@ -53,8 +57,14 @@
 void buildSQLStateFree( BuildSQLState* state ){
 	
 	if( state ) {
+		if( state->result ) {
+			dbi_result_free( state->result );
+			state->result = NULL;
+		}
 		osrfStringArrayFree( state->error_msgs );
 		buffer_free( state->sql );
+		if( state->bindvar_list )
+			osrfHashFree( state->bindvar_list );
 		while( state->query_stack )
 			pop_id( &state->query_stack );
 		while( state->expr_stack )

Modified: trunk/Open-ILS/src/c-apps/oils_qstore.c
===================================================================
--- trunk/Open-ILS/src/c-apps/oils_qstore.c	2010-05-18 20:12:38 UTC (rev 16449)
+++ trunk/Open-ILS/src/c-apps/oils_qstore.c	2010-05-19 12:46:18 UTC (rev 16450)
@@ -77,6 +77,10 @@
 	if ( !oilsIDLInit( osrf_settings_host_value( "/IDL" )))
 		return 1; /* return non-zero to indicate error */
 
+	// Set the SQL options.  Here the second and third parameters are irrelevant, but we need
+	// to set the module name for use in error messages.
+	oilsSetSQLOptions( modulename, 0, 100 );
+
 	growing_buffer* method_name = buffer_init( 64 );
 
 	OSRF_BUFFER_ADD( method_name, modulename );
@@ -188,6 +192,8 @@
 	osrfLogInfo( OSRF_LOG_MARK, "Loading query for id # %d", query_id );
 
 	BuildSQLState* state = buildSQLStateNew( dbhandle );
+	state->defaults_usable = 1;
+	state->values_required = 0;
 	StoredQ* query = getStoredQuery( state, query_id );
 	if( state->error ) {
 		osrfLogWarning( OSRF_LOG_MARK, "Unable to load stored query # %d", query_id );

Modified: trunk/Open-ILS/src/c-apps/oils_storedq.c
===================================================================
--- trunk/Open-ILS/src/c-apps/oils_storedq.c	2010-05-18 20:12:38 UTC (rev 16449)
+++ trunk/Open-ILS/src/c-apps/oils_storedq.c	2010-05-19 12:46:18 UTC (rev 16450)
@@ -9,6 +9,7 @@
 #include "opensrf/utils.h"
 #include "opensrf/log.h"
 #include "opensrf/string_array.h"
+#include "opensrf/osrf_hash.h"
 #include "openils/oils_buildq.h"
 
 #define PRINT if( verbose ) printf
@@ -36,6 +37,10 @@
 static SelectItem* constructSelectItem( BuildSQLState* state, dbi_result result );
 static void selectListFree( SelectItem* sel );
 
+static BindVar* getBindVar( BuildSQLState* state, const char* name );
+static BindVar* constructBindVar( BuildSQLState* state, dbi_result result );
+static void bindVarFree( char* name, void* p );
+
 static Expression* getExpression( BuildSQLState* state, int id );
 static Expression* constructExpression( BuildSQLState* state, dbi_result result );
 static void expressionFree( Expression* exp );
@@ -52,6 +57,7 @@
 static StoredQ* free_storedq_list = NULL;
 static FromRelation* free_from_relation_list = NULL;
 static SelectItem* free_select_item_list = NULL;
+static BindVar* free_bindvar_list = NULL;
 static Expression* free_expression_list = NULL;
 static IdNode* free_id_node_list = NULL;
 static QSeq* free_qseq_list = NULL;
@@ -221,6 +227,7 @@
 			freeQSeqList( child_list );
 			fromRelationFree( from_clause );
 			selectListFree( select_list );
+			state->error = 1;
 			return NULL;
 		}
 	}
@@ -236,6 +243,7 @@
 			freeQSeqList( child_list );
 			fromRelationFree( from_clause );
 			selectListFree( select_list );
+			state->error = 1;
 			return NULL;
 		}
 	}
@@ -613,6 +621,7 @@
 			osrfLogWarning( OSRF_LOG_MARK, sqlAddMsg( state,
 				"Unable to load ON condition for FROM relation # %d", id ));
 			joinListFree( join_list );
+			state->error = 1;
 			return NULL;
 		}
 		else
@@ -810,6 +819,7 @@
 	if( !expression ) {
 		osrfLogError( OSRF_LOG_MARK, sqlAddMsg( state,
 			"Unable to fetch expression for id = %d", expression_id ));
+		state->error = 1;
 		return NULL;
 	};
 
@@ -833,6 +843,13 @@
 	return sel;
 }
 
+/**
+	@brief Free a list of SelectItems.
+	@param sel Pointer to the first item in the list to be freed.
+
+	Free the column alias and expression owned by each item.  Put the entire list into a free
+	list of SelectItems.
+*/
 static void selectListFree( SelectItem* sel ) {
 	if( !sel )
 		return;    // Nothing to free
@@ -855,6 +872,152 @@
 	free_select_item_list = first;
 }
 
+/**
+	@brief Given the name of a bind variable, build a corresponding BindVar.
+	@param state Pointer to the query-building context.
+	@param name Name of the bind variable.
+	@return Pointer to the newly-built BindVar.
+
+	Since the same bind variable may appear multiple times, we load it only once for the
+	entire query, and reference the one copy wherever needed.
+*/
+static BindVar* getBindVar( BuildSQLState* state, const char* name ) {
+	BindVar* bind = NULL;
+	if( state->bindvar_list ) {
+		bind = osrfHashGet( state->bindvar_list, name );
+		if( bind )
+			return bind;   // Already loaded it...
+	}
+
+	// Load a BindVar from the Database.
+	dbi_result result = dbi_conn_queryf( state->dbhandle,
+		"SELECT name, type, description, default_value, label "
+		"FROM query.bind_variable WHERE name = \'%s\';", name );
+	if( result ) {
+		if( dbi_result_first_row( result ) ) {
+			bind = constructBindVar( state, result );
+			if( bind ) {
+				PRINT( "Got a bind variable for %s\n", name );
+			} else 
+				osrfLogError( OSRF_LOG_MARK, sqlAddMsg( state,
+					"Unable to load bind variable \"%s\"", name ));
+		} else {
+			osrfLogError( OSRF_LOG_MARK, sqlAddMsg( state,
+				"No bind variable found with name \"%s\"", name ));
+		}
+	} else {
+		const char* msg;
+		int errnum = dbi_conn_error( state->dbhandle, &msg );
+		osrfLogError( OSRF_LOG_MARK, sqlAddMsg( state,
+			"Unable to query query.bind_variable table for \"%s\": #%d %s",
+			name, errnum, msg ? msg : "No description available" ));
+	}
+
+	if( bind ) {
+		// Add the new bind variable to the list
+		if( !state->bindvar_list ) {
+			// Don't have a list yet?  Start one.
+			state->bindvar_list = osrfNewHash();
+			osrfHashSetCallback( state->bindvar_list, bindVarFree );
+		}
+		osrfHashSet( state->bindvar_list, bind, name );
+	} else
+		state->error = 1;
+
+	return bind;
+}
+
+/**
+	@brief Construct a BindVar to represent a bind variable.
+	@param Pointer to the query-building context.
+	@param result Database cursor positioned at a row in query.bind_variable.
+	@return Pointer to a newly constructed BindVar, if successful, or NULL if not.
+
+	The calling code is responsible for freeing the BindVar by calling bindVarFree().
+*/
+static BindVar* constructBindVar( BuildSQLState* state, dbi_result result ) {
+
+	const char* name = dbi_result_get_string_idx( result, 1 );
+
+	const char* type_str = dbi_result_get_string_idx( result, 2 );
+	BindVarType type;
+	if( !strcmp( type_str, "string" ))
+		type = BIND_STR;
+	else if( !strcmp( type_str, "number" ))
+		type = BIND_NUM;
+	else if( !strcmp( type_str, "string_list" ))
+		type = BIND_STR_LIST;
+	else if( !strcmp( type_str, "number_list" ))
+		type = BIND_NUM_LIST;;
+
+	const char* description = dbi_result_get_string_idx( result, 3 );
+
+	// The default value is encoded as JSON.  Translate it into a jsonObject.
+	const char* default_value_str = dbi_result_get_string_idx( result, 4 );
+	jsonObject* default_value = NULL;
+	if( default_value_str ) {
+		default_value = jsonParse( default_value_str );
+		if( !default_value ) {
+			osrfLogError( OSRF_LOG_MARK, sqlAddMsg( state,
+				"Unable to parse JSON string for default value of bind variable \"%s\"",
+				name ));
+			state->error = 1;
+			return NULL;
+		}
+	}
+
+	const char* label = dbi_result_get_string_idx( result, 5 );
+
+	// Allocate a BindVar: from the free list if possible, from the heap if necessary
+	BindVar* bind = NULL;
+	if( free_bindvar_list ) {
+		bind = free_bindvar_list;
+		free_bindvar_list = free_bindvar_list->next;
+	} else
+		bind = safe_malloc( sizeof( BindVar ) );
+
+	bind->next = NULL;
+	bind->name = strdup( name );
+	bind->label = strdup( label );
+	bind->type = type;
+	bind->description = strdup( description );
+	bind->default_value = default_value;
+	bind->actual_value = NULL;
+
+	return bind;
+}
+
+/**
+	@brief Deallocate a BindVar.
+	@param key Pointer to the bind variable name (not used).
+	@param p Pointer to the BindVar to be deallocated, cast to a void pointer.
+
+	Free the strings and jsonObjects owned by the BindVar, and put the BindVar itself into a
+	free list.
+
+	This function is a callback installed in an osrfHash; hence the peculiar signature.
+*/
+static void bindVarFree( char* key, void* p ) {
+	if( p ) {
+		BindVar* bind = p;
+		free( bind->name );
+		free( bind->label );
+		free( bind->description );
+		if( bind->default_value ) {
+			jsonObjectFree( bind->default_value );
+			bind->default_value = NULL;
+		}
+		if( bind->actual_value ) {
+			jsonObjectFree( bind->actual_value );
+			bind->actual_value = NULL;
+		}
+
+		// Prepend to free list
+		bind->next = free_bindvar_list;
+		free_bindvar_list = bind;
+	}
+}
+
 static Expression* getExpression( BuildSQLState* state, int id ) {
 	
 	// Check the stack to see if the current expression is nested inside itself.  If it is,
@@ -868,10 +1031,11 @@
 	} else
 		push_id( &state->expr_stack, id, NULL );
 
-		Expression* exp = NULL;
+	Expression* exp = NULL;
 	dbi_result result = dbi_conn_queryf( state->dbhandle,
 		"SELECT id, type, parenthesize, parent_expr, seq_no, literal, table_alias, column_name, "
-		"left_operand, operator, right_operand, function_id, subquery, cast_type, negate "
+		"left_operand, operator, right_operand, function_id, subquery, cast_type, negate, "
+		"bind_variable "
 		"FROM query.expression WHERE id = %d;", id );
 	if( result ) {
 		if( dbi_result_first_row( result ) ) {
@@ -914,6 +1078,8 @@
 	ExprType type;
 	if( !strcmp( type_str, "xbet" ))
 		type = EXP_BETWEEN;
+	else if( !strcmp( type_str, "xbind" ))
+		type = EXP_BIND;
 	else if( !strcmp( type_str, "xbool" ))
 		type = EXP_BOOL;
 	else if( !strcmp( type_str, "xcase" ))
@@ -989,10 +1155,12 @@
 		cast_type_id = dbi_result_get_int_idx( result, 14 );
 
 	int negate = oils_result_get_bool_idx( result, 15 );
+	const char* bind_variable = dbi_result_get_string_idx( result, 16 );
 
 	Expression* left_operand = NULL;
 	Expression* right_operand = NULL;
 	StoredQ* subquery = NULL;
+	BindVar* bind = NULL;
 
 	if( EXP_OPERATOR == type ) {
 		// Load left and/or right operands
@@ -1092,6 +1260,25 @@
 			}
 			PRINT( "\tExpression is subquery %d\n", subquery_id );
 		}
+	} else if( EXP_BIND == type ) {
+		if( bind_variable ) {
+			// To do: Build a BindVar
+			bind = getBindVar( state, bind_variable );
+			if( ! bind ) {
+				osrfLogWarning( OSRF_LOG_MARK, sqlAddMsg( state,
+					"Unable to load bind variable \"%s\" for expression # %d",
+					bind_variable, id ));
+				state->error = 1;
+				return NULL;
+			}
+			PRINT( "\tBind variable is \"%s\"\n", bind_variable );
+		} else {
+			osrfLogWarning( OSRF_LOG_MARK, sqlAddMsg( state,
+				"No variable specified for bind variable expression # %d",
+			bind_variable, id ));
+			state->error = 1;
+			return NULL;
+		}
 	}
 
 	// Allocate an Expression: from the free list if possible, from the heap if necessary
@@ -1120,6 +1307,7 @@
 	exp->subquery = subquery;
 	exp->cast_type_id = subquery_id;
 	exp->negate = negate;
+	exp->bind = bind;
 
 	return exp;
 }
@@ -1152,7 +1340,10 @@
 			storedQFree( exp->subquery );
 			exp->subquery = NULL;
 		}
+		// We don't free the bind member here because the Expression doesn't own it;
+		// the bindvar_list hash owns it, so that multiple Expressions can reference it.
 
+		// Prepend to the free list
 		exp->next = free_expression_list;
 		free_expression_list = exp;
 	}
@@ -1225,6 +1416,7 @@
 	if( !expression ) {
 		osrfLogError( OSRF_LOG_MARK, sqlAddMsg( state,
 			"Unable to fetch ORDER BY expression for id = %d", expression_id ));
+		state->error = 1;
 		return NULL;
 	};
 
@@ -1470,6 +1662,14 @@
 		free( ord );
 		ord = free_order_item_list;
 	}
+
+	// Free all the nodes in the bind variable free list
+	BindVar* bind = free_bindvar_list;
+	while( bind ) {
+		free_bindvar_list = bind->next;
+		free( bind );
+		bind = free_bindvar_list;
+	}
 }
 
 /**



More information about the open-ils-commits mailing list