Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Microsoft.Data.Sqlite.Core issue with multiple Blob colums #32770

Merged
merged 4 commits into from
Jan 27, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 25 additions & 9 deletions src/Microsoft.Data.Sqlite.Core/SqliteDataRecord.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,16 +14,30 @@

namespace Microsoft.Data.Sqlite
{

internal class SqliteDataRecord : SqliteValueReader, IDisposable
{
internal class RowIdInfo
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason not to use a value tuple for this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was only for readability

{
public int Ordinal { get; set; }
public string TableName { get; set; }

public RowIdInfo(int ordinal, string tableName)
{
Ordinal = ordinal;
TableName = tableName;
}
}

private readonly SqliteConnection _connection;
private readonly Action<int> _addChanges;
private byte[][]? _blobCache;
private int?[]? _typeCache;
private Dictionary<string, int>? _columnNameOrdinalCache;
private string[]? _columnNameCache;
private bool _stepped;
private int? _rowidOrdinal;
readonly Dictionary<string, RowIdInfo> RowIds = new Dictionary<string, RowIdInfo>();

private bool _alreadyThrown;
private bool _alreadyAddedChanges;

Expand Down Expand Up @@ -310,11 +324,11 @@ 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)
RowIdInfo? rowIdForOrdinal = null;
string rowidkey = $"{blobDatabaseName}_{blobTableName}";
if (!RowIds.TryGetValue(rowidkey, out rowIdForOrdinal))
{
_rowidOrdinal = -1;
var pkColumns = -1L;

for (var i = 0; i < FieldCount; i++)
{
if (i == ordinal)
Expand All @@ -337,7 +351,8 @@ public virtual Stream GetStream(int ordinal)
var columnName = sqlite3_column_origin_name(Handle, i).utf8_to_string();
if (columnName == "rowid")
{
_rowidOrdinal = i;
rowIdForOrdinal = new RowIdInfo(i, tableName);
RowIds.Add(rowidkey, rowIdForOrdinal);
break;
}

Expand Down Expand Up @@ -368,22 +383,23 @@ public virtual Stream GetStream(int ordinal)

if (pkColumns == 1L)
{
_rowidOrdinal = i;
rowIdForOrdinal = new RowIdInfo(i, tableName);
RowIds.Add(rowidkey, rowIdForOrdinal);
break;
}
}
}

Debug.Assert(_rowidOrdinal.HasValue);
//Debug.Assert(rowIdForOrdinal!=null);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-add Assert

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Puppy error, sorry. Commented for initial checks and later committed. Not intentional. Sorry again!
I readded, relaunched the tests and finally committed.

}

if (_rowidOrdinal.Value < 0)
if (rowIdForOrdinal == null)
{
return new MemoryStream(GetCachedBlob(ordinal), false);
}

var blobColumnName = sqlite3_column_origin_name(Handle, ordinal).utf8_to_string();
var rowid = GetInt64(_rowidOrdinal.Value);
var rowid = GetInt64(rowIdForOrdinal.Ordinal);

return new SqliteBlob(_connection, blobDatabaseName, blobTableName, blobColumnName, rowid, readOnly: true);
}
Expand Down
38 changes: 38 additions & 0 deletions test/Microsoft.Data.Sqlite.Tests/SqliteDataReaderTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,44 @@ public void GetBytes_works_streaming()
}
}

[Fact]
public void GetBytes_works_streaming_join()
{
using (var connection = new SqliteConnection("Data Source=:memory:"))
{
connection.Open();

connection.ExecuteNonQuery("CREATE TABLE A (ID INTEGER PRIMARY KEY,VALUE BLOB); INSERT INTO A (ID, VALUE) VALUES (1,x'01020304');");
connection.ExecuteNonQuery("CREATE TABLE B (ID INTEGER PRIMARY KEY,FATHER_ID INTEGER NOT NULL,VALUE BLOB); INSERT INTO B (ID,FATHER_ID, VALUE) VALUES (1000,1,x'05060708');");

using (var reader = connection.ExecuteReader(@"SELECT
A.ID as AID,
A.VALUE as AVALUE,
B.ID as BID,
B.VALUE as BVALUE
FROM
A JOIN B
ON B.FATHER_ID=A.ID "))
{
var hasData = reader.Read();
Assert.True(hasData);

//reading fields that does not involve blobs should be ok
Console.WriteLine($"A.ID={reader.GetInt32(0)} B.ID={reader.GetInt32(2)}");

//get len of abuff
var abuff = new byte[2];
reader.GetBytes(1, 1, abuff, 0, abuff.Length);
Assert.Equal([0x02, 0x03], abuff);

var bbuff = new byte[2];
reader.GetBytes(3, 1, bbuff, 0, bbuff.Length); //this was failing. now should be fixed
Assert.Equal([0x06, 0x07], bbuff);

}
}
}

[Fact]
public void GetBytes_NullBuffer()
{
Expand Down