From 0f19de38fbb1b796959caa1091a8a20a4a9ec958 Mon Sep 17 00:00:00 2001 From: Benjamin Russell Date: Tue, 20 Dec 2016 11:22:00 -0800 Subject: [PATCH] Date & Time Type Fixes (#196) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This basically is a replacement to the fix for Adding Milliseconds to DateTime fields. I didn't take into consideration that `DATE` columns would report as DateTime type. Date columns have a numeric scale set to 255, leading to the formatting string for the date time to include 255 millisecond places, which is invalid. This change also reverses the change to store DateTime precision in the buffer file. Instead, the column metadata is now used when deserializing the rows from the db. `DATETIME` and `DATETIME2` columns are differentiated by their numeric scale while `DATE` columns are differentiated by their datatype name field. More unit tests were added. Additionally, this fixes an unreported bug that `DATE` columns were being displayed with times, which is incorrect. * Revert "Adding Milliseconds to DateTime fields (#173)" This reverts commit 431dfa41565412dc6ddb8cc243b286f668ef7f45. * Reworking the reader to use the column metadata for date types * DbColumn -> DbColumnWrapper * Fina tweaks to support DATETIME2(0) * Removing the unused arguments --- .../DataStorage/IFileStreamReader.cs | 17 --- .../ServiceBufferFileStreamReader.cs | 140 +++++++++++------- ...erviceBufferFileStreamReaderWriterTests.cs | 119 ++++++++++++++- .../Utility/TestDbColumn.cs | 12 ++ 4 files changed, 217 insertions(+), 71 deletions(-) diff --git a/src/Microsoft.SqlTools.ServiceLayer/QueryExecution/DataStorage/IFileStreamReader.cs b/src/Microsoft.SqlTools.ServiceLayer/QueryExecution/DataStorage/IFileStreamReader.cs index cfbe4fa1..38a609a7 100644 --- a/src/Microsoft.SqlTools.ServiceLayer/QueryExecution/DataStorage/IFileStreamReader.cs +++ b/src/Microsoft.SqlTools.ServiceLayer/QueryExecution/DataStorage/IFileStreamReader.cs @@ -15,22 +15,5 @@ namespace Microsoft.SqlTools.ServiceLayer.QueryExecution.DataStorage public interface IFileStreamReader : IDisposable { IList ReadRow(long offset, IEnumerable columns); - FileStreamReadResult ReadInt16(long i64Offset); - FileStreamReadResult ReadInt32(long i64Offset); - FileStreamReadResult ReadInt64(long i64Offset); - FileStreamReadResult ReadByte(long i64Offset); - FileStreamReadResult ReadChar(long i64Offset); - FileStreamReadResult ReadBoolean(long i64Offset); - FileStreamReadResult ReadSingle(long i64Offset); - FileStreamReadResult ReadDouble(long i64Offset); - FileStreamReadResult ReadSqlDecimal(long i64Offset); - FileStreamReadResult ReadDecimal(long i64Offset); - FileStreamReadResult ReadDateTime(long i64Offset); - FileStreamReadResult ReadTimeSpan(long i64Offset); - FileStreamReadResult ReadString(long i64Offset); - FileStreamReadResult ReadBytes(long i64Offset); - FileStreamReadResult ReadDateTimeOffset(long i64Offset); - FileStreamReadResult ReadGuid(long offset); - FileStreamReadResult ReadMoney(long offset); } } diff --git a/src/Microsoft.SqlTools.ServiceLayer/QueryExecution/DataStorage/ServiceBufferFileStreamReader.cs b/src/Microsoft.SqlTools.ServiceLayer/QueryExecution/DataStorage/ServiceBufferFileStreamReader.cs index 5df33596..e1e27635 100644 --- a/src/Microsoft.SqlTools.ServiceLayer/QueryExecution/DataStorage/ServiceBufferFileStreamReader.cs +++ b/src/Microsoft.SqlTools.ServiceLayer/QueryExecution/DataStorage/ServiceBufferFileStreamReader.cs @@ -17,7 +17,14 @@ namespace Microsoft.SqlTools.ServiceLayer.QueryExecution.DataStorage /// public class ServiceBufferFileStreamReader : IFileStreamReader { + + #region Constants + private const int DefaultBufferSize = 8192; + private const string DateFormatString = "yyyy-MM-dd"; + private const string TimeFormatString = "HH:mm:ss"; + + #endregion #region Member Variables @@ -25,7 +32,7 @@ namespace Microsoft.SqlTools.ServiceLayer.QueryExecution.DataStorage private readonly Stream fileStream; - private readonly Dictionary> readMethods; + private readonly Dictionary> readMethods; #endregion @@ -46,37 +53,37 @@ namespace Microsoft.SqlTools.ServiceLayer.QueryExecution.DataStorage buffer = new byte[DefaultBufferSize]; // Create the methods that will be used to read back - readMethods = new Dictionary> + readMethods = new Dictionary> { - {typeof(string), ReadString}, - {typeof(short), ReadInt16}, - {typeof(int), ReadInt32}, - {typeof(long), ReadInt64}, - {typeof(byte), ReadByte}, - {typeof(char), ReadChar}, - {typeof(bool), ReadBoolean}, - {typeof(double), ReadDouble}, - {typeof(float), ReadSingle}, - {typeof(decimal), ReadDecimal}, - {typeof(DateTime), ReadDateTime}, - {typeof(DateTimeOffset), ReadDateTimeOffset}, - {typeof(TimeSpan), ReadTimeSpan}, - {typeof(byte[]), ReadBytes}, + {typeof(string), (o, col) => ReadString(o)}, + {typeof(short), (o, col) => ReadInt16(o)}, + {typeof(int), (o, col) => ReadInt32(o)}, + {typeof(long), (o, col) => ReadInt64(o)}, + {typeof(byte), (o, col) => ReadByte(o)}, + {typeof(char), (o, col) => ReadChar(o)}, + {typeof(bool), (o, col) => ReadBoolean(o)}, + {typeof(double), (o, col) => ReadDouble(o)}, + {typeof(float), (o, col) => ReadSingle(o)}, + {typeof(decimal), (o, col) => ReadDecimal(o)}, + {typeof(DateTime), ReadDateTime}, + {typeof(DateTimeOffset), (o, col) => ReadDateTimeOffset(o)}, + {typeof(TimeSpan), (o, col) => ReadTimeSpan(o)}, + {typeof(byte[]), (o, col) => ReadBytes(o)}, - {typeof(SqlString), ReadString}, - {typeof(SqlInt16), ReadInt16}, - {typeof(SqlInt32), ReadInt32}, - {typeof(SqlInt64), ReadInt64}, - {typeof(SqlByte), ReadByte}, - {typeof(SqlBoolean), ReadBoolean}, - {typeof(SqlDouble), ReadDouble}, - {typeof(SqlSingle), ReadSingle}, - {typeof(SqlDecimal), ReadSqlDecimal}, - {typeof(SqlDateTime), ReadDateTime}, - {typeof(SqlBytes), ReadBytes}, - {typeof(SqlBinary), ReadBytes}, - {typeof(SqlGuid), ReadGuid}, - {typeof(SqlMoney), ReadMoney}, + {typeof(SqlString), (o, col) => ReadString(o)}, + {typeof(SqlInt16), (o, col) => ReadInt16(o)}, + {typeof(SqlInt32), (o, col) => ReadInt32(o)}, + {typeof(SqlInt64), (o, col) => ReadInt64(o)}, + {typeof(SqlByte), (o, col) => ReadByte(o)}, + {typeof(SqlBoolean), (o, col) => ReadBoolean(o)}, + {typeof(SqlDouble), (o, col) => ReadDouble(o)}, + {typeof(SqlSingle), (o, col) => ReadSingle(o)}, + {typeof(SqlDecimal), (o, col) => ReadSqlDecimal(o)}, + {typeof(SqlDateTime), ReadDateTime}, + {typeof(SqlBytes), (o, col) => ReadBytes(o)}, + {typeof(SqlBinary), (o, col) => ReadBytes(o)}, + {typeof(SqlGuid), (o, col) => ReadGuid(o)}, + {typeof(SqlMoney), (o, col) => ReadMoney(o)}, }; } @@ -129,13 +136,13 @@ namespace Microsoft.SqlTools.ServiceLayer.QueryExecution.DataStorage } // Use the right read function for the type to read the data from the file - Func readFunc; + Func readFunc; if(!readMethods.TryGetValue(colType, out readFunc)) { // Treat everything else as a string - readFunc = ReadString; + readFunc = readMethods[typeof(string)]; } - FileStreamReadResult result = readFunc(currentFileOffset); + FileStreamReadResult result = readFunc(currentFileOffset, column); currentFileOffset += result.TotalLength; results.Add(result.Value); } @@ -148,7 +155,7 @@ namespace Microsoft.SqlTools.ServiceLayer.QueryExecution.DataStorage /// /// Offset into the file to read the short from /// A short - public FileStreamReadResult ReadInt16(long fileOffset) + internal FileStreamReadResult ReadInt16(long fileOffset) { return ReadCellHelper(fileOffset, length => BitConverter.ToInt16(buffer, 0)); } @@ -158,7 +165,7 @@ namespace Microsoft.SqlTools.ServiceLayer.QueryExecution.DataStorage /// /// Offset into the file to read the int from /// An int - public FileStreamReadResult ReadInt32(long fileOffset) + internal FileStreamReadResult ReadInt32(long fileOffset) { return ReadCellHelper(fileOffset, length => BitConverter.ToInt32(buffer, 0)); } @@ -168,7 +175,7 @@ namespace Microsoft.SqlTools.ServiceLayer.QueryExecution.DataStorage /// /// Offset into the file to read the long from /// A long - public FileStreamReadResult ReadInt64(long fileOffset) + internal FileStreamReadResult ReadInt64(long fileOffset) { return ReadCellHelper(fileOffset, length => BitConverter.ToInt64(buffer, 0)); } @@ -178,7 +185,7 @@ namespace Microsoft.SqlTools.ServiceLayer.QueryExecution.DataStorage /// /// Offset into the file to read the byte from /// A byte - public FileStreamReadResult ReadByte(long fileOffset) + internal FileStreamReadResult ReadByte(long fileOffset) { return ReadCellHelper(fileOffset, length => buffer[0]); } @@ -188,7 +195,7 @@ namespace Microsoft.SqlTools.ServiceLayer.QueryExecution.DataStorage /// /// Offset into the file to read the char from /// A char - public FileStreamReadResult ReadChar(long fileOffset) + internal FileStreamReadResult ReadChar(long fileOffset) { return ReadCellHelper(fileOffset, length => BitConverter.ToChar(buffer, 0)); } @@ -198,7 +205,7 @@ namespace Microsoft.SqlTools.ServiceLayer.QueryExecution.DataStorage /// /// Offset into the file to read the bool from /// A bool - public FileStreamReadResult ReadBoolean(long fileOffset) + internal FileStreamReadResult ReadBoolean(long fileOffset) { return ReadCellHelper(fileOffset, length => buffer[0] == 0x1); } @@ -208,7 +215,7 @@ namespace Microsoft.SqlTools.ServiceLayer.QueryExecution.DataStorage /// /// Offset into the file to read the single from /// A single - public FileStreamReadResult ReadSingle(long fileOffset) + internal FileStreamReadResult ReadSingle(long fileOffset) { return ReadCellHelper(fileOffset, length => BitConverter.ToSingle(buffer, 0)); } @@ -218,7 +225,7 @@ namespace Microsoft.SqlTools.ServiceLayer.QueryExecution.DataStorage /// /// Offset into the file to read the double from /// A double - public FileStreamReadResult ReadDouble(long fileOffset) + internal FileStreamReadResult ReadDouble(long fileOffset) { return ReadCellHelper(fileOffset, length => BitConverter.ToDouble(buffer, 0)); } @@ -228,7 +235,7 @@ namespace Microsoft.SqlTools.ServiceLayer.QueryExecution.DataStorage /// /// Offset into the file to read the SqlDecimal from /// A SqlDecimal - public FileStreamReadResult ReadSqlDecimal(long offset) + internal FileStreamReadResult ReadSqlDecimal(long offset) { return ReadCellHelper(offset, length => { @@ -243,7 +250,7 @@ namespace Microsoft.SqlTools.ServiceLayer.QueryExecution.DataStorage /// /// Offset into the file to read the decimal from /// A decimal - public FileStreamReadResult ReadDecimal(long offset) + internal FileStreamReadResult ReadDecimal(long offset) { return ReadCellHelper(offset, length => { @@ -257,13 +264,44 @@ namespace Microsoft.SqlTools.ServiceLayer.QueryExecution.DataStorage /// Reads a DateTime from the file at the offset provided /// /// Offset into the file to read the DateTime from + /// Column metadata, used for determining what precision to output /// A DateTime - public FileStreamReadResult ReadDateTime(long offset) + internal FileStreamReadResult ReadDateTime(long offset, DbColumnWrapper col) { return ReadCellHelper(offset, length => { long ticks = BitConverter.ToInt64(buffer, 0); return new DateTime(ticks); + }, null, dt => + { + // Switch based on the type of column + string formatString; + if (col.DataTypeName.Equals("DATE", StringComparison.OrdinalIgnoreCase)) + { + // DATE columns should only show the date + formatString = DateFormatString; + } + else if (col.DataTypeName.StartsWith("DATETIME", StringComparison.OrdinalIgnoreCase)) + { + // DATETIME and DATETIME2 columns should show date, time, and a variable number + // of milliseconds (for DATETIME, it is fixed at 3, but returned as null) + // If for some strange reason a scale > 7 is sent, we will cap it at 7 to avoid + // an exception from invalid date/time formatting + int scale = Math.Min(col.NumericScale ?? 3, 7); + formatString = $"{DateFormatString} {TimeFormatString}"; + if (scale > 0) + { + string millisecondString = new string('f', scale); + formatString += $".{millisecondString}"; + } + } + else + { + // For anything else that returns as a CLR DateTime, just show date and time + formatString = $"{DateFormatString} {TimeFormatString}"; + } + + return dt.ToString(formatString); }); } @@ -272,7 +310,7 @@ namespace Microsoft.SqlTools.ServiceLayer.QueryExecution.DataStorage /// /// Offset into the file to read the DateTimeOffset from /// A DateTimeOffset - public FileStreamReadResult ReadDateTimeOffset(long offset) + internal FileStreamReadResult ReadDateTimeOffset(long offset) { // DateTimeOffset is represented by DateTime.Ticks followed by TimeSpan.Ticks // both as Int64 values @@ -288,7 +326,7 @@ namespace Microsoft.SqlTools.ServiceLayer.QueryExecution.DataStorage /// /// Offset into the file to read the TimeSpan from /// A TimeSpan - public FileStreamReadResult ReadTimeSpan(long offset) + internal FileStreamReadResult ReadTimeSpan(long offset) { return ReadCellHelper(offset, length => { @@ -302,7 +340,7 @@ namespace Microsoft.SqlTools.ServiceLayer.QueryExecution.DataStorage /// /// Offset into the file to read the string from /// A string - public FileStreamReadResult ReadString(long offset) + internal FileStreamReadResult ReadString(long offset) { return ReadCellHelper(offset, length => length > 0 @@ -315,7 +353,7 @@ namespace Microsoft.SqlTools.ServiceLayer.QueryExecution.DataStorage /// /// Offset into the file to read the bytes from /// A byte array - public FileStreamReadResult ReadBytes(long offset) + internal FileStreamReadResult ReadBytes(long offset) { return ReadCellHelper(offset, length => { @@ -339,7 +377,7 @@ namespace Microsoft.SqlTools.ServiceLayer.QueryExecution.DataStorage /// /// Offset into the file to read the bytes from /// A guid type object - public FileStreamReadResult ReadGuid(long offset) + internal FileStreamReadResult ReadGuid(long offset) { return ReadCellHelper(offset, length => { @@ -353,9 +391,9 @@ namespace Microsoft.SqlTools.ServiceLayer.QueryExecution.DataStorage /// Reads a SqlMoney type from the offset provided /// into a /// - /// + /// Offset into the file to read the value /// A sql money type object - public FileStreamReadResult ReadMoney(long offset) + internal FileStreamReadResult ReadMoney(long offset) { return ReadCellHelper(offset, length => { diff --git a/test/Microsoft.SqlTools.ServiceLayer.Test/QueryExecution/DataStorage/ServiceBufferFileStreamReaderWriterTests.cs b/test/Microsoft.SqlTools.ServiceLayer.Test/QueryExecution/DataStorage/ServiceBufferFileStreamReaderWriterTests.cs index b454eafb..150e6281 100644 --- a/test/Microsoft.SqlTools.ServiceLayer.Test/QueryExecution/DataStorage/ServiceBufferFileStreamReaderWriterTests.cs +++ b/test/Microsoft.SqlTools.ServiceLayer.Test/QueryExecution/DataStorage/ServiceBufferFileStreamReaderWriterTests.cs @@ -6,9 +6,13 @@ using System; using System.Collections.Generic; using System.Data.SqlTypes; +using System.Diagnostics.CodeAnalysis; using System.IO; using System.Text; +using System.Text.RegularExpressions; +using Microsoft.SqlTools.ServiceLayer.QueryExecution.Contracts; using Microsoft.SqlTools.ServiceLayer.QueryExecution.DataStorage; +using Microsoft.SqlTools.ServiceLayer.Test.Utility; using Moq; using Xunit; @@ -76,7 +80,8 @@ namespace Microsoft.SqlTools.ServiceLayer.Test.QueryExecution.DataStorage }); } - private static void VerifyReadWrite(int valueLength, T value, Func writeFunc, Func readFunc) + [SuppressMessage("ReSharper", "UnusedParameter.Local")] + private static string VerifyReadWrite(int valueLength, T value, Func writeFunc, Func readFunc) { // Setup: Create a mock file stream byte[] storage = new byte[8192]; @@ -100,6 +105,8 @@ namespace Microsoft.SqlTools.ServiceLayer.Test.QueryExecution.DataStorage Assert.Equal(value, outValue.Value.RawObject); Assert.Equal(valueLength, outValue.TotalLength); Assert.NotNull(outValue.Value); + + return outValue.Value.DisplayValue; } [Theory] @@ -230,7 +237,7 @@ namespace Microsoft.SqlTools.ServiceLayer.Test.QueryExecution.DataStorage } [Fact] - public void DateTimeTest() + public void DateTest() { // Setup: Create some test values // NOTE: We are doing these here instead of InlineData because DateTime values can't be written as constant expressions @@ -238,9 +245,115 @@ namespace Microsoft.SqlTools.ServiceLayer.Test.QueryExecution.DataStorage { DateTime.Now, DateTime.UtcNow, DateTime.MinValue, DateTime.MaxValue }; + + // Setup: Create a DATE column + DbColumnWrapper col = new DbColumnWrapper(new TestDbColumn("col", "DaTe")); + foreach (DateTime value in testValues) { - VerifyReadWrite(sizeof(long) + 1, value, (writer, val) => writer.WriteDateTime(val), reader => reader.ReadDateTime(0)); + string displayValue = VerifyReadWrite(sizeof(long) + 1, value, (writer, val) => writer.WriteDateTime(val), reader => reader.ReadDateTime(0, col)); + + // Make sure the display value does not have a time string + Assert.True(Regex.IsMatch(displayValue, @"^[\d]{4}-[\d]{2}-[\d]{2}$")); + } + } + + [Fact] + public void DateTimeTest() + { + // Setup: Create some test values + // NOTE: We are doing these here instead of InlineData because DateTime values can't be written as constant expressions + DateTime[] testValues = + { + DateTime.Now, DateTime.UtcNow, DateTime.MinValue, DateTime.MaxValue + }; + + // Setup: Create a DATETIME column + DbColumnWrapper col = new DbColumnWrapper(new TestDbColumn("col", "DaTeTiMe")); + + foreach (DateTime value in testValues) + { + string displayValue = VerifyReadWrite(sizeof(long) + 1, value, (writer, val) => writer.WriteDateTime(val), reader => reader.ReadDateTime(0, col)); + + // Make sure the display value has a time string with 3 milliseconds + Assert.True(Regex.IsMatch(displayValue, @"^[\d]{4}-[\d]{2}-[\d]{2} [\d]{2}:[\d]{2}:[\d]{2}\.[\d]{3}$")); + } + } + + [Theory] + [InlineData(1)] + [InlineData(2)] + [InlineData(3)] + [InlineData(4)] + [InlineData(5)] + [InlineData(6)] + [InlineData(7)] + public void DateTime2Test(int precision) + { + // Setup: Create some test values + // NOTE: We are doing these here instead of InlineData because DateTime values can't be written as constant expressions + DateTime[] testValues = + { + DateTime.Now, DateTime.UtcNow, DateTime.MinValue, DateTime.MaxValue + }; + + // Setup: Create a DATETIME column + DbColumnWrapper col = new DbColumnWrapper(new TestDbColumn("col", "DaTeTiMe2", precision)); + + foreach (DateTime value in testValues) + { + string displayValue = VerifyReadWrite(sizeof(long) + 1, value, (writer, val) => writer.WriteDateTime(val), reader => reader.ReadDateTime(0, col)); + + // Make sure the display value has a time string with variable number of milliseconds + Assert.True(Regex.IsMatch(displayValue, @"^[\d]{4}-[\d]{2}-[\d]{2} [\d]{2}:[\d]{2}:[\d]{2}")); + if (precision > 0) + { + Assert.True(Regex.IsMatch(displayValue, $@"\.[\d]{{{precision}}}$")); + } + } + } + + [Fact] + public void DateTime2ZeroScaleTest() + { + // Setup: Create some test values + // NOTE: We are doing these here instead of InlineData because DateTime values can't be written as constant expressions + DateTime[] testValues = + { + DateTime.Now, DateTime.UtcNow, DateTime.MinValue, DateTime.MaxValue + }; + + // Setup: Create a DATETIME2 column + DbColumnWrapper col = new DbColumnWrapper(new TestDbColumn("col", "DaTeTiMe2", 0)); + + foreach (DateTime value in testValues) + { + string displayValue = VerifyReadWrite(sizeof(long) + 1, value, (writer, val) => writer.WriteDateTime(val), reader => reader.ReadDateTime(0, col)); + + // Make sure the display value has a time string with 0 milliseconds + Assert.True(Regex.IsMatch(displayValue, @"^[\d]{4}-[\d]{2}-[\d]{2} [\d]{2}:[\d]{2}:[\d]{2}$")); + } + } + + [Fact] + public void DateTime2InvalidScaleTest() + { + // Setup: Create some test values + // NOTE: We are doing these here instead of InlineData because DateTime values can't be written as constant expressions + DateTime[] testValues = + { + DateTime.Now, DateTime.UtcNow, DateTime.MinValue, DateTime.MaxValue + }; + + // Setup: Create a DATETIME2 column + DbColumnWrapper col = new DbColumnWrapper(new TestDbColumn("col", "DaTeTiMe2", 255)); + + foreach (DateTime value in testValues) + { + string displayValue = VerifyReadWrite(sizeof(long) + 1, value, (writer, val) => writer.WriteDateTime(val), reader => reader.ReadDateTime(0, col)); + + // Make sure the display value has a time string with 7 milliseconds + Assert.True(Regex.IsMatch(displayValue, @"^[\d]{4}-[\d]{2}-[\d]{2} [\d]{2}:[\d]{2}:[\d]{2}\.[\d]{7}$")); } } diff --git a/test/Microsoft.SqlTools.ServiceLayer.Test/Utility/TestDbColumn.cs b/test/Microsoft.SqlTools.ServiceLayer.Test/Utility/TestDbColumn.cs index b8b93276..0d916f75 100644 --- a/test/Microsoft.SqlTools.ServiceLayer.Test/Utility/TestDbColumn.cs +++ b/test/Microsoft.SqlTools.ServiceLayer.Test/Utility/TestDbColumn.cs @@ -17,5 +17,17 @@ namespace Microsoft.SqlTools.ServiceLayer.Test.Utility base.DataType = typeof(string); base.DataTypeName = "nvarchar"; } + + public TestDbColumn(string columnName, string columnType) + : this(columnName) + { + base.DataTypeName = columnType; + } + + public TestDbColumn(string columnName, string columnType, int scale) + : this(columnName, columnType) + { + base.NumericScale = scale; + } } }