[open-ils-commits] r16537 - in trunk/Open-ILS: include/openils src/c-apps (scottmk)
svn at svn.open-ils.org
svn at svn.open-ils.org
Sun May 30 23:03:05 EDT 2010
Author: scottmk
Date: 2010-05-30 23:03:03 -0400 (Sun, 30 May 2010)
New Revision: 16537
Modified:
trunk/Open-ILS/include/openils/oils_buildq.h
trunk/Open-ILS/src/c-apps/buildSQL.c
trunk/Open-ILS/src/c-apps/oils_storedq.c
Log:
1. Add support for function calls. Note that certain functions have
peculiar calling syntax. They will require special handling as exceptions,
and are not yet supported.
2. Add a bit of sanity checking for numeric and string literal expressions.
3. Eliminate the function_id member of Expression.
M Open-ILS/include/openils/oils_buildq.h
M Open-ILS/src/c-apps/oils_storedq.c
M Open-ILS/src/c-apps/buildSQL.c
Modified: trunk/Open-ILS/include/openils/oils_buildq.h
===================================================================
--- trunk/Open-ILS/include/openils/oils_buildq.h 2010-05-30 03:50:47 UTC (rev 16536)
+++ trunk/Open-ILS/include/openils/oils_buildq.h 2010-05-31 03:03:03 UTC (rev 16537)
@@ -176,13 +176,16 @@
Expression* left_operand;
char* op;
Expression* right_operand;
- int function_id;
int subquery_id;
StoredQ* subquery;
int cast_type_id;
int negate; // Boolean
BindVar* bind;
Expression* subexp_list; // Linked list of subexpressions
+ // The next two columns come, not from query.expression,
+ // but from query.function_sig:
+ char* function_name;
+ int is_aggregate; // Boolean
};
struct QSeq_ {
Modified: trunk/Open-ILS/src/c-apps/buildSQL.c
===================================================================
--- trunk/Open-ILS/src/c-apps/buildSQL.c 2010-05-30 03:50:47 UTC (rev 16536)
+++ trunk/Open-ILS/src/c-apps/buildSQL.c 2010-05-31 03:03:03 UTC (rev 16537)
@@ -23,6 +23,7 @@
static void buildSelectList( BuildSQLState* state, const SelectItem* item );
static void buildOrderBy( BuildSQLState* state, const OrderItem* ord_list );
static void buildExpression( BuildSQLState* state, const Expression* expr );
+static void buildFunction( BuildSQLState* state, const Expression* exp );
static void buildSeries( BuildSQLState* state, const Expression* subexp_list, const char* op );
static void buildBindVar( BuildSQLState* state, const BindVar* bind );
static void buildScalar( BuildSQLState* state, int numeric, const jsonObject* obj );
@@ -653,11 +654,7 @@
state->error = 1;
break;
case EXP_FUNCTION :
- if( expr->negate )
- buffer_add( state->sql, "NOT " );
-
- sqlAddMsg( state, "Function expressions not yet supported" );
- state->error = 1;
+ buildFunction( state, expr );
break;
case EXP_IN :
if( expr->left_operand ) {
@@ -678,7 +675,7 @@
buffer_add_char( state->sql, ')' );
}
} else {
- buildSeries( state, expr->subexp_list, expr->op );
+ buildSeries( state, expr->subexp_list, NULL );
if( state->error )
sqlAddMsg( state, "Unable to build IN list" );
else
@@ -794,6 +791,29 @@
}
/**
+ @brief Build a function call.
+ @param state Pointer to the query-building context.
+ @param exp Pointer to an Expression representing a function call.
+
+ This function does not currently accommodate certain functions with idiosyncratic
+ syntax, such as the absence of parentheses, or the use of certain keywords in
+ in the parameter list.
+*/
+static void buildFunction( BuildSQLState* state, const Expression* expr ) {
+ if( expr->negate )
+ buffer_add( state->sql, "NOT " );
+
+ // We rely on the input side to ensure that the function name is available
+ buffer_add( state->sql, expr->function_name );
+ buffer_add_char( state->sql, '(' );
+
+ // Add the parameters, if any
+ buildSeries( state, expr->subexp_list, NULL );
+
+ buffer_add_char( state->sql, ')' );
+}
+
+/**
@brief Build a series of expressions separated by a specified operator, or by commas.
@param state Pointer to the query-building context.
@param subexp_list Pointer to the first Expression in a linked list.
@@ -804,6 +824,9 @@
*/
static void buildSeries( BuildSQLState* state, const Expression* subexp_list, const char* op ) {
+ if( !subexp_list)
+ return; // List is empty
+
int comma = 0; // Boolean; true if separator is a comma
int newline_needed = 0; // Boolean; true if operator is AND or OR
Modified: trunk/Open-ILS/src/c-apps/oils_storedq.c
===================================================================
--- trunk/Open-ILS/src/c-apps/oils_storedq.c 2010-05-30 03:50:47 UTC (rev 16536)
+++ trunk/Open-ILS/src/c-apps/oils_storedq.c 2010-05-31 03:03:03 UTC (rev 16537)
@@ -450,6 +450,15 @@
}
}
+/**
+ @brief Given an id from query.from_relation, load a FromRelation.
+ @param state Pointer the the query-building context.
+ @param id Id of the FromRelation.
+ @return Pointer to a newly-created FromRelation if successful, or NULL if not.
+
+ The calling code is responsible for freeing the new FromRelation by calling
+ fromRelationFree().
+*/
static FromRelation* getFromRelation( BuildSQLState* state, int id ) {
FromRelation* fr = NULL;
dbi_result result = dbi_conn_queryf( state->dbhandle,
@@ -1052,10 +1061,13 @@
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, "
- "bind_variable "
- "FROM query.expression WHERE id = %d;", id );
+ "SELECT exp.id, exp.type, exp.parenthesize, exp.parent_expr, exp.seq_no, "
+ "exp.literal, exp.table_alias, exp.column_name, exp.left_operand, exp.operator, "
+ "exp.right_operand, exp.subquery, exp.cast_type, exp.negate, exp.bind_variable, "
+ "func.function_name, COALESCE(func.is_aggregate, false) "
+ "FROM query.expression AS exp LEFT JOIN query.function_sig AS func "
+ "ON (exp.function_id = func.id) "
+ "WHERE exp.id = %d;", id );
if( result ) {
if( dbi_result_first_row( result ) ) {
exp = constructExpression( state, result );
@@ -1160,26 +1172,22 @@
else
right_operand_id = dbi_result_get_int_idx( result, 11 );
- int function_id;
+ int subquery_id;
if( dbi_result_field_is_null_idx( result, 12 ))
- function_id = -1;
- else
- function_id = dbi_result_get_int_idx( result, 12 );
-
- int subquery_id;
- if( dbi_result_field_is_null_idx( result, 13 ))
subquery_id = -1;
else
- subquery_id = dbi_result_get_int_idx( result, 13 );
+ subquery_id = dbi_result_get_int_idx( result, 12 );
int cast_type_id;
- if( dbi_result_field_is_null_idx( result, 14 ))
+ if( dbi_result_field_is_null_idx( result, 13 ))
cast_type_id = -1;
else
- cast_type_id = dbi_result_get_int_idx( result, 14 );
+ cast_type_id = dbi_result_get_int_idx( result, 13 );
- int negate = oils_result_get_bool_idx( result, 15 );
- const char* bind_variable = dbi_result_get_string_idx( result, 16 );
+ int negate = oils_result_get_bool_idx( result, 14 );
+ const char* bind_variable = dbi_result_get_string_idx( result, 15 );
+ const char* function_name = dbi_result_get_string_idx( result, 16 );
+ int is_aggregate = oils_result_get_bool_idx( result, 17 );
Expression* left_operand = NULL;
Expression* right_operand = NULL;
@@ -1206,25 +1214,6 @@
state->error = 1;
return NULL;
}
- } else if( EXP_OPERATOR == type ) {
- // Load left and/or right operands
- if( -1 == left_operand_id && -1 == right_operand_id ) {
- osrfLogWarning( OSRF_LOG_MARK, sqlAddMsg( state,
- "Expression # %d is an operator with no operands", id ));
- state->error = 1;
- return NULL;
- }
-
- if( left_operand_id != -1 ) {
- left_operand = getExpression( state, left_operand_id );
- if( !left_operand ) {
- osrfLogWarning( OSRF_LOG_MARK, sqlAddMsg( state,
- "Unable to get left operand in expression # %d", id ));
- state->error = 1;
- return NULL;
- }
- }
-
if( right_operand_id != -1 ) {
right_operand = getExpression( state, right_operand_id );
if( !right_operand ) {
@@ -1235,6 +1224,22 @@
return NULL;
}
}
+
+ } else if( EXP_FUNCTION == type ) {
+ if( !function_name ) {
+ osrfLogWarning( OSRF_LOG_MARK, sqlAddMsg( state,
+ "Function call expression # %d provides no function name", id ));
+ state->error = 1;
+ return NULL;
+ } else {
+ subexp_list = getExpressionList( state, id );
+ if( state->error ) {
+ osrfLogWarning( OSRF_LOG_MARK, sqlAddMsg( state,
+ "Unable to get parameter list for function expression # %d", id ));
+ return NULL;
+ }
+ }
+
} else if( EXP_IN == type ) {
if( -1 == left_operand_id ) {
osrfLogWarning( OSRF_LOG_MARK, sqlAddMsg( state,
@@ -1274,6 +1279,7 @@
return NULL;
}
}
+
} else if( EXP_ISNULL == type ) {
if( -1 == left_operand_id ) {
osrfLogWarning( OSRF_LOG_MARK, sqlAddMsg( state,
@@ -1306,6 +1312,44 @@
return NULL;
}
}
+
+ } else if( EXP_NUMBER == type ) {
+ if( !literal ) {
+ osrfLogWarning( OSRF_LOG_MARK, sqlAddMsg( state,
+ "Numeric expression # %d provides no numeric value", id ));
+ state->error = 1;
+ return NULL;
+ }
+
+ } else if( EXP_OPERATOR == type ) {
+ // Load left and/or right operands
+ if( -1 == left_operand_id && -1 == right_operand_id ) {
+ osrfLogWarning( OSRF_LOG_MARK, sqlAddMsg( state,
+ "Expression # %d is an operator with no operands", id ));
+ state->error = 1;
+ return NULL;
+ }
+
+ if( left_operand_id != -1 ) {
+ left_operand = getExpression( state, left_operand_id );
+ if( !left_operand ) {
+ osrfLogWarning( OSRF_LOG_MARK, sqlAddMsg( state,
+ "Unable to get left operand in expression # %d", id ));
+ state->error = 1;
+ return NULL;
+ }
+ }
+
+ if( right_operand_id != -1 ) {
+ right_operand = getExpression( state, right_operand_id );
+ if( !right_operand ) {
+ osrfLogWarning( OSRF_LOG_MARK, sqlAddMsg( state,
+ "Unable to get right operand in expression # %d", id ));
+ state->error = 1;
+ return NULL;
+ }
+ }
+
} else if( EXP_SERIES == type ) {
subexp_list = getExpressionList( state, id );
if( state->error ) {
@@ -1320,6 +1364,14 @@
return NULL;
}
+ } else if( EXP_STRING == type ) {
+ if( !literal ) {
+ osrfLogWarning( OSRF_LOG_MARK, sqlAddMsg( state,
+ "String expression # %d provides no string value", id ));
+ state->error = 1;
+ return NULL;
+ }
+
} else if( EXP_SUBQUERY == type ) {
if( -1 == subquery_id ) {
osrfLogWarning( OSRF_LOG_MARK, sqlAddMsg( state,
@@ -1366,13 +1418,14 @@
exp->left_operand = left_operand;
exp->op = operator ? strdup( operator ) : NULL;
exp->right_operand = right_operand;
- exp->function_id = function_id;
exp->subquery_id = subquery_id;
exp->subquery = subquery;
exp->cast_type_id = subquery_id;
exp->negate = negate;
exp->bind = bind;
exp->subexp_list = subexp_list;
+ exp->function_name = function_name ? strdup( function_name ) : NULL;
+ exp->is_aggregate = is_aggregate;
return exp;
}
@@ -1428,6 +1481,11 @@
exp->subexp_list = NULL;
}
+ if( exp->function_name ) {
+ free( exp->function_name );
+ exp->function_name = NULL;
+ }
+
// Prepend to the free list
exp->next = free_expression_list;
free_expression_list = exp;
@@ -1447,11 +1505,14 @@
// The ORDER BY is in descending order so that we can build the list by adding to
// the head, and it will wind up in the right order.
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, "
- "bind_variable "
- "FROM query.expression WHERE parent_expr = %d "
- "ORDER BY seq_no desc;", id );
+ "SELECT exp.id, exp.type, exp.parenthesize, exp.parent_expr, exp.seq_no, "
+ "exp.literal, exp.table_alias, exp.column_name, exp.left_operand, exp.operator, "
+ "exp.right_operand, exp.subquery, exp.cast_type, exp.negate, exp.bind_variable, "
+ "func.function_name, COALESCE(func.is_aggregate, false) "
+ "FROM query.expression AS exp LEFT JOIN query.function_sig AS func "
+ "ON (exp.function_id = func.id) "
+ "WHERE exp.parent_expr = %d "
+ "ORDER BY exp.seq_no desc;", id );
if( result ) {
if( dbi_result_first_row( result ) ) {
More information about the open-ils-commits
mailing list