Skip to content

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

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.

Sign up for GitHub

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

Merged
ajcvickers merged 4 commits into from
Jan 27, 2024
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a6e07b7
blob fix in case of join with multiple tables and related rowid
francopettinari Jan 8, 2024
bd3301a
Merge branch 'dotnet:main' into main
francopettinari Jan 10, 2024
ed6ac41
test case for issue #32770
francopettinari Jan 16, 2024
f08c4cf
assert readded
francopettinari Jan 22, 2024
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
View file
Open in desktop
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
Contributor

@ajcvickers ajcvickers Jan 16, 2024

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

@francopettinari francopettinari Jan 16, 2024

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);
}

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
View file
Open in desktop
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