Skip to content

Recover the buffer position when type handler throws, for sequential read #3656

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

Closed
Emill wants to merge 1 commit into from
Closed

Recover the buffer position when type handler throws, for sequential read #3656

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
7 changes: 7 additions & 0 deletions src/Npgsql/Internal/NpgsqlReadBuffer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,9 @@ internal TimeSpan Timeout
internal int ReadPosition { get; set; }
internal int ReadBytesLeft => FilledBytes - ReadPosition;

int _flushedBytes; // modulo 2^32
internal int CumulativeReadPosition => unchecked(_flushedBytes + ReadPosition);

internal readonly byte[] Buffer;
internal int FilledBytes;

Expand Down Expand Up @@ -161,6 +164,7 @@ static async Task EnsureLong(
{
Array.Copy(buffer.Buffer, buffer.ReadPosition, buffer.Buffer, 0, buffer.ReadBytesLeft);
buffer.FilledBytes = buffer.ReadBytesLeft;
buffer._flushedBytes = unchecked(buffer._flushedBytes + buffer.ReadPosition);
buffer.ReadPosition = 0;
}

Expand Down Expand Up @@ -494,6 +498,7 @@ public int Read(Span<byte> output)
var read = Underlying.Read(output);
if (read == 0)
throw new EndOfStreamException();
_flushedBytes = unchecked(_flushedBytes + read);
return read;
}
catch (Exception e)
Expand Down Expand Up @@ -526,6 +531,7 @@ static async ValueTask<int> ReadAsyncLong(NpgsqlReadBuffer buffer, Memory<byte>
var read = await buffer.Underlying.ReadAsync(output, cancellationToken);
if (read == 0)
throw new EndOfStreamException();
buffer._flushedBytes = unchecked(buffer._flushedBytes + read);
return read;
}
catch (Exception e)
Expand Down Expand Up @@ -637,6 +643,7 @@ public void Dispose()

internal void Clear()
{
_flushedBytes = unchecked(_flushedBytes + FilledBytes);
ReadPosition = 0;
FilledBytes = 0;
}
Expand Down
22 changes: 11 additions & 11 deletions src/Npgsql/NpgsqlDataReader.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1520,8 +1520,8 @@ public override T GetFieldValue<T>(int ordinal)
{
if (Connector.State != ConnectorState.Broken)
{
var writtenBytes = Buffer.ReadPosition - position;
var remainingBytes = ColumnLen - writtenBytes;
var readBytes = Buffer.ReadPosition - position;
var remainingBytes = ColumnLen - readBytes;
if (remainingBytes > 0)
Buffer.Skip(remainingBytes, false).GetAwaiter().GetResult();
}
Comment on lines 1520 to 1527
Copy link
Contributor Author

@Emill Emill Apr 11, 2021

Choose a reason for hiding this comment

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

Do we really need to catch and handle exception thrown inside type handlers for the non-sequential case, and make sure skip to end? Can't we leave it at any ReadPosition? When the next column or row is read, a new seek to column (or seek to row end if we move to the next row) is done anyway.

Expand Down Expand Up @@ -1554,7 +1554,7 @@ async ValueTask<T> GetFieldValueSequential<T>(int column, bool async, Cancellati
ThrowHelper.ThrowInvalidCastException_NoValue(field);
}

var position = Buffer.ReadPosition;
var position = Buffer.CumulativeReadPosition;
try
{
return NullableHandler<T>.Exists
Expand All @@ -1573,8 +1573,8 @@ async ValueTask<T> GetFieldValueSequential<T>(int column, bool async, Cancellati
{
if (Connector.State != ConnectorState.Broken)
{
var writtenBytes = Buffer.ReadPosition - position;
var remainingBytes = ColumnLen - writtenBytes;
var readBytes = unchecked(Buffer.CumulativeReadPosition - position);
var remainingBytes = ColumnLen - readBytes;
if (remainingBytes > 0)
await Buffer.Skip(remainingBytes, async);
}
Expand Down Expand Up @@ -1610,7 +1610,7 @@ public override object GetValue(int ordinal)
return DBNull.Value;

object result;
var position = Buffer.ReadPosition;
var position = Buffer.CumulativeReadPosition;
try
{
result = _isSequential
Expand All @@ -1621,8 +1621,8 @@ public override object GetValue(int ordinal)
{
if (Connector.State != ConnectorState.Broken)
{
var writtenBytes = Buffer.ReadPosition - position;
var remainingBytes = ColumnLen - writtenBytes;
var readBytes = unchecked(Buffer.CumulativeReadPosition - position);
var remainingBytes = ColumnLen - readBytes;
if (remainingBytes > 0)
Buffer.Skip(remainingBytes, false).GetAwaiter().GetResult();
}
Expand Down Expand Up @@ -1666,7 +1666,7 @@ public override object GetProviderSpecificValue(int ordinal)
if (ColumnLen == -1)
return DBNull.Value;

var position = Buffer.ReadPosition;
var position = Buffer.CumulativeReadPosition;
try
{
return _isSequential
Expand All @@ -1677,8 +1677,8 @@ public override object GetProviderSpecificValue(int ordinal)
{
if (Connector.State != ConnectorState.Broken)
{
var writtenBytes = Buffer.ReadPosition - position;
var remainingBytes = ColumnLen - writtenBytes;
var readBytes = unchecked(Buffer.CumulativeReadPosition - position);
var remainingBytes = ColumnLen - readBytes;
if (remainingBytes > 0)
Buffer.Skip(remainingBytes, false).GetAwaiter().GetResult();
}
Expand Down
12 changes: 12 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 @@ -1224,6 +1224,18 @@ await pgMock
Assert.DoesNotThrowAsync(async () => await reader.DisposeAsync());
}

[Test]
[IssueLink("https://github.com/npgsql/npgsql/issues/3648")]
public async Task RecoversFromExceptionInLongColumn()
{
using var conn = await OpenConnectionAsync();
using var cmd = new NpgsqlCommand(@"SELECT array_cat(array_fill(NULL::numeric, ARRAY[2048]), ARRAY['NaN'::numeric]), 9", conn);
using var reader = await cmd.ExecuteReaderAsync(Behavior);
await reader.ReadAsync();
Assert.Throws<InvalidCastException>(() => reader.GetFieldValue<decimal?[]>(0));
Assert.That(reader.GetInt32(1), Is.EqualTo(9));
}

#region GetBytes / GetStream

[Test]
Expand Down