Skip to content

Fixed non sequential consume for sequential read #3771

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
vonzshik merged 3 commits into from
May 23, 2021
Merged

Fixed non sequential consume for sequential read #3771

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8c8070c
Fixed non sequential consume for sequential read
vonzshik May 22, 2021
25ce422
Added a test
vonzshik May 22, 2021
85af53f
Removed unnecessary commentary
vonzshik May 22, 2021
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
16 changes: 5 additions & 11 deletions src/Npgsql/NpgsqlDataReader.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,15 +98,7 @@ public sealed class NpgsqlDataReader : DbDataReader, IDbColumnSchemaGenerator
/// Mostly useful for a sequential mode, when the row is already in the buffer.
/// Should always be true for the non-sequential mode.
/// </summary>
bool CanConsumeRowNonSequentially
{
get
{
var result = _dataMsgEnd - Buffer.ReadPosition <= Buffer.ReadBytesLeft;
Debug.Assert(result || _isSequential);
return result;
}
}
bool _canConsumeRowNonSequentially;

int _charPos;

Expand Down Expand Up @@ -225,7 +217,7 @@ public override Task<bool> ReadAsync(CancellationToken cancellationToken)
State = ReaderState.InResult;
return true;
case ReaderState.InResult:
if (!CanConsumeRowNonSequentially)
if (!_canConsumeRowNonSequentially)
return null;
// We get here, if we're in a non-sequential mode (or the row is already in the buffer)
ConsumeRowNonSequential();
Expand Down Expand Up @@ -729,9 +721,11 @@ void ProcessDataRowMessage(DataRowMessage msg)
$"Row's number of columns ({_numColumns}) differs from the row description's ({RowDescription.Count})");

_dataMsgEnd = Buffer.ReadPosition + msg.Length - 2;
_canConsumeRowNonSequentially = Buffer.ReadBytesLeft >= msg.Length - 2;

if (!_isSequential)
{
Debug.Assert(_canConsumeRowNonSequentially);
// Initialize our columns array with the offset and length of the first column
_columns.Clear();
var len = Buffer.ReadInt32();
Expand Down Expand Up @@ -2141,7 +2135,7 @@ Task ConsumeRow(bool async)

UniqueRowId++;

if (!CanConsumeRowNonSequentially)
if (!_canConsumeRowNonSequentially)
return ConsumeRowSequential(async);

// We get here, if we're in a non-sequential mode (or the row is already in the buffer)
Expand Down
50 changes: 50 additions & 0 deletions test/Npgsql.Tests/ReaderTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1196,6 +1196,56 @@ public async Task UnboundReaderReuse()
}
}

[Test, IssueLink("https://github.com/npgsql/npgsql/issues/3772")]
public async Task Bug3772()
{
if (!IsSequential)
return;

await using var postmasterMock = PgPostmasterMock.Start(ConnectionString);
using var _ = CreateTempPool(postmasterMock.ConnectionString, out var connectionString);
await using var conn = await OpenConnectionAsync(connectionString);

var pgMock = await postmasterMock.WaitForServerConnection();
pgMock
.WriteParseComplete()
.WriteBindComplete()
.WriteRowDescription(new FieldDescription(PostgresTypeOIDs.Int4), new FieldDescription(PostgresTypeOIDs.Bytea));

var intValue = new byte[] { 0, 0, 0, 1 };
var byteValue = new byte[] { 1, 2, 3, 4 };

var writeBuffer = pgMock.WriteBuffer;
writeBuffer.WriteByte((byte)BackendMessageCode.DataRow);
writeBuffer.WriteInt32(4 + 2 + intValue.Length + byteValue.Length + 8);
writeBuffer.WriteInt16(2);
writeBuffer.WriteInt32(intValue.Length);
writeBuffer.WriteBytes(intValue);
await pgMock.FlushAsync();

using var cmd = new NpgsqlCommand("SELECT some_int, some_byte FROM some_table", conn);
await using var reader = await cmd.ExecuteReaderAsync(Behavior);

await reader.ReadAsync();

reader.GetInt32(0);

Assert.Zero(reader.Connector.ReadBuffer.ReadBytesLeft);
Assert.NotZero(reader.Connector.ReadBuffer.ReadPosition);

writeBuffer.WriteInt32(byteValue.Length);
writeBuffer.WriteBytes(byteValue);
await pgMock
.WriteDataRow(intValue, Enumerable.Range(1, 100).Select(x => (byte)x).ToArray())
.WriteCommandComplete()
.WriteReadyForQuery()
.FlushAsync();

await reader.GetFieldValueAsync<byte[]>(1);

Assert.DoesNotThrowAsync(reader.ReadAsync);
}

[Test]
public async Task DisposeSwallowsExceptions([Values(true, false)] bool async)
{
Expand Down
3 changes: 2 additions & 1 deletion test/Npgsql.Tests/Support/PgServerMock.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ class PgServerMock : IDisposable
internal int ProcessId { get; }

internal NpgsqlReadBuffer ReadBuffer => _readBuffer;
internal NpgsqlWriteBuffer WriteBuffer => _writeBuffer;

internal PgServerMock(
NetworkStream stream,
Expand Down Expand Up @@ -249,7 +250,7 @@ internal async Task WriteDataRowWithFlush(params byte[][] columnValues)
{
CheckDisposed();

_writeBuffer.WriteByte((byte) BackendMessageCode.DataRow);
_writeBuffer.WriteByte((byte)BackendMessageCode.DataRow);
_writeBuffer.WriteInt32(4 + 2 + columnValues.Sum(v => 4 + v.Length));
_writeBuffer.WriteInt16(columnValues.Length);

Expand Down