From 47a23733c367e7d240f7fd0acabe4c8df9a1d0d4 Mon Sep 17 00:00:00 2001 From: Smit Patel Date: Thu, 27 Feb 2020 11:29:14 -0800 Subject: [PATCH 1/2] Revert "Clean up PR #18438 and add some tests" This reverts commit 5c5227befeb94d92ccf8c2a09d9055f637c332a8. --- .../SqliteDataReader.cs | 10 -- .../SqliteDataRecord.cs | 52 ++++---- .../SqliteDataReaderTest.cs | 121 ------------------ 3 files changed, 27 insertions(+), 156 deletions(-) diff --git a/src/Microsoft.Data.Sqlite.Core/SqliteDataReader.cs b/src/Microsoft.Data.Sqlite.Core/SqliteDataReader.cs index 93fe3834ddc..bd4fbc4a875 100644 --- a/src/Microsoft.Data.Sqlite.Core/SqliteDataReader.cs +++ b/src/Microsoft.Data.Sqlite.Core/SqliteDataReader.cs @@ -544,16 +544,6 @@ public override Stream GetStream(int ordinal) ? throw new InvalidOperationException(Resources.NoData) : _record.GetStream(ordinal); - /// - /// Retrieves data as a . - /// - /// The zero-based column ordinal. - /// The returned object. - public override TextReader GetTextReader(int ordinal) - => IsDBNull(ordinal) - ? (TextReader)new StringReader(string.Empty) - : new StreamReader(GetStream(ordinal), Encoding.UTF8); - /// /// Gets the value of the specified column. /// diff --git a/src/Microsoft.Data.Sqlite.Core/SqliteDataRecord.cs b/src/Microsoft.Data.Sqlite.Core/SqliteDataRecord.cs index 6bd6397e918..cec5e326d70 100644 --- a/src/Microsoft.Data.Sqlite.Core/SqliteDataRecord.cs +++ b/src/Microsoft.Data.Sqlite.Core/SqliteDataRecord.cs @@ -18,7 +18,7 @@ internal class SqliteDataRecord : SqliteValueReader, IDisposable private readonly byte[][] _blobCache; private readonly int?[] _typeCache; private bool _stepped; - private int? _rowidOrdinal; + private int? _rowOrdinalCache; public SqliteDataRecord(sqlite3_stmt stmt, bool hasRows, SqliteConnection connection) { @@ -27,6 +27,7 @@ public SqliteDataRecord(sqlite3_stmt stmt, bool hasRows, SqliteConnection connec _connection = connection; _blobCache = new byte[FieldCount][]; _typeCache = new int?[FieldCount]; + _rowOrdinalCache = null; } public virtual object this[string name] @@ -198,33 +199,32 @@ public static Type GetFieldType(string type) public virtual long GetBytes(int ordinal, long dataOffset, byte[] buffer, int bufferOffset, int length) { - using var stream = GetStream(ordinal); + var blob = GetStream(ordinal); - if (buffer == null) + long bytesToRead = blob.Length - dataOffset; + if (buffer != null) { - return stream.Length - dataOffset; + bytesToRead = Math.Min(bytesToRead, length); + using (var binaryReader = new BinaryReader(blob)) + { + Array.Copy(binaryReader.ReadBytes((int)blob.Length), dataOffset, buffer, bufferOffset, bytesToRead); + } } - stream.Position = dataOffset; - - return stream.Read(buffer, bufferOffset, length); + return bytesToRead; } public virtual long GetChars(int ordinal, long dataOffset, char[] buffer, int bufferOffset, int length) { - using var stream = GetStream(ordinal); - using var reader = new StreamReader(stream, Encoding.UTF8); + var textStream = GetStream(ordinal); + long charsToRead = textStream.Length - dataOffset; - for (var position = 0; position < dataOffset; position++) + using (var streamReader = new StreamReader(textStream, Encoding.UTF8)) { - if (reader.Read() == -1) - { - // NB: Message is provided by the framework - throw new ArgumentOutOfRangeException(nameof(dataOffset), dataOffset, message: null); - } + Array.Copy(streamReader.ReadToEnd().ToCharArray(), dataOffset, buffer, bufferOffset, Math.Min(charsToRead, length)); } - return reader.Read(buffer, bufferOffset, length); + return charsToRead; } public virtual Stream GetStream(int ordinal) @@ -238,10 +238,14 @@ public virtual Stream GetStream(int ordinal) var blobDatabaseName = sqlite3_column_database_name(Handle, ordinal).utf8_to_string(); var blobTableName = sqlite3_column_table_name(Handle, ordinal).utf8_to_string(); - if (!_rowidOrdinal.HasValue) - { - _rowidOrdinal = -1; + int rowidOrdinal = -1; + if (_rowOrdinalCache.HasValue) + { + rowidOrdinal = _rowOrdinalCache.Value; + } + else + { for (var i = 0; i < FieldCount; i++) { if (i == ordinal) @@ -264,7 +268,7 @@ public virtual Stream GetStream(int ordinal) var columnName = sqlite3_column_origin_name(Handle, i).utf8_to_string(); if (columnName == "rowid") { - _rowidOrdinal = i; + rowidOrdinal = i; break; } @@ -282,21 +286,19 @@ public virtual Stream GetStream(int ordinal) if (string.Equals(dataType, "INTEGER", StringComparison.OrdinalIgnoreCase) && primaryKey != 0) { - _rowidOrdinal = i; + _rowOrdinalCache = rowidOrdinal = i; break; } } - - Debug.Assert(_rowidOrdinal.HasValue); } - if (_rowidOrdinal.Value < 0) + if (rowidOrdinal < 0) { return new MemoryStream(GetCachedBlob(ordinal), false); } var blobColumnName = sqlite3_column_origin_name(Handle, ordinal).utf8_to_string(); - var rowid = GetInt32(_rowidOrdinal.Value); + var rowid = GetInt32(rowidOrdinal); return new SqliteBlob(_connection, blobTableName, blobColumnName, rowid, readOnly: true); } diff --git a/test/Microsoft.Data.Sqlite.Tests/SqliteDataReaderTest.cs b/test/Microsoft.Data.Sqlite.Tests/SqliteDataReaderTest.cs index e0078458eae..aca57006ddb 100644 --- a/test/Microsoft.Data.Sqlite.Tests/SqliteDataReaderTest.cs +++ b/test/Microsoft.Data.Sqlite.Tests/SqliteDataReaderTest.cs @@ -127,27 +127,6 @@ public void GetBytes_works() } } - [Fact] - public void GetBytes_works_streaming() - { - using (var connection = new SqliteConnection("Data Source=:memory:")) - { - connection.Open(); - - connection.ExecuteNonQuery("CREATE TABLE Data (Value); INSERT INTO Data VALUES (x'01020304');"); - - using (var reader = connection.ExecuteReader("SELECT rowid, Value FROM Data;")) - { - var hasData = reader.Read(); - Assert.True(hasData); - - var buffer = new byte[2]; - reader.GetBytes(1, 1, buffer, 0, buffer.Length); - Assert.Equal(new byte[] { 0x02, 0x03 }, buffer); - } - } - } - [Fact] public void GetBytes_NullBuffer() { @@ -273,26 +252,6 @@ public void GetChars_works_with_overflow() } } - [Fact] - public void GetChars_throws_when_dataOffset_out_of_range() - { - using (var connection = new SqliteConnection("Data Source=:memory:")) - { - connection.Open(); - - using (var reader = connection.ExecuteReader("SELECT 'test';")) - { - var hasData = reader.Read(); - Assert.True(hasData); - - var buffer = new char[1]; - var ex = Assert.Throws( - () => reader.GetChars(0, 5, buffer, 0, buffer.Length)); - Assert.Equal("dataOffset", ex.ParamName); - } - } - } - [Fact] public void GetChars_throws_when_closed() { @@ -303,27 +262,6 @@ public void GetChars_throws_when_closed() public void GetChars_throws_when_non_query() => X_throws_when_non_query(r => r.GetChars(0, 0, null, 0, 0)); - [Fact] - public void GetChars_works_streaming() - { - using (var connection = new SqliteConnection("Data Source=:memory:")) - { - connection.Open(); - - connection.ExecuteNonQuery("CREATE TABLE Data (Value); INSERT INTO Data VALUES ('test');"); - - using (var reader = connection.ExecuteReader("SELECT rowid, Value FROM Data;")) - { - var hasData = reader.Read(); - Assert.True(hasData); - - var buffer = new char[2]; - reader.GetChars(1, 1, buffer, 0, buffer.Length); - Assert.Equal(new[] { 'e', 's' }, buffer); - } - } - } - [Fact] public void GetStream_works() { @@ -450,65 +388,6 @@ public void GetStream_throws_when_closed() public void GetStream_throws_when_non_query() => X_throws_when_non_query(r => r.GetStream(0)); - [Fact] - public void GetTextReader_works() - { - using (var connection = new SqliteConnection("Data Source=:memory:")) - { - connection.Open(); - - using (var reader = connection.ExecuteReader("SELECT 'test';")) - { - var hasData = reader.Read(); - Assert.True(hasData); - - var textReader = reader.GetTextReader(0); - Assert.IsType(Assert.IsType(textReader).BaseStream); - Assert.Equal("test", textReader.ReadToEnd()); - } - } - } - - [Fact] - public void GetTextReader_works_when_null() - { - using (var connection = new SqliteConnection("Data Source=:memory:")) - { - connection.Open(); - - using (var reader = connection.ExecuteReader("SELECT NULL;")) - { - var hasData = reader.Read(); - Assert.True(hasData); - - var textReader = reader.GetTextReader(0); - Assert.IsType(textReader); - Assert.Empty(textReader.ReadToEnd()); - } - } - } - - [Fact] - public void GetTextReader_works_streaming() - { - using (var connection = new SqliteConnection("Data Source=:memory:")) - { - connection.Open(); - - connection.ExecuteNonQuery("CREATE TABLE Data (Value); INSERT INTO Data VALUES ('test');"); - - using (var reader = connection.ExecuteReader("SELECT rowid, Value FROM Data;")) - { - var hasData = reader.Read(); - Assert.True(hasData); - - var textReader = reader.GetTextReader(1); - Assert.IsType(Assert.IsType(textReader).BaseStream); - Assert.Equal("test", textReader.ReadToEnd()); - } - } - } - [Fact] public void GetDateTime_works_with_text() => GetX_works( From f2afe87835b3eb1ec26acf056990294b6ecc0f96 Mon Sep 17 00:00:00 2001 From: Smit Patel Date: Thu, 27 Feb 2020 11:29:24 -0800 Subject: [PATCH 2/2] Revert "Improve SqliteDataRecord class Efficiency" This reverts commit 84611fbedf4fb0d5d6a43661afca1ff7158245af. --- .../SqliteDataRecord.cs | 111 ++++++++---------- 1 file changed, 47 insertions(+), 64 deletions(-) diff --git a/src/Microsoft.Data.Sqlite.Core/SqliteDataRecord.cs b/src/Microsoft.Data.Sqlite.Core/SqliteDataRecord.cs index cec5e326d70..44d3121eb6b 100644 --- a/src/Microsoft.Data.Sqlite.Core/SqliteDataRecord.cs +++ b/src/Microsoft.Data.Sqlite.Core/SqliteDataRecord.cs @@ -5,7 +5,6 @@ using System.Diagnostics; using System.IO; using System.Linq; -using System.Text; using Microsoft.Data.Sqlite.Properties; using SQLitePCL; using static SQLitePCL.raw; @@ -18,7 +17,6 @@ internal class SqliteDataRecord : SqliteValueReader, IDisposable private readonly byte[][] _blobCache; private readonly int?[] _typeCache; private bool _stepped; - private int? _rowOrdinalCache; public SqliteDataRecord(sqlite3_stmt stmt, bool hasRows, SqliteConnection connection) { @@ -27,7 +25,6 @@ public SqliteDataRecord(sqlite3_stmt stmt, bool hasRows, SqliteConnection connec _connection = connection; _blobCache = new byte[FieldCount][]; _typeCache = new int?[FieldCount]; - _rowOrdinalCache = null; } public virtual object this[string name] @@ -199,16 +196,13 @@ public static Type GetFieldType(string type) public virtual long GetBytes(int ordinal, long dataOffset, byte[] buffer, int bufferOffset, int length) { - var blob = GetStream(ordinal); + var blob = GetCachedBlob(ordinal); long bytesToRead = blob.Length - dataOffset; if (buffer != null) { bytesToRead = Math.Min(bytesToRead, length); - using (var binaryReader = new BinaryReader(blob)) - { - Array.Copy(binaryReader.ReadBytes((int)blob.Length), dataOffset, buffer, bufferOffset, bytesToRead); - } + Array.Copy(blob, dataOffset, buffer, bufferOffset, bytesToRead); } return bytesToRead; @@ -216,14 +210,11 @@ public virtual long GetBytes(int ordinal, long dataOffset, byte[] buffer, int bu public virtual long GetChars(int ordinal, long dataOffset, char[] buffer, int bufferOffset, int length) { - var textStream = GetStream(ordinal); - long charsToRead = textStream.Length - dataOffset; - - using (var streamReader = new StreamReader(textStream, Encoding.UTF8)) - { - Array.Copy(streamReader.ReadToEnd().ToCharArray(), dataOffset, buffer, bufferOffset, Math.Min(charsToRead, length)); - } + var text = GetString(ordinal); + int charsToRead = text.Length - (int)dataOffset; + charsToRead = Math.Min(charsToRead, length); + text.CopyTo((int)dataOffset, buffer, bufferOffset, charsToRead); return charsToRead; } @@ -238,57 +229,49 @@ public virtual Stream GetStream(int ordinal) var blobDatabaseName = sqlite3_column_database_name(Handle, ordinal).utf8_to_string(); var blobTableName = sqlite3_column_table_name(Handle, ordinal).utf8_to_string(); - int rowidOrdinal = -1; - - if (_rowOrdinalCache.HasValue) - { - rowidOrdinal = _rowOrdinalCache.Value; - } - else + var rowidOrdinal = -1; + for (var i = 0; i < FieldCount; i++) { - for (var i = 0; i < FieldCount; i++) + if (i == ordinal) + { + continue; + } + + var databaseName = sqlite3_column_database_name(Handle, i).utf8_to_string(); + if (databaseName != blobDatabaseName) + { + continue; + } + + var tableName = sqlite3_column_table_name(Handle, i).utf8_to_string(); + if (tableName != blobTableName) + { + continue; + } + + var columnName = sqlite3_column_origin_name(Handle, i).utf8_to_string(); + if (columnName == "rowid") + { + rowidOrdinal = i; + break; + } + + var rc = sqlite3_table_column_metadata( + _connection.Handle, + databaseName, + tableName, + columnName, + out var dataType, + out var collSeq, + out var notNull, + out var primaryKey, + out var autoInc); + SqliteException.ThrowExceptionForRC(rc, _connection.Handle); + if (string.Equals(dataType, "INTEGER", StringComparison.OrdinalIgnoreCase) + && primaryKey != 0) { - if (i == ordinal) - { - continue; - } - - var databaseName = sqlite3_column_database_name(Handle, i).utf8_to_string(); - if (databaseName != blobDatabaseName) - { - continue; - } - - var tableName = sqlite3_column_table_name(Handle, i).utf8_to_string(); - if (tableName != blobTableName) - { - continue; - } - - var columnName = sqlite3_column_origin_name(Handle, i).utf8_to_string(); - if (columnName == "rowid") - { - rowidOrdinal = i; - break; - } - - var rc = sqlite3_table_column_metadata( - _connection.Handle, - databaseName, - tableName, - columnName, - out var dataType, - out var collSeq, - out var notNull, - out var primaryKey, - out var autoInc); - SqliteException.ThrowExceptionForRC(rc, _connection.Handle); - if (string.Equals(dataType, "INTEGER", StringComparison.OrdinalIgnoreCase) - && primaryKey != 0) - { - _rowOrdinalCache = rowidOrdinal = i; - break; - } + rowidOrdinal = i; + break; } }