SQL Procedure incorrectly checks if value exists

Question

I'm building a Windows Forms Application with a connection to an SQL Database. On start-up of my app, it will send some queries to the database to compare values:

Here is the code that generates the query:

    private void CreateInsertQuery(DirectoryInfo source, string Printer)
    {
         foreach (FileInfo file in source.GetFiles())
        {
            queries.Add("EXECUTE sqlp_UpdateInsertFiles '"+ file.Name +"', '" + Printer + "'"); 
        }
         foreach (DirectoryInfo folder in source.GetDirectories())
        {
            queries.Add("EXECUTE sqlp_UpdateInsertFiles '" + folder.Name + "', '" + Printer + "'");
            CreateInsertQuery(folder, Printer); 
        }
    }

queries is a public List.

This is the code that sends the query to the db:

   public bool InsertQueries()
    {
        con.Open(); 
        using(OleDbTransaction trans = con.BeginTransaction())
        {
            try
            {
                OleDbCommand cmd;
                foreach (string query in queries)
                {
                    try
                    {
                        cmd = new OleDbCommand(query, con, trans);
                        cmd.ExecuteNonQuery();
                    }
                    catch (Exception ex)
                    {
                        if (ex.HResult != -2147217873)
                        {
                            MessageBox.Show(ex.Message);
                        }
                    }
                }
                trans.Commit();
                con.Close();
                return true;
            }
            catch (Exception ex)
            {
                trans.Rollback();
                con.Close();
                return false;
            }
        }

    }

In my SQL database, I've created a stored procedure that gets called when the database receives the query:

    AS
        BEGIN
            BEGIN TRANSACTION;
            SET NOCOUNT ON;
            BEGIN TRY
                IF EXISTS
                   (SELECT TOP 1 fName, Printer
                    FROM   dbo.FileTranslation
                    WHERE  fName = @fName AND Printer = @Printer)
                BEGIN
                    UPDATE dbo.FileTranslation
                        SET fName = @fName, Printer = @Printer
                END;
                ELSE
                BEGIN
                    INSERT INTO dbo.FileTranslation(fName, Printer) VALUES (@fName, @Printer);
                END;
            COMMIT TRANSACTION;
            
            END TRY
            BEGIN CATCH
                IF @@TRANCOUNT > 0
                BEGIN
                    ROLLBACK TRANSACTION;
                END
            END CATCH
        END;
GO

When I run my application on an empty database, then the values will get added without any problem:

.

I also do not get any error occurrences. It's only when I start my application for a second time, that the first 2 query's do not get checked on the IF EXISTS. Because it is still inserting the data into my database, 5x to be exact.

.

Which is weird as there are only 2 queries containing the data, but it gets executed every time.

Answer 1

I assume the id column is an sql identity column, right? Because the first continous 7 entries are all the same I think your app is started on multiple threads which at the beginning are executing head-by-head but later their execution diverges maybe because of extra time of exception handling block. That's why only the first records are multiplied.

The problem is that your stored procedure isn't thread-safe. No locks placed on dbo.FileTranslation table by the IF EXISTS(SELECT ... which in parallel execution may result in situation where multiple executing stored procedures find the required record unexisting and will continue with the INSERT branch.

Applying the answers from https://dba.stackexchange.com/questions/187405/sql-server-concurrent-inserts-and-deletes thread this may work for you:

...
IF EXISTS
   (SELECT TOP 1 fName, Printer
    FROM   dbo.FileTranslation WITH (UPDLOCK, SERIALIZABLE)
    WHERE  fName = @fName AND Printer = @Printer)
...

PS: Not related to your question but take care about @Lamu's comment on SQL injection and use try...finally or using pattern for you conn handling!