Update SqliteCommand.Prepare implementation

.gitignore

@@ -1,5 +1,4 @@
 /.build/
-/global.json
 
 ## Ignore Visual Studio temporary files, build results, and
 ## files generated by popular Visual Studio add-ons.
@@ -11,7 +10,6 @@
 *.user
 *.userosscache
 *.sln.docstates
-*.user.sln*
 
 # User-specific files (MonoDevelop/Xamarin Studio)
 *.userprefs
@@ -46,6 +44,9 @@ TestResult.xml
 [Rr]eleasePS/
 dlldata.c
 
+# Benchmark Results
+BenchmarkDotNet.Artifacts/
+
 # .NET Core
 project.lock.json
 project.fragment.lock.json
@@ -187,6 +188,7 @@ AppPackages/
 BundleArtifacts/
 Package.StoreAssociation.xml
 _pkginfo.txt
+*.appx
 
 # Visual Studio cache files
 # files ending in .cache can be ignored
@@ -223,6 +225,7 @@ UpgradeLog*.htm
 # SQL Server files
 *.mdf
 *.ldf
+*.ndf
 
 # Business Intelligence projects
 *.rdl.data
@@ -281,6 +284,9 @@ __pycache__/
 # tools/**
 # !tools/packages.config
 
+# Tabs Studio
+*.tss
+
 # Telerik's JustMock configuration file
 *.jmconfig
 

src/Microsoft.Data.Sqlite.Core/Extensions/SqliteConnectionExtensions.cs

@@ -9,10 +9,12 @@ internal static class SqliteConnectionExtensions
             this SqliteConnection connection,
             string commandText)
         {
-            var command = connection.CreateCommand();
-            command.CommandText = commandText;
+            using (var command = connection.CreateCommand())
+            {
+                command.CommandText = commandText;
 
-            return command.ExecuteNonQuery();
+                return command.ExecuteNonQuery();
+            }
         }
 
         public static T ExecuteScalar<T>(
@@ -22,10 +24,12 @@ internal static class SqliteConnectionExtensions
 
         private static object ExecuteScalar(this SqliteConnection connection, string commandText)
         {
-            var command = connection.CreateCommand();
-            command.CommandText = commandText;
+            using (var command = connection.CreateCommand())
+            {
+                command.CommandText = commandText;
 
-            return command.ExecuteScalar();
+                return command.ExecuteScalar();
+            }
         }
     }
 }

src/Microsoft.Data.Sqlite.Core/Properties/Resources.resx

@@ -198,4 +198,10 @@
   <data name="CannotStoreNaN" xml:space="preserve">
     <value>Cannot store 'NaN' values.</value>
   </data>
+  <data name="DataReaderOpen" xml:space="preserve">
+    <value>An open reader is already associated with this command. Close it before opening a new one.</value>
+  </data>
+  <data name="SetRequiresNoOpenReader" xml:space="preserve">
+    <value>An open reader is associated with this command. Close it before changing the {propertyName} property.</value>
+  </data>
 </root>

src/Microsoft.Data.Sqlite.Core/SqliteCommand.cs

