diff --git a/src/Microsoft.SqlTools.ServiceLayer/QueryExecution/Batch.cs b/src/Microsoft.SqlTools.ServiceLayer/QueryExecution/Batch.cs index 4b81dfc4..38528b60 100644 --- a/src/Microsoft.SqlTools.ServiceLayer/QueryExecution/Batch.cs +++ b/src/Microsoft.SqlTools.ServiceLayer/QueryExecution/Batch.cs @@ -38,15 +38,31 @@ namespace Microsoft.SqlTools.ServiceLayer.QueryExecution /// public bool HasExecuted { get; set; } + /// + /// Internal representation of the messages so we can modify internally + /// + private List resultMessages; + /// /// Messages that have come back from the server /// - public List ResultMessages { get; set; } + public IEnumerable ResultMessages + { + get { return resultMessages; } + } + + /// + /// Internal representation of the result sets so we can modify internally + /// + private List resultSets; /// /// The result sets of the batch execution /// - public List ResultSets { get; set; } + public IEnumerable ResultSets + { + get { return resultSets; } + } /// /// Property for generating a set result set summaries from the result sets @@ -67,7 +83,7 @@ namespace Microsoft.SqlTools.ServiceLayer.QueryExecution /// /// The 0-indexed line number that this batch started on /// - private int StartLine { get; set; } + internal int StartLine { get; set; } #endregion @@ -81,10 +97,10 @@ namespace Microsoft.SqlTools.ServiceLayer.QueryExecution // Initialize the internal state BatchText = batchText; - StartLine = startLine - 1; + StartLine = startLine - 1; // -1 to make sure that the line number of the batch is 0-indexed, since SqlParser gives 1-indexed line numbers HasExecuted = false; - ResultSets = new List(); - ResultMessages = new List(); + resultSets = new List(); + resultMessages = new List(); } /// @@ -125,7 +141,7 @@ namespace Microsoft.SqlTools.ServiceLayer.QueryExecution if (!reader.HasRows && reader.FieldCount == 0) { // Create a message with the number of affected rows -- IF the query affects rows - ResultMessages.Add(reader.RecordsAffected >= 0 + resultMessages.Add(reader.RecordsAffected >= 0 ? string.Format(RowsAffectedFormat, reader.RecordsAffected) : "Command(s) completed successfully."); continue; @@ -145,10 +161,10 @@ namespace Microsoft.SqlTools.ServiceLayer.QueryExecution } // Add the result set to the results of the query - ResultSets.Add(resultSet); + resultSets.Add(resultSet); // Add a message for the number of rows the query returned - ResultMessages.Add(string.Format(RowsAffectedFormat, resultSet.Rows.Count)); + resultMessages.Add(string.Format(RowsAffectedFormat, resultSet.Rows.Count)); } while (await reader.NextResultAsync(cancellationToken)); } } @@ -187,14 +203,14 @@ namespace Microsoft.SqlTools.ServiceLayer.QueryExecution public ResultSetSubset GetSubset(int resultSetIndex, int startRow, int rowCount) { // Sanity check to make sure we have valid numbers - if (resultSetIndex < 0 || resultSetIndex >= ResultSets.Count) + if (resultSetIndex < 0 || resultSetIndex >= resultSets.Count) { throw new ArgumentOutOfRangeException(nameof(resultSetIndex), "Result set index cannot be less than 0" + "or greater than the number of result sets"); } // Retrieve the result set - return ResultSets[resultSetIndex].GetSubset(startRow, rowCount); + return resultSets[resultSetIndex].GetSubset(startRow, rowCount); } #region Private Helpers @@ -208,7 +224,7 @@ namespace Microsoft.SqlTools.ServiceLayer.QueryExecution /// Arguments from the event private void StoreDbMessage(object sender, SqlInfoMessageEventArgs args) { - ResultMessages.Add(args.Message); + resultMessages.Add(args.Message); } /// @@ -232,13 +248,13 @@ namespace Microsoft.SqlTools.ServiceLayer.QueryExecution string message = String.Format("Msg {0}, Level {1}, State {2}, Line {3}{4}{5}", sqlError.Number, sqlError.Class, sqlError.State, lineNumber, Environment.NewLine, sqlError.Message); - ResultMessages.Add(message); + resultMessages.Add(message); } } } else { - ResultMessages.Add(dbe.Message); + resultMessages.Add(dbe.Message); } } diff --git a/src/Microsoft.SqlTools.ServiceLayer/QueryExecution/Query.cs b/src/Microsoft.SqlTools.ServiceLayer/QueryExecution/Query.cs index add05b34..73d91e5e 100644 --- a/src/Microsoft.SqlTools.ServiceLayer/QueryExecution/Query.cs +++ b/src/Microsoft.SqlTools.ServiceLayer/QueryExecution/Query.cs @@ -60,6 +60,8 @@ namespace Microsoft.SqlTools.ServiceLayer.QueryExecution /// private ConnectionInfo EditorConnection { get; set; } + private bool HasExecuteBeenCalled { get; set; } + /// /// Whether or not the query has completed executed, regardless of success or failure /// @@ -68,9 +70,10 @@ namespace Microsoft.SqlTools.ServiceLayer.QueryExecution /// public bool HasExecuted { - get { return Batches.All(b => b.HasExecuted); } + get { return Batches.Length == 0 ? HasExecuteBeenCalled : Batches.All(b => b.HasExecuted); } internal set { + HasExecuteBeenCalled = value; foreach (var batch in Batches) { batch.HasExecuted = value; @@ -94,7 +97,7 @@ namespace Microsoft.SqlTools.ServiceLayer.QueryExecution public Query(string queryText, ConnectionInfo connection, QueryExecutionSettings settings) { // Sanity check for input - if (String.IsNullOrEmpty(queryText)) + if (string.IsNullOrEmpty(queryText)) { throw new ArgumentNullException(nameof(queryText), "Query text cannot be null"); } @@ -117,7 +120,9 @@ namespace Microsoft.SqlTools.ServiceLayer.QueryExecution { BatchSeparator = settings.BatchSeparator }); - Batches = parseResult.Script.Batches.Select(b => new Batch(b.Sql, b.StartLocation.LineNumber)).ToArray(); + // NOTE: We only want to process batches that have statements (ie, ignore comments and empty lines) + Batches = parseResult.Script.Batches.Where(b => b.Statements.Count > 0) + .Select(b => new Batch(b.Sql, b.StartLocation.LineNumber)).ToArray(); } /// @@ -125,6 +130,15 @@ namespace Microsoft.SqlTools.ServiceLayer.QueryExecution /// public async Task Execute() { + // Mark that we've internally executed + HasExecuteBeenCalled = true; + + // Don't actually execute if there aren't any batches to execute + if (Batches.Length == 0) + { + return; + } + // Open up a connection for querying the database string connectionString = ConnectionService.BuildConnectionString(EditorConnection.ConnectionDetails); using (DbConnection conn = EditorConnection.Factory.CreateSqlConnection(connectionString)) diff --git a/test/Microsoft.SqlTools.ServiceLayer.Test/QueryExecution/Common.cs b/test/Microsoft.SqlTools.ServiceLayer.Test/QueryExecution/Common.cs index aa72cb4e..6d265de2 100644 --- a/test/Microsoft.SqlTools.ServiceLayer.Test/QueryExecution/Common.cs +++ b/test/Microsoft.SqlTools.ServiceLayer.Test/QueryExecution/Common.cs @@ -26,6 +26,10 @@ namespace Microsoft.SqlTools.ServiceLayer.Test.QueryExecution { public const string StandardQuery = "SELECT * FROM sys.objects"; + public const string InvalidQuery = "SELECT *** FROM sys.objects"; + + public const string NoOpQuery = "-- No ops here, just us chickens."; + public const string OwnerUri = "testFile"; public const int StandardRows = 5; @@ -55,7 +59,7 @@ namespace Microsoft.SqlTools.ServiceLayer.Test.QueryExecution public static Batch GetBasicExecutedBatch() { - Batch batch = new Batch(StandardQuery); + Batch batch = new Batch(StandardQuery, 1); batch.Execute(CreateTestConnection(new[] {StandardTestData}, false), CancellationToken.None).Wait(); return batch; } diff --git a/test/Microsoft.SqlTools.ServiceLayer.Test/QueryExecution/ExecuteTests.cs b/test/Microsoft.SqlTools.ServiceLayer.Test/QueryExecution/ExecuteTests.cs index f4788014..ee98f8a2 100644 --- a/test/Microsoft.SqlTools.ServiceLayer.Test/QueryExecution/ExecuteTests.cs +++ b/test/Microsoft.SqlTools.ServiceLayer.Test/QueryExecution/ExecuteTests.cs @@ -28,7 +28,7 @@ namespace Microsoft.SqlTools.ServiceLayer.Test.QueryExecution public void BatchCreationTest() { // If I create a new batch... - Batch batch = new Batch(Common.StandardQuery); + Batch batch = new Batch(Common.StandardQuery, 1); // Then: // ... The text of the batch should be stored @@ -42,13 +42,16 @@ namespace Microsoft.SqlTools.ServiceLayer.Test.QueryExecution Assert.Empty(batch.ResultSets); Assert.Empty(batch.ResultSummaries); Assert.Empty(batch.ResultMessages); + + // ... The start line of the batch should be 0 + Assert.Equal(0, batch.StartLine); } [Fact] public void BatchExecuteNoResultSets() { // If I execute a query that should get no result sets - Batch batch = new Batch(Common.StandardQuery); + Batch batch = new Batch(Common.StandardQuery, 1); batch.Execute(GetConnection(Common.CreateTestConnectionInfo(null, false)), CancellationToken.None).Wait(); // Then: @@ -65,7 +68,7 @@ namespace Microsoft.SqlTools.ServiceLayer.Test.QueryExecution Assert.NotNull(batch.ResultSummaries); // ... There should be a message for how many rows were affected - Assert.Equal(1, batch.ResultMessages.Count); + Assert.Equal(1, batch.ResultMessages.Count()); } [Fact] @@ -75,7 +78,7 @@ namespace Microsoft.SqlTools.ServiceLayer.Test.QueryExecution ConnectionInfo ci = Common.CreateTestConnectionInfo(new[] { Common.StandardTestData }, false); // If I execute a query that should get one result set - Batch batch = new Batch(Common.StandardQuery); + Batch batch = new Batch(Common.StandardQuery, 1); batch.Execute(GetConnection(ci), CancellationToken.None).Wait(); // Then: @@ -84,20 +87,20 @@ namespace Microsoft.SqlTools.ServiceLayer.Test.QueryExecution Assert.False(batch.HasError, "The batch should not have an error"); // ... There should be exactly one result set - Assert.Equal(resultSets, batch.ResultSets.Count); + Assert.Equal(resultSets, batch.ResultSets.Count()); Assert.Equal(resultSets, batch.ResultSummaries.Length); // ... Inside the result set should be with 5 rows - Assert.Equal(Common.StandardRows, batch.ResultSets[0].Rows.Count); + Assert.Equal(Common.StandardRows, batch.ResultSets.First().Rows.Count); Assert.Equal(Common.StandardRows, batch.ResultSummaries[0].RowCount); // ... Inside the result set should have 5 columns and 5 column definitions - Assert.Equal(Common.StandardColumns, batch.ResultSets[0].Rows[0].Length); - Assert.Equal(Common.StandardColumns, batch.ResultSets[0].Columns.Length); + Assert.Equal(Common.StandardColumns, batch.ResultSets.First().Rows[0].Length); + Assert.Equal(Common.StandardColumns, batch.ResultSets.First().Columns.Length); Assert.Equal(Common.StandardColumns, batch.ResultSummaries[0].ColumnInfo.Length); // ... There should be a message for how many rows were affected - Assert.Equal(resultSets, batch.ResultMessages.Count); + Assert.Equal(resultSets, batch.ResultMessages.Count()); } [Fact] @@ -108,7 +111,7 @@ namespace Microsoft.SqlTools.ServiceLayer.Test.QueryExecution ConnectionInfo ci = Common.CreateTestConnectionInfo(dataset, false); // If I execute a query that should get two result sets - Batch batch = new Batch(Common.StandardQuery); + Batch batch = new Batch(Common.StandardQuery, 1); batch.Execute(GetConnection(ci), CancellationToken.None).Wait(); // Then: @@ -117,7 +120,7 @@ namespace Microsoft.SqlTools.ServiceLayer.Test.QueryExecution Assert.False(batch.HasError, "The batch should not have an error"); // ... There should be exactly two result sets - Assert.Equal(resultSets, batch.ResultSets.Count); + Assert.Equal(resultSets, batch.ResultSets.Count()); foreach (ResultSet rs in batch.ResultSets) { @@ -142,7 +145,7 @@ namespace Microsoft.SqlTools.ServiceLayer.Test.QueryExecution } // ... There should be a message for how many rows were affected - Assert.Equal(resultSets, batch.ResultMessages.Count); + Assert.Equal(resultSets, batch.ResultMessages.Count()); } [Fact] @@ -151,7 +154,7 @@ namespace Microsoft.SqlTools.ServiceLayer.Test.QueryExecution ConnectionInfo ci = Common.CreateTestConnectionInfo(null, true); // If I execute a batch that is invalid - Batch batch = new Batch(Common.StandardQuery); + Batch batch = new Batch(Common.StandardQuery, 1); batch.Execute(GetConnection(ci), CancellationToken.None).Wait(); // Then: @@ -173,7 +176,7 @@ namespace Microsoft.SqlTools.ServiceLayer.Test.QueryExecution ConnectionInfo ci = Common.CreateTestConnectionInfo(new[] { Common.StandardTestData }, false); // If I execute a batch - Batch batch = new Batch(Common.StandardQuery); + Batch batch = new Batch(Common.StandardQuery, 1); batch.Execute(GetConnection(ci), CancellationToken.None).Wait(); // Then: @@ -203,7 +206,7 @@ namespace Microsoft.SqlTools.ServiceLayer.Test.QueryExecution // ... I create a batch that has an empty query // Then: // ... It should throw an exception - Assert.Throws(() => new Batch(query)); + Assert.Throws(() => new Batch(query, 1)); } #endregion @@ -269,6 +272,31 @@ namespace Microsoft.SqlTools.ServiceLayer.Test.QueryExecution Assert.Equal(1, query.BatchSummaries.Length); } + [Fact] + public void QueryExecuteNoOpBatch() + { + // If: + // ... I create a query from a single batch that does nothing + ConnectionInfo ci = Common.CreateTestConnectionInfo(null, false); + Query query = new Query(Common.NoOpQuery, ci, new QueryExecutionSettings()); + + // Then: + // ... I should get no batches back + Assert.NotEmpty(query.QueryText); + Assert.Empty(query.Batches); + Assert.False(query.HasExecuted); + Assert.Throws(() => query.BatchSummaries); + + // If: + // ... I Then execute the query + query.Execute().Wait(); + + // Then: + // ... The query should have completed successfully with no batch summaries returned + Assert.True(query.HasExecuted); + Assert.Empty(query.BatchSummaries); + } + [Fact] public void QueryExecuteMultipleBatches() { @@ -297,13 +325,40 @@ namespace Microsoft.SqlTools.ServiceLayer.Test.QueryExecution Assert.Equal(2, query.BatchSummaries.Length); } + [Fact] + public void QueryExecuteMultipleBatchesWithNoOp() + { + // If: + // ... I create a query from a two batches (with separator) + ConnectionInfo ci = Common.CreateTestConnectionInfo(null, false); + string queryText = string.Format("{0}\r\nGO\r\n{1}", Common.StandardQuery, Common.NoOpQuery); + Query query = new Query(queryText, ci, new QueryExecutionSettings()); + + // Then: + // ... I should get back one batch to execute that hasn't been executed + Assert.NotEmpty(query.QueryText); + Assert.NotEmpty(query.Batches); + Assert.Equal(1, query.Batches.Length); + Assert.False(query.HasExecuted); + Assert.Throws(() => query.BatchSummaries); + + // If: + // .. I then execute the query + query.Execute().Wait(); + + // ... The query should have completed successfully with one batch summary returned + Assert.True(query.HasExecuted); + Assert.NotEmpty(query.BatchSummaries); + Assert.Equal(1, query.BatchSummaries.Length); + } + [Fact] public void QueryExecuteInvalidBatch() { // If: // ... I create a query from an invalid batch ConnectionInfo ci = Common.CreateTestConnectionInfo(null, true); - Query query = new Query("SELECT *** FROM sys.objects", ci, new QueryExecutionSettings()); + Query query = new Query(Common.InvalidQuery, ci, new QueryExecutionSettings()); // Then: // ... I should get back a query with one batch not executed diff --git a/test/Microsoft.SqlTools.ServiceLayer.Test/QueryExecution/SubsetTests.cs b/test/Microsoft.SqlTools.ServiceLayer.Test/QueryExecution/SubsetTests.cs index cf0d66a7..ad0f7075 100644 --- a/test/Microsoft.SqlTools.ServiceLayer.Test/QueryExecution/SubsetTests.cs +++ b/test/Microsoft.SqlTools.ServiceLayer.Test/QueryExecution/SubsetTests.cs @@ -143,7 +143,7 @@ namespace Microsoft.SqlTools.ServiceLayer.Test.QueryExecution var executeParams = new QueryExecuteParams { QueryText = "Doesn'tMatter", OwnerUri = Common.OwnerUri }; var executeRequest = Common.GetQueryExecuteResultContextMock(null, null, null); queryService.HandleExecuteRequest(executeParams, executeRequest.Object).Wait(); - //queryService.ActiveQueries[Common.OwnerUri].HasExecuted = false; + queryService.ActiveQueries[Common.OwnerUri].HasExecuted = false; // ... And I then ask for a valid set of results from it var subsetParams = new QueryExecuteSubsetParams { OwnerUri = Common.OwnerUri, RowsCount = 1, ResultSetIndex = 0, RowsStartIndex = 0 };