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

diff --git a/Open-ILS/include/openils/oils_buildq.h b/Open-ILS/include/openils/oils_buildq.h
index 6f25490..c3f07f0 100644 (file)
--- 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 af0e27c..bf83fa6 100644 (file)
--- 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 7ecd4ff..3c90dc1 100644 (file)
--- 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 ) ) {