Skip to content

Commit d5754c8

Browse files
rojiroji
committed
Bad hack to reallow output params with sequential
Npgsql 4.0 accidentally removed support for output parameters when using CommandBehavior.Sequential, but unfortunately DbDataAdapter relies on that. 4.0's refactoring into two reader classes (sequential, non-sequential) made this support very hacky, as the 1st row needs to be traversed twice, once for output param population and once as a regular row. We definitely need to clean this up in 4.1. Fixes npgsql#2091
1 parent 8a1f766 commit d5754c8

File tree

5 files changed

+70
-62
lines changed

5 files changed

+70
-62
lines changed

‎src/Npgsql/NpgsqlCommand.cs‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1137,8 +1137,6 @@ async ValueTask<DbDataReader> ExecuteDbDataReader(CommandBehavior behavior, bool
11371137
using (cancellationToken.Register(cmd => ((NpgsqlCommand)cmd).Cancel(), this))
11381138
{
11391139
ValidateParameters();
1140-
if ((behavior & CommandBehavior.SequentialAccess) != 0 && Parameters.HasOutputParameters)
1141-
throw new NotSupportedException("Output parameters aren't supported with SequentialAccess");
11421140

11431141
if (IsExplicitlyPrepared)
11441142
{

‎src/Npgsql/NpgsqlDataReader.cs‎

Lines changed: 64 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -367,7 +367,7 @@ public override Task<bool> NextResultAsync(CancellationToken cancellationToken)
367367
/// Internal implementation of NextResult
368368
/// </summary>
369369
[MethodImpl(MethodImplOptions.AggressiveInlining)]
370-
protected virtual async Task<bool> NextResult(bool async, bool isConsuming=false)
370+
async Task<bool> NextResult(bool async, bool isConsuming=false)
371371
{
372372
IBackendMessage msg;
373373
Debug.Assert(!IsSchemaOnly);
@@ -464,7 +464,30 @@ protected virtual async Task<bool> NextResult(bool async, bool isConsuming=false
464464
}
465465
}
466466

467-
msg = await ReadMessage(async);
467+
// The following is a pretty awful hack to bring back output parameters for sequential readers (#2091)
468+
// We should definitely clean up the entire reader design (#1853)
469+
if (StatementIndex == 0 && RowDescription != null && Command.Parameters.HasOutputParameters)
470+
{
471+
// If output parameters are present and this is the first row of the first resultset,
472+
// we must read it in non-sequential mode because it will be traversed twice (once
473+
// here for the parameters, then as a regular row).
474+
msg = await Connector.ReadMessage(async);
475+
476+
// If we got an actual first row (i.e. resultset wasn't empty), we process the message so it can
477+
// be read by PopulateOutputParameters(). We then rewind the buffer to the start of the row, and
478+
// continue to the normal flow, where we will process it again, as if we're reading a totally
479+
// new row (using the same first row).
480+
if (msg is DataRowMessage row)
481+
{
482+
var pos = Connector.ReadBuffer.ReadPosition;
483+
ProcessMessage(row);
484+
PopulateOutputParameters();
485+
Connector.ReadBuffer.ReadPosition = pos;
486+
}
487+
}
488+
else
489+
msg = await ReadMessage(async);
490+
468491
if (RowDescription == null)
469492
{
470493
// Statement did not generate a resultset (e.g. INSERT)
@@ -502,6 +525,45 @@ protected virtual async Task<bool> NextResult(bool async, bool isConsuming=false
502525
return false;
503526
}
504527

528+
void PopulateOutputParameters()
529+
{
530+
// The first row in a stored procedure command that has output parameters needs to be traversed twice -
531+
// once for populating the output parameters and once for the actual result set traversal. So in this
532+
// case we can't be sequential.
533+
Debug.Assert(Command.Parameters.Any(p => p.IsOutputDirection));
534+
Debug.Assert(StatementIndex == 0);
535+
Debug.Assert(RowDescription != null);
536+
Debug.Assert(State == ReaderState.BeforeResult);
537+
538+
// Temporarily set our state to InResult to allow us to read the values
539+
State = ReaderState.InResult;
540+
541+
var pending = new Queue<object>();
542+
var taken = new List<NpgsqlParameter>();
543+
for (var i = 0; i < FieldCount; i++)
544+
{
545+
if (Command.Parameters.TryGetValue(GetName(i), out var p) && p.IsOutputDirection)
546+
{
547+
p.Value = GetValue(i);
548+
taken.Add(p);
549+
}
550+
else
551+
pending.Enqueue(GetValue(i));
552+
}
553+
554+
// Not sure where this odd behavior comes from: all output parameters which did not get matched by
555+
// name now get populated with column values which weren't matched. Keeping this for backwards compat,
556+
// opened #2252 for investigation.
557+
foreach (var p in Command.Parameters.Where(p => p.IsOutputDirection && !taken.Contains(p)))
558+
{
559+
if (pending.Count == 0)
560+
break;
561+
p.Value = pending.Dequeue();
562+
}
563+
564+
State = ReaderState.BeforeResult; // Set the state back
565+
}
566+
505567
/// <summary>
506568
/// Note that in SchemaOnly mode there are no resultsets, and we read nothing from the backend (all
507569
/// RowDescriptions have already been processed and are available)

‎src/Npgsql/NpgsqlDefaultDataReader.cs‎

Lines changed: 0 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -33,56 +33,6 @@ internal NpgsqlDefaultDataReader(NpgsqlConnector connector) : base(connector) {}
3333
internal override ValueTask<IBackendMessage> ReadMessage(bool async)
3434
=> Connector.ReadMessage(async);
3535

36-
protected override Task<bool> NextResult(bool async, bool isConsuming=false)
37-
{
38-
return Command.Parameters.HasOutputParameters && StatementIndex == -1
39-
? NextResultWithOutputParams()
40-
: base.NextResult(async, isConsuming);
41-
42-
async Task<bool> NextResultWithOutputParams()
43-
{
44-
var hasResultSet = await base.NextResult(async, isConsuming);
45-
46-
if (!hasResultSet || !HasRows)
47-
return hasResultSet;
48-
49-
// The first row in a stored procedure command that has output parameters needs to be traversed twice -
50-
// once for populating the output parameters and once for the actual result set traversal. So in this
51-
// case we can't be sequential.
52-
Debug.Assert(Command.Parameters.Any(p => p.IsOutputDirection));
53-
Debug.Assert(StatementIndex == 0);
54-
Debug.Assert(RowDescription != null);
55-
Debug.Assert(State == ReaderState.BeforeResult);
56-
57-
// Temporarily set our state to InResult to allow us to read the values
58-
State = ReaderState.InResult;
59-
60-
var pending = new Queue<NpgsqlParameter>();
61-
var taken = new List<int>();
62-
foreach (var p in Command.Parameters.Where(p => p.IsOutputDirection))
63-
{
64-
if (RowDescription.TryGetFieldIndex(p.TrimmedName, out var idx))
65-
{
66-
// TODO: Provider-specific check?
67-
p.Value = GetValue(idx);
68-
taken.Add(idx);
69-
}
70-
else
71-
pending.Enqueue(p);
72-
}
73-
for (var i = 0; pending.Count != 0 && i != RowDescription.NumFields; ++i)
74-
{
75-
// TODO: Need to get the provider-specific value based on the out param's type
76-
if (!taken.Contains(i))
77-
pending.Dequeue().Value = GetValue(i);
78-
}
79-
80-
State = ReaderState.BeforeResult; // Set the state back
81-
82-
return hasResultSet;
83-
}
84-
}
85-
8636
[MethodImpl(MethodImplOptions.AggressiveInlining)]
8737
internal override Task ConsumeRow(bool async)
8838
{

‎test/Npgsql.Tests/CommandTests.cs‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -818,19 +818,19 @@ public void TableDirect()
818818
}
819819

820820
[Test]
821-
public void InputAndOutputParameters()
821+
[TestCase(CommandBehavior.Default)]
822+
[TestCase(CommandBehavior.SequentialAccess)]
823+
public void InputAndOutputParameters(CommandBehavior behavior)
822824
{
823825
using (var conn = OpenConnection())
824-
using (var cmd = new NpgsqlCommand())
826+
using (var cmd = new NpgsqlCommand("SELECT @c-1 AS c, @a+2 AS b", conn))
825827
{
826-
cmd.Connection = conn;
827-
cmd.CommandText = "Select :a + 2 as b, :c - 1 as c";
828+
cmd.Parameters.Add(new NpgsqlParameter("a", 3));
828829
var b = new NpgsqlParameter { ParameterName = "b", Direction = ParameterDirection.Output };
829830
cmd.Parameters.Add(b);
830-
cmd.Parameters.Add(new NpgsqlParameter("a", 3));
831831
var c = new NpgsqlParameter { ParameterName = "c", Direction = ParameterDirection.InputOutput, Value = 4 };
832832
cmd.Parameters.Add(c);
833-
using (cmd.ExecuteReader())
833+
using (cmd.ExecuteReader(behavior))
834834
{
835835
Assert.AreEqual(5, b.Value);
836836
Assert.AreEqual(3, c.Value);

‎test/Npgsql.Tests/ReaderTests.cs‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -549,8 +549,6 @@ public void GetProviderSpecificValues()
549549
[Test]
550550
public void ExecuteReaderGettingEmptyResultSetWithOutputParameter()
551551
{
552-
if (IsSequential)
553-
Assert.Pass("Not supported in sequential mode");
554552
using (var conn = OpenConnection())
555553
{
556554
conn.ExecuteNonQuery("CREATE TEMP TABLE data (name TEXT)");

0 commit comments

Comments
 (0)