Inserting persons in file into SQLite

Question

I've just started working with SQLite in C# to test various features of an application. In building my first SQLite example I wanted to insert a large .csv into a table (Person) with two columns, A and B. The .csv file is 50MB in size and contains multiple rows of the same data ("abc, def").

I wrote the following code, but I think this is fairly inefficient and am looking for advice on a better method(s) to optimize this code to read the file and finally insert it into SQLite.

Regex splitRx = new Regex(@",\s*", RegexOptions.Compiled);
ArrayList al = new ArrayList();

using (StreamReader sr = new StreamReader(@"c:\Temp\test.csv"))
{
    string line = null;
    int ln = 0;
    while ((line = sr.ReadLine()) != null)
    {
        string[] fields = splitRx.Split(line);
        if (fields.Length != 2)
        {
            Console.WriteLine("Invalid Input on line:" + ln);
            continue;
        }
        ln++;
        al.Add(fields);
    }
}

using (var conn = new SQLiteConnection(@"Data Source=C:\Temp\test.sqlite"))
{
    conn.Open();

    using (var cmd = new SQLiteCommand(conn))
    {
        using (var transaction = conn.BeginTransaction())
        {
            foreach (string[] sa in al)
            {
                cmd.CommandText =
                    "INSERT INTO Person (FirstName, LastName) VALUES ('" + sa[0] + "', '" + sa[1] + "');";
                cmd.ExecuteNonQuery();
            }

            transaction.Commit();
        }
    }

    conn.Close();
}

Answer 1

1. You parse the CSV fields as string yet you use an ArrayList - I'd go for the generic List<string> in that case.

2. Your CSV parser is broken in the general sense - i.e. it can't deal with things like this 1234, "foo,bar", "go" - it would split the foo,bar value in the middle. Given your data that might not matter but it's probably easier to look for a 3rd party CSV parser on NuGet to future proof it.

3. Given that you ignore invalid lines in the CSV file anyway I'd consider changing the implementation of the CSV reading into an enumerator which avoids reading the entire file into memory. Something along these lines:

   private IEnumerable<string[]> ParseCsv(string fileName)
   {
       using (var sr = new StreamReader(fileName))
       {
           string line = null;
           int lineNumber = 0;
           while ((line = sr.ReadLine()) != null)
           {
               string[] fields = splitRx.Split(line);
               lineNumber ++;
               if (fields.Length != 2)
               {
                   Console.WriteLine("Invalid Input on line:" + lineNumber);
                   continue;
               }
               yield return fields;
           }
       }
   }
   

   then you can call it in your insert code:

       foreach (var sa in ParseCsv(@"c:\Temp\test.csv"))
       {
           cmd.CommandText =
               "INSERT INTO Person (FirstName, LastName) VALUES ('" + sa[0] + "', '" + sa[1] + "');";
           cmd.ExecuteNonQuery();
       }
   

4. In fact given that your inserts happen inside a single transaction you could consider throwing an exception rather than printing a message when an ill-formed line occurs. This way you get all or nothing - which might or might not be preferable (depends on what kind of data you are importing for what purposes)

Answer 2

This seems quite fine, in no small part thanks to the use of using (...) { ... } blocks to manage resources.

Use prepared statements

The biggest improvement would be to use prepared statements for inserting the records. See the example in the docs.

A small bug

You have a small bug in the line counting:

    if (fields.Length != 2)
    {
        Console.WriteLine("Invalid Input on line:" + ln);
        continue;
    }
    ln++;

Since you only increment the counter after the check on fields.length, the line number will be correct for the first invalid line, and off by one for the next invalid line, off by 2 for the one after, and so on. In other words, to keep the count correct, you need to do it before the if.

Use the var keyword consistently

You used the var keyword almost everywhere, except here:

using (StreamReader sr = new StreamReader(@"c:\Temp\test.csv"))

You can simplify with var sr = ...

Answer 3

@janos mentioned prepared statements in the context of improving the performance of the code. While this is true, there is a much more important reason to use a prepared statement. It will prevent SQL injection vulnerabilities.

You are currently using string concatenation to construct your SQL query with insufficient validation of the input you are receiving from an external source. If that text file contains a line with the following:

a, a'); DROP TABLE Person;#

The result of the concatenation would be:

INSERT INTO Person (FirstName, LastName) VALUES ('a', 'a'); DROP TABLE Person;#');

This is perfectly valid SQL and will be destroy all of the data that is currently in the database. Your code does nothing to prevent this from happening. However, if you use a prepared statement, the input values will be properly escaped and your database will be fine.

Never use string concatenation for SQL commands with outside input. Never assume outside input will be what your code expects it to be.