@@ -84,7 +84,12 @@ public override string CommandText
             get => _commandText;
             set
             {
-                if (!value.Equals(_commandText))
+                if (DataReader != null)
+                {
+                    throw new InvalidOperationException(Resources.SetRequiresNoOpenReader(nameof(CommandText)));
+                }
+
+                if (value != _commandText)
                 {
                     DisposePreparedStatements();
                     _commandText = value;
@@ -101,6 +106,11 @@ public new virtual SqliteConnection Connection
             get => _connection;
             set
             {
+                if (DataReader != null)
+                {
+                    throw new InvalidOperationException(Resources.SetRequiresNoOpenReader(nameof(Connection)));
+                }
+
                 if (value != _connection)
                 {
                     DisposePreparedStatements();
@@ -173,6 +183,12 @@ protected override DbParameterCollection DbParameterCollection
         /// <value>A value indicating how the results are applied to the row being updated.</value>
         public override UpdateRowSource UpdatedRowSource { get; set; }
 
+        /// <summary>
+        /// Gets or sets the data reader currently being used by the command, or null if none.
+        /// </summary>
+        /// <value>The data reader currently being used by the command.</value>
+        protected internal virtual SqliteDataReader DataReader { get; set; }
+
         /// <summary>
         /// Releases any resources used by the connection and closes it.
         /// </summary>
@@ -181,7 +197,7 @@ protected override DbParameterCollection DbParameterCollection
         /// </param>
         protected override void Dispose(bool disposing)
         {
-            DisposePreparedStatements();
+            DisposePreparedStatements(disposing);
 
             base.Dispose(disposing);
         }
@@ -220,12 +236,21 @@ public override void Prepare()
                 return;
             }
 
-            using (var enumerator = PrepareAndEnumerateStatements().GetEnumerator())
+            try
             {
-                while (enumerator.MoveNext())
+                using (var enumerator = PrepareAndEnumerateStatements().GetEnumerator())
                 {
+                    while (enumerator.MoveNext())
+                    {
+                    }
                 }
             }
+            catch
+            {
+                DisposePreparedStatements();
+
+                throw;
+            }
         }
 
         /// <summary>
@@ -255,6 +280,11 @@ public new virtual SqliteDataReader ExecuteReader(CommandBehavior behavior)
                 throw new ArgumentException(Resources.InvalidCommandBehavior(behavior));
             }
 
+            if (DataReader != null)
+            {
+                throw new InvalidOperationException(Resources.DataReaderOpen);
+            }
+
             if (_connection?.State != ConnectionState.Open)
             {
                 throw new InvalidOperationException(Resources.CallRequiresOpenConnection(nameof(ExecuteReader)));
@@ -281,70 +311,80 @@ public new virtual SqliteDataReader ExecuteReader(CommandBehavior behavior)
             var changes = 0;
             int rc;
             var stmts = new Queue<(sqlite3_stmt, bool)>();
+            var unprepared = _preparedStatements.Count == 0;
 
-            foreach (var stmt in _preparedStatements.Count == 0
-                ? PrepareAndEnumerateStatements()
-                : ResetAndEnumerateStatements())
+            try
             {
-                var boundParams = 0;
-
-                if (_parameters.IsValueCreated)
+                foreach (var stmt in unprepared
+                    ? PrepareAndEnumerateStatements()
+                    : _preparedStatements)
                 {
-                    boundParams = _parameters.Value.Bind(stmt);
-                }
+                    var boundParams = 0;
 
-                var expectedParams = raw.sqlite3_bind_parameter_count(stmt);
-                if (expectedParams != boundParams)
-                {
-                    var unboundParams = new List<string>();
-                    for (var i = 1; i <= expectedParams; i++)
+                    if (_parameters.IsValueCreated)
                     {
-                        var name = raw.sqlite3_bind_parameter_name(stmt, i);
+                        boundParams = _parameters.Value.Bind(stmt);
+                    }
 
-                        if (_parameters.IsValueCreated
-                            ||
-                            !_parameters.Value.Cast<SqliteParameter>().Any(p => p.ParameterName == name))
+                    var expectedParams = raw.sqlite3_bind_parameter_count(stmt);
+                    if (expectedParams != boundParams)
+                    {
+                        var unboundParams = new List<string>();
+                        for (var i = 1; i <= expectedParams; i++)
                         {
-                            unboundParams.Add(name);
+                            var name = raw.sqlite3_bind_parameter_name(stmt, i);
+
+                            if (_parameters.IsValueCreated
+                                ||
+                                !_parameters.Value.Cast<SqliteParameter>().Any(p => p.ParameterName == name))
+                            {
+                                unboundParams.Add(name);
+                            }
                         }
-                    }
 
-                    throw new InvalidOperationException(Resources.MissingParameters(string.Join(", ", unboundParams)));
-                }
+                        throw new InvalidOperationException(Resources.MissingParameters(string.Join(", ", unboundParams)));
+                    }
 
-                var timer = Stopwatch.StartNew();
-                while (raw.SQLITE_LOCKED == (rc = raw.sqlite3_step(stmt)) || rc == raw.SQLITE_BUSY)
-                {
-                    if (timer.ElapsedMilliseconds >= CommandTimeout * 1000)
+                    var timer = Stopwatch.StartNew();
+                    while (raw.SQLITE_LOCKED == (rc = raw.sqlite3_step(stmt)) || rc == raw.SQLITE_BUSY)
                     {
-                        break;
-                    }
+                        if (timer.ElapsedMilliseconds >= CommandTimeout * 1000)
+                        {
+                            break;
+                        }
 
-                    raw.sqlite3_reset(stmt);
+                        raw.sqlite3_reset(stmt);
 
-                    // TODO: Consider having an async path that uses Task.Delay()
-                    Thread.Sleep(150);
-                }
+                        // TODO: Consider having an async path that uses Task.Delay()
+                        Thread.Sleep(150);
+                    }
 
-                SqliteException.ThrowExceptionForRC(rc, _connection.Handle);
+                    SqliteException.ThrowExceptionForRC(rc, _connection.Handle);
 
-                if (rc == raw.SQLITE_ROW
-                    // NB: This is only a heuristic to separate SELECT statements from INSERT/UPDATE/DELETE statements.
-                    //     It will result in false positives, but it's the best we can do without re-parsing SQL
-                    || raw.sqlite3_stmt_readonly(stmt) != 0)
-                {
-                    stmts.Enqueue((stmt, rc != raw.SQLITE_DONE));
-                }
-                else
-                {
-                    hasChanges = true;
-                    changes += raw.sqlite3_changes(_connection.Handle);
+                    if (rc == raw.SQLITE_ROW
+                        // NB: This is only a heuristic to separate SELECT statements from INSERT/UPDATE/DELETE statements.
+                        //     It will result in false positives, but it's the best we can do without re-parsing SQL
+                        || raw.sqlite3_stmt_readonly(stmt) != 0)
+                    {
+                        stmts.Enqueue((stmt, rc != raw.SQLITE_DONE));
+                    }
+                    else
+                    {
+                        hasChanges = true;
+                        changes += raw.sqlite3_changes(_connection.Handle);
+                    }
                 }
             }
