Building a flexible upsert statement for SQL This is a single function in C#. It works fine. My concern is that although I've made the code as short and as multi-functional as I can, what it's doing seems very unclear. It looks like the sort of thing I'm going to come back to in six months and hate myself for writing.

Its purpose is to build and execute generic "upsert" sql statements for a variety of scenarios. Each record that it comes across has a name, an address and a flag to say whether the operation on that record is a delete and whether that record represents someone who's gone away. The former tells us whether we're setting a bit value to true or false, while the latter determines the column name.

How can I make this code clearer without turning it into a mess of flow control?

private void Sprail(FileRow fr, string columnName)
{
    List&lt;Suppression&gt; addressMatches = GetMatchingRecordsFromDb(SanitiseForMySql(fr.CleanedAddress), SanitiseForMySql(fr.CleanedName));

    using (MySqlConnection con = new MySqlConnection(_connectionString))
    {
        string sqlTargetFlag = columnName;
        string sqlTargetValue = "1";
        string sql = "";

        if (fr.IsDeleted)
        {
            sqlTargetValue = "0";
        }

        if (fr.IsGoneaway)
        {
            sqlTargetFlag = sqlTargetFlag + "Goneaway";
        }

        if (string.IsNullOrWhiteSpace(fr.CleanedAddress))
        {
            sql = BuildInsertSql(sqlTargetFlag, fr, sqlTargetValue);
        }

        foreach (Suppression s in addressMatches)
        {
            if (fr.CleanedName.ToString().ToLower() == s.NameKey.ToLower())
            {
                sql = string.Format("UPDATE MyTable set {0} = {1} where SuppressionId = {2}", sqlTargetFlag, sqlTargetValue, s.SuppressionId);
                break;
            }
        }

        if(sql != "")
        {
            con.Open();
            MySqlCommand cmd = new MySqlCommand(sql, con);
            cmd.ExecuteNonQuery();
        }
    }
}

private string BuildInsertSql(string sqlTargetFlag, FileRow fr, string sqlTargetValue)
{
    return string.Format(@"INSERT INTO MyTable (`AddressKey`, `NameKey`, `{0}`) VALUES ('{1}', '{2}', '{3}')", 
                                sqlTargetFlag, SanitiseForMySql(fr.CleanedAddress), SanitiseForMySql(fr.CleanedName), sqlTargetValue);
}
Code like this is bound to be a bit hairy.

However

You aren't using parameters

This is a big security vulnerability. What's that SanitiseForMySql method doing? Is it doing everything by the book? Does it handle every possible obscure corner case? Let ADO the MySQL connector manipulate your query, especially if there's a chance you're handling values supplied by the user.

MySQL can perform the so-called MERGE (also called UPSERT)

You can do it via INSERT ... ON DUPLICATE KEY UPDATE statement. In case you are not aware what that is, it's basically a way to either insert or update in case the INSERT fails because of a duplicate key error. It's for sure faster than making a separate query to retrieve the records that are currently present. 

By the way, code like this leaves your code open to race conditions in case another user inserts a record exactly between when you read the current records and the moment you execute your INSERT. This is bad, and you're not currently guarding against it. MySQL can do it for you, take advantage of it.

EDIT

Also


This:

        if (fr.IsGoneaway)
        {
            sqlTargetFlag = sqlTargetFlag + "Goneaway";
        }

are you sure this is correct? Concatenating a column name to the other like that? Are you assuming the column name will be passed in followed by a comma? And anyway, isn't it going to blow up with a syntax error when you do the `UPDATE`? Or will the column name get passed as an empty string? Might be a good idea to check what garbage you are receiving through your parameters, assuming said checks aren't already in place at an upper layer. How can you be sure all callers will remember to pass your arguments exactly the way you expect them? Plus, just my two cents, but if my assumptions are correct, this is not a very fun API to work with.