From 271c3c79c289149be5de76e56c2988609781a349 Mon Sep 17 00:00:00 2001 From: scottmk Date: Mon, 31 May 2010 03:03:03 +0000 Subject: [PATCH] 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 git-svn-id: svn://svn.open-ils.org/ILS/trunk@16537 dcc99617-32d9-48b4-a31d-7c20da2025e4 --- Open-ILS/include/openils/oils_buildq.h | 5 +- Open-ILS/src/c-apps/buildSQL.c | 35 ++++++-- Open-ILS/src/c-apps/oils_storedq.c | 143 +++++++++++++++++++++++---------- 3 files changed, 135 insertions(+), 48 deletions(-) diff --git a/Open-ILS/include/openils/oils_buildq.h b/Open-ILS/include/openils/oils_buildq.h index 6f25490328..c3f07f09ea 100644 --- a/Open-ILS/include/openils/oils_buildq.h +++ b/Open-ILS/include/openils/oils_buildq.h @@ -176,13 +176,16 @@ struct Expression_ { 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_ { diff --git a/Open-ILS/src/c-apps/buildSQL.c b/Open-ILS/src/c-apps/buildSQL.c index af0e27c573..bf83fa64ed 100644 --- a/Open-ILS/src/c-apps/buildSQL.c +++ b/Open-ILS/src/c-apps/buildSQL.c @@ -23,6 +23,7 @@ static void buildJoin( BuildSQLState* state, const FromRelation* join ); 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 @@ static void buildExpression( BuildSQLState* state, const Expression* expr ) { 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 @@ static void buildExpression( BuildSQLState* state, const Expression* expr ) { 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 @@ static void buildExpression( BuildSQLState* state, const Expression* expr ) { } /** + @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 buildExpression( BuildSQLState* state, const Expression* expr ) { */ 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 diff --git a/Open-ILS/src/c-apps/oils_storedq.c b/Open-ILS/src/c-apps/oils_storedq.c index 7ecd4ff9ba..3c90dc1290 100644 --- a/Open-ILS/src/c-apps/oils_storedq.c +++ b/Open-ILS/src/c-apps/oils_storedq.c @@ -450,6 +450,15 @@ void storedQFree( StoredQ* sq ) { } } +/** + @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 @@ static Expression* getExpression( BuildSQLState* state, int id ) { 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 @@ static Expression* constructExpression( BuildSQLState* state, dbi_result result else right_operand_id = dbi_result_get_int_idx( result, 11 ); - int function_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 )) + if( dbi_result_field_is_null_idx( result, 12 )) 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 @@ static Expression* constructExpression( BuildSQLState* state, dbi_result result 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 @@ static Expression* constructExpression( BuildSQLState* state, dbi_result result 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 @@ static Expression* constructExpression( BuildSQLState* state, dbi_result result return NULL; } } + } else if( EXP_ISNULL == type ) { if( -1 == left_operand_id ) { osrfLogWarning( OSRF_LOG_MARK, sqlAddMsg( state, @@ -1306,6 +1312,44 @@ static Expression* constructExpression( BuildSQLState* state, dbi_result result 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 @@ static Expression* constructExpression( BuildSQLState* state, dbi_result result 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 @@ static Expression* constructExpression( BuildSQLState* state, dbi_result result 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 @@ static void expressionFree( Expression* exp ) { 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 @@ static Expression* getExpressionList( BuildSQLState* state, int id ) { // 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 ) ) { -- 2.11.0