Skip to content

Commit 12e5ec2

Browse files
NinoFlorisNinoFloris
authored
Fix output parameter population in batch commands (npgsql#5644)
Fixes npgsql#5642
1 parent 9e70a62 commit 12e5ec2

6 files changed

Lines changed: 94 additions & 56 deletions

File tree

‎src/Npgsql/NpgsqlCommand.cs‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -874,7 +874,10 @@ internal void ProcessRawQuery(SqlQueryParser? parser, bool standardConformingStr
874874
batchCommand = TruncateStatementsToOne();
875875
batchCommand.FinalCommandText = CommandText;
876876
if (parameters is not null)
877+
{
877878
batchCommand.PositionalParameters = parameters.InternalList;
879+
batchCommand._parameters = parameters;
880+
}
878881
}
879882
else
880883
{
@@ -911,8 +914,6 @@ internal void ProcessRawQuery(SqlQueryParser? parser, bool standardConformingStr
911914
else
912915
{
913916
parser.ParseRawQuery(batchCommand, standardConformingStrings);
914-
if (batchCommand._parameters?.HasOutputParameters == true)
915-
ThrowHelper.ThrowNotSupportedException("Batches cannot cannot have out parameters");
916917
ValidateParameterCount(batchCommand);
917918
}
918919

@@ -989,6 +990,7 @@ internal void ProcessRawQuery(SqlQueryParser? parser, bool standardConformingStr
989990

990991
batchCommand ??= TruncateStatementsToOne();
991992
batchCommand.FinalCommandText = sqlBuilder.ToString();
993+
batchCommand._parameters = parameters;
992994
batchCommand.PositionalParameters.AddRange(inputParameters);
993995
ValidateParameterCount(batchCommand);
994996

‎src/Npgsql/NpgsqlDataReader.cs‎

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -459,15 +459,15 @@ async Task<bool> NextResult(bool async, bool isConsuming = false, CancellationTo
459459
continue;
460460
}
461461

462-
if (!Command.IsWrappedByBatch && StatementIndex == 0 && Command._parameters?.HasOutputParameters == true)
462+
if ((Command.IsWrappedByBatch || StatementIndex is 0) && Command.InternalBatchCommands[StatementIndex]._parameters?.HasOutputParameters == true)
463463
{
464-
// If output parameters are present and this is the first row of the first resultset,
464+
// If output parameters are present and this is the first row of the resultset,
465465
// we must always read it in non-sequential mode because it will be traversed twice (once
466466
// here for the parameters, then as a regular row).
467467
msg = await Connector.ReadMessage(async).ConfigureAwait(false);
468468
ProcessMessage(msg);
469469
if (msg.Code == BackendMessageCode.DataRow)
470-
PopulateOutputParameters();
470+
PopulateOutputParameters(Command.InternalBatchCommands[StatementIndex]._parameters!);
471471
}
472472
else
473473
{
@@ -598,12 +598,11 @@ async ValueTask ConsumeResultSet(bool async)
598598
}
599599

600600

601-
void PopulateOutputParameters()
601+
void PopulateOutputParameters(NpgsqlParameterCollection parameters)
602602
{
603603
// The first row in a stored procedure command that has output parameters needs to be traversed twice -
604604
// once for populating the output parameters and once for the actual result set traversal. So in this
605605
// case we can't be sequential.
606-
Debug.Assert(StatementIndex == 0);
607606
Debug.Assert(RowDescription != null);
608607
Debug.Assert(State == ReaderState.BeforeResult);
609608

@@ -616,7 +615,7 @@ void PopulateOutputParameters()
616615
var taken = new List<NpgsqlParameter>();
617616
for (var i = 0; i < ColumnCount; i++)
618617
{
619-
if (Command.Parameters.TryGetValue(GetName(i), out var p) && p.IsOutputDirection)
618+
if (parameters.TryGetValue(GetName(i), out var p) && p.IsOutputDirection)
620619
{
621620
p.Value = GetValue(i);
622621
taken.Add(p);
@@ -628,7 +627,7 @@ void PopulateOutputParameters()
628627
// Not sure where this odd behavior comes from: all output parameters which did not get matched by
629628
// name now get populated with column values which weren't matched. Keeping this for backwards compat,
630629
// opened #2252 for investigation.
631-
foreach (var p in (IEnumerable<NpgsqlParameter>)Command.Parameters)
630+
foreach (var p in (IEnumerable<NpgsqlParameter>)parameters)
632631
{
633632
if (!p.IsOutputDirection || taken.Contains(p))
634633
continue;

‎src/Npgsql/SqlQueryParser.cs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -501,10 +501,11 @@ void MoveToNextBatchCommand()
501501
{
502502
batchCommand = batchCommands[statementIndex];
503503
batchCommand.Reset();
504+
batchCommand._parameters = parameters;
504505
}
505506
else
506507
{
507-
batchCommand = new NpgsqlBatchCommand();
508+
batchCommand = new NpgsqlBatchCommand { _parameters = parameters };
508509
batchCommands.Add(batchCommand);
509510
}
510511
}

‎test/Npgsql.Tests/BatchTests.cs‎

Lines changed: 0 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -70,24 +70,6 @@ public async Task Positional_parameters()
7070
Assert.That(await reader.NextResultAsync(), Is.False);
7171
}
7272

73-
[Test]
74-
public async Task Out_parameters_are_not_allowed()
75-
{
76-
await using var conn = await OpenConnectionAsync();
77-
await using var batch = new NpgsqlBatch(conn)
78-
{
79-
BatchCommands =
80-
{
81-
new("SELECT @p1")
82-
{
83-
Parameters = { new("p", 8) { Direction = ParameterDirection.InputOutput } }
84-
}
85-
}
86-
};
87-
88-
Assert.That(() => batch.ExecuteReaderAsync(Behavior), Throws.Exception.TypeOf<NotSupportedException>());
89-
}
90-
9173
#endregion Parameters
9274

9375
#region NpgsqlBatchCommand

‎test/Npgsql.Tests/CommandTests.cs‎

Lines changed: 0 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -547,34 +547,6 @@ public async Task CloseConnection_with_exception()
547547

548548
#endregion
549549

550-
[Test]
551-
public async Task StoredProcedure_positional_parameters_works()
552-
{
553-
if (IsMultiplexing)
554-
return;
555-
556-
await using var connection = await DataSource.OpenConnectionAsync();
557-
await using var transaction = await connection.BeginTransactionAsync(IsolationLevel.Serializable);
558-
await using var batch = new NpgsqlBatch(connection, transaction)
559-
{
560-
BatchCommands =
561-
{
562-
new("unknown_procedure")
563-
{
564-
CommandType = CommandType.StoredProcedure,
565-
Parameters =
566-
{
567-
new() { Value = "" },
568-
new() { DbType = DbType.Int64, Direction = ParameterDirection.Output }
569-
}
570-
},
571-
new ("COMMIT")
572-
}
573-
};
574-
575-
Assert.ThrowsAsync<PostgresException>(() => batch.ExecuteNonQueryAsync());
576-
}
577-
578550
[Test]
579551
public async Task SingleRow([Values(PrepareOrNot.NotPrepared, PrepareOrNot.Prepared)] PrepareOrNot prepare)
580552
{

‎test/Npgsql.Tests/StoredProcedureTests.cs‎

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,88 @@ LANGUAGE plpgsql
129129
Assert.That(reader[1], Is.EqualTo(11));
130130
}
131131

132+
[Test]
133+
public async Task Batch_positional_parameters_works()
134+
{
135+
var tempname = await GetTempProcedureName(DataSource);
136+
await using var connection = await DataSource.OpenConnectionAsync();
137+
await using var transaction = await connection.BeginTransactionAsync(IsolationLevel.Serializable);
138+
await using var batch = new NpgsqlBatch(connection, transaction)
139+
{
140+
BatchCommands =
141+
{
142+
new(tempname)
143+
{
144+
CommandType = CommandType.StoredProcedure,
145+
Parameters =
146+
{
147+
new() { Value = "" },
148+
new() { DbType = DbType.Int64, Direction = ParameterDirection.Output }
149+
}
150+
},
151+
new ("COMMIT")
152+
}
153+
};
154+
155+
Assert.ThrowsAsync<PostgresException>(() => batch.ExecuteNonQueryAsync());
156+
}
157+
158+
[Test]
159+
public async Task Batch_StoredProcedure_output_parameters_works()
160+
{
161+
// Proper OUT params were introduced in PostgreSQL 14
162+
MinimumPgVersion(DataSource, "14.0", "Stored procedure OUT parameters are only support starting with version 14");
163+
var sproc = await GetTempProcedureName(DataSource);
164+
165+
await using var connection = await DataSource.OpenConnectionAsync();
166+
await using var transaction = await connection.BeginTransactionAsync(IsolationLevel.Serializable);
167+
var c = connection.CreateCommand();
168+
c.CommandText = $"""
169+
CREATE OR REPLACE PROCEDURE {sproc}
170+
(
171+
p_username TEXT,
172+
OUT p_user_id BIGINT
173+
)
174+
LANGUAGE plpgsql
175+
AS $$
176+
BEGIN
177+
p_user_id = 1;
178+
return;
179+
END;
180+
$$;
181+
""";
182+
await c.ExecuteNonQueryAsync();
183+
184+
await using var batch = new NpgsqlBatch(connection, transaction)
185+
{
186+
BatchCommands =
187+
{
188+
new(sproc)
189+
{
190+
CommandType = CommandType.StoredProcedure,
191+
Parameters =
192+
{
193+
new() { Value = "" },
194+
new() { NpgsqlDbType = NpgsqlDbType.Bigint, Direction = ParameterDirection.Output }
195+
}
196+
},
197+
new(sproc)
198+
{
199+
CommandType = CommandType.StoredProcedure,
200+
Parameters =
201+
{
202+
new() { Value = "" },
203+
new() { NpgsqlDbType = NpgsqlDbType.Bigint, Direction = ParameterDirection.Output }
204+
}
205+
}
206+
}
207+
};
208+
209+
await batch.ExecuteNonQueryAsync();
210+
Assert.AreEqual(1, batch.BatchCommands[0].Parameters[1].Value);
211+
Assert.AreEqual(1, batch.BatchCommands[1].Parameters[1].Value);
212+
}
213+
132214
#region DeriveParameters
133215

134216
[Test, Description("Tests function parameter derivation with IN, OUT and INOUT parameters")]

0 commit comments

Comments
 (0)