More FP-correct way to create an update sql query

Question

I am working on access a database using F# and my initial attempt at creating a function to create the update query is flawed.

let BuildUserUpdateQuery (oldUser:UserType) (newUser:UserType) =
    let buf = new System.Text.StringBuilder("UPDATE users SET ");
    if (oldUser.FirstName.Equals(newUser.FirstName) = false)  then buf.Append("SET first_name='").Append(newUser.FirstName).Append("'" ) |> ignore
    if (oldUser.LastName.Equals(newUser.LastName) = false)  then buf.Append("SET last_name='").Append(newUser.LastName).Append("'" ) |> ignore
    if (oldUser.UserName.Equals(newUser.UserName) = false)  then buf.Append("SET username='").Append(newUser.UserName).Append("'" ) |> ignore
    buf.Append(" WHERE id=").Append(newUser.Id).ToString()

This doesn't properly put a , between any update parts after the first, for example:

UPDATE users SET first_name='Firstname', last_name='lastname' WHERE id=...

I could put in a mutable variable to keep track when the first part of the set clause is appended, but that seems wrong.

I could just create an list of tuples, where each tuple is oldtext, newtext, columnname, so that I could then loop through the list and build up the query, but it seems that I should be passing in a StringBuilder to a recursive function, returning back a boolean which is then passed as a parameter to the recursive function.

Does this seem to be the best approach, or is there a better one?

UPDATE:

Here is what I am using as my current solution, as I wanted to make it more generalized, so I just need to write an abstract class for my entities to derive from and they can use the same function. I chose to split up how I do the function so I can pass in how to create the SET part of the update so I can test with different ideas.

let BuildUserUpdateQuery3 (oldUser:UserType) (newUser:UserType) =
    let properties = List.zip3 oldUser.ToSqlValuesList newUser.ToSqlValuesList oldUser.ToSqlColumnList 
    let init = false, new StringBuilder()
    let anyChange, (formatted:StringBuilder) = 
        properties |> Seq.fold (fun (anyChange, sb) (oldVal, newVal, name) ->
            match(oldVal=newVal) with
            | true -> anyChange, sb
            | _ ->
                match(anyChange) with
                | true -> true, sb.AppendFormat(",{0} = '{1}'", name, newVal)
                | _ -> true, sb.AppendFormat("{0} = '{1}'", name, newVal)                    
            ) init
    formatted.ToString()

let BuildUserUpdateQuery (oldUser:UserType) (newUser:UserType) (updatequery:UserType->UserType->String) =
    let buf = StringBuilder("UPDATE users SET ");
    buf.AppendFormat(" {0} WHERE id={1}", (updatequery oldUser newUser), newUser.Id)

let UpdateUser conn (oldUser:UserType) (newUser:UserType) =
    let query = BuildUserUpdateQuery oldUser newUser BuildUserUpdateQuery3
    execNonQuery conn (query.ToString())

Answer 1

Is this the tuple solution you had in mind?

let BuildUserUpdateQuery (oldUser:UserType) (newUser:UserType) =
    let buf = StringBuilder("UPDATE users set ")
    let properties = 
        [(oldUser.FirstName, newUser.FirstName, "first_name")
         (oldUser.LastName, newUser.LastName, "last_name")
         (oldUser.UserName, newUser.UserName, "username")]
         |> Seq.map (fun (oldV, newV, field) -> 
                        if oldV <> newV 
                            then sprintf "%s='%s'" field newV 
                            else null)
         |> Seq.filter (fun p -> p <> null)
         |> Seq.toArray
    if properties.Length = 0
        then None
        else
            bprintf buf "%s" (String.Join(", ", properties))
            bprintf buf " where id=%d" newUser.Id
            Some <| buf.ToString()

I don't see how the recursive solution could be simpler than this...

BTW I would strongly advise to use proper SQL parameters instead of just concatenating the values, you might become vulnerable to injection attacks...

Answer 2

Just for completeness, here is a version that does the same thing directly using the fold function. This can be done quite elegantly, because methods of StringBuilder return the StringBuilder (which allows you to chain them in C#). This can be also used nicely for folding.

Let's assume that we have the list of tuples from the solution by Mauricio:

let properties =  
   [ (oldUser.FirstName, newUser.FirstName, "first_name") 
     (oldUser.LastName, newUser.LastName, "last_name") 
     (oldUser.UserName, newUser.UserName, "username") ] 

Now you can write the following code (it also returns a flag whether anything has changed):

let init = false, new StringBuilder()
let anyChange, formatted = 
  properties |> Seq.fold (fun (anyChange, sb) (oldVal, newVal, name) ->
      if (oldVal = newVal) anyChange, sb
      else true, sb.AppendFormat("{0} = '{1}'", name, newVal)) init

The state kept during folding has type bool * StringBuilder and we start with an initial value containing empty string builder and false. In each step, we either return the original state (if value is the same as previous) or a new state containing true and a new version of the StringBuilder returned by AppendFormat.

Using recursion explicitly would also work, but when you can use some built-in F# function, it is usually easier to use this approach. If you needed to process nested entities of each entity, you could use the Seq.collect function together with recursion to get a list of properties that you need to process using fold. Pseudo-code might look like this:

let rec processEntities list names =
  // Pair matching entity with the name from the list of names
  List.zip list names 
  |> List.collect (fun (entity, name) ->
    // Current element containing old value, new value and property name
    let current = (entity.OldValue, entity.NewValue, name)
    // Recursively proces nested entitites
    let nested = processEntities entity.Nested
    current::nested)

This can be more elegantly written using sequence expressions:

let rec processEntities list =
  seq { for entity, name in List.zip list names do 
          yield (entity.OldValue, entity.NewValue, name)
          yield! processEntities entity.Nested }

Then you could simply call processEntities which returns a flat list of entities and process the entities using fold as in the first case.

Answer 3

I like both Mauricio's and Tomas's solutions, but perhaps this is more like what you originally envisioned?

let sqlFormat (value:'a) = //'
  match box value with
  | :? int | :? float -> value.ToString()
  | _ -> sprintf "'%A'" value // this should actually use database specific escaping logic to make it safe

let appendToQuery getProp (sqlName:string) (oldEntity,newEntity,statements) =
  let newStatements =
    if (getProp oldEntity <> getProp newEntity) then (sprintf "%s=%s" sqlName (sqlFormat (getProp newEntity)))::statements
    else statements
  (oldEntity, newEntity, newStatements)

let createUserUpdate (oldUser:UserType) newUser =
  let (_,_,statements) =
    (oldUser,newUser,[])
    |> appendToQuery (fun u -> u.FirstName) "first_name"
    |> appendToQuery (fun u -> u.LastName) "last_name"
    |> appendToQuery (fun u -> u.UserName) "username"
    // ...

  let statementArr = statements |> List.toArray
  if (statementArr.Length > 0) then
    let joinedStatements = System.String.Join(", ", statementArr)
    Some(sprintf "UPDATE users SET %s WHERE ID=%i" joinedStatements newUser.ID)
  else
    None

If you have lots of properties to check, this may be a bit more concise. One benefit to this approach is that it works even if you're checking properties of multiple types, whereas the other approaches require all properties to have the same type (since they're stored in a list).