+            catch when (unprepared)
+            {
+                DisposePreparedStatements();
+
+                throw;
+            }
 
             var closeConnection = (behavior & CommandBehavior.CloseConnection) != 0;
 
-            return new SqliteDataReader(this, stmts, hasChanges ? changes : -1, closeConnection);
+            return DataReader = new SqliteDataReader(this, stmts, hasChanges ? changes : -1, closeConnection);
         }
 
         /// <summary>
@@ -481,7 +521,7 @@ private IEnumerable<sqlite3_stmt> PrepareAndEnumerateStatements()
                 var rc = raw.sqlite3_prepare_v2(
                     _connection.Handle,
                     tail,
-                    out sqlite3_stmt stmt,
+                    out var stmt,
                     out tail);
                 SqliteException.ThrowExceptionForRC(rc, _connection.Handle);
 
@@ -503,18 +543,14 @@ private IEnumerable<sqlite3_stmt> PrepareAndEnumerateStatements()
             while (!string.IsNullOrEmpty(tail));
         }
 
-        private IEnumerable<sqlite3_stmt> ResetAndEnumerateStatements()
+        private void DisposePreparedStatements(bool disposing = true)
         {
-            foreach (var stmt in _preparedStatements)
+            if (disposing && DataReader != null)
             {
-                raw.sqlite3_reset(stmt);
-
-                yield return stmt;
+                DataReader.Dispose();
+                DataReader = null;
             }
-        }
 
-        private void DisposePreparedStatements()
-        {
             foreach (var stmt in _preparedStatements)
             {
                 stmt.Dispose();

src/Microsoft.Data.Sqlite.Core/SqliteConnection.cs

@@ -284,7 +284,7 @@ internal void AddCommand(SqliteCommand command)
 
         internal void RemoveCommand(SqliteCommand command)
         {
-            for (int i = _commands.Count - 1; i >= 0; i--)
+            for (var i = _commands.Count - 1; i >= 0; i--)
             {
                 if (!_commands[i].TryGetTarget(out var item) || item == command)
                 {