Skip to content

Commit

Permalink
Fix output parameter population in batch commands (#5644)
Browse files Browse the repository at this point in the history
Fixes #5642

(cherry picked from commit 12e5ec2)
  • Loading branch information
NinoFloris committed Apr 21, 2024
1 parent 5fd00dd commit be50041
Show file tree
Hide file tree
Showing 6 changed files with 94 additions and 56 deletions.
6 changes: 4 additions & 2 deletions src/Npgsql/NpgsqlCommand.cs
Original file line number Diff line number Diff line change
Expand Up @@ -878,7 +878,10 @@ internal void ProcessRawQuery(SqlQueryParser? parser, bool standardConformingStr
batchCommand = TruncateStatementsToOne();
batchCommand.FinalCommandText = CommandText;
if (parameters is not null)
{
batchCommand.PositionalParameters = parameters.InternalList;
batchCommand._parameters = parameters;
}
}
else
{
Expand Down Expand Up @@ -915,8 +918,6 @@ internal void ProcessRawQuery(SqlQueryParser? parser, bool standardConformingStr
else
{
parser.ParseRawQuery(batchCommand, standardConformingStrings);
if (batchCommand._parameters?.HasOutputParameters == true)
ThrowHelper.ThrowNotSupportedException("Batches cannot cannot have out parameters");
ValidateParameterCount(batchCommand);
}
Expand Down Expand Up @@ -993,6 +994,7 @@ internal void ProcessRawQuery(SqlQueryParser? parser, bool standardConformingStr
batchCommand ??= TruncateStatementsToOne();
batchCommand.FinalCommandText = sqlBuilder.ToString();
batchCommand._parameters = parameters;
batchCommand.PositionalParameters.AddRange(inputParameters);
ValidateParameterCount(batchCommand);
Expand Down
13 changes: 6 additions & 7 deletions src/Npgsql/NpgsqlDataReader.cs
Original file line number Diff line number Diff line change
Expand Up @@ -455,15 +455,15 @@ async Task<bool> NextResult(bool async, bool isConsuming = false, CancellationTo
continue;
}

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


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

Expand All @@ -612,7 +611,7 @@ void PopulateOutputParameters()
var taken = new List<NpgsqlParameter>();
for (var i = 0; i < FieldCount; i++)
{
if (Command.Parameters.TryGetValue(GetName(i), out var p) && p.IsOutputDirection)
if (parameters.TryGetValue(GetName(i), out var p) && p.IsOutputDirection)
{
p.Value = GetValue(i);
taken.Add(p);
Expand All @@ -624,7 +623,7 @@ void PopulateOutputParameters()
// Not sure where this odd behavior comes from: all output parameters which did not get matched by
// name now get populated with column values which weren't matched. Keeping this for backwards compat,
// opened #2252 for investigation.
foreach (var p in (IEnumerable<NpgsqlParameter>)Command.Parameters)
foreach (var p in (IEnumerable<NpgsqlParameter>)parameters)
{
if (!p.IsOutputDirection || taken.Contains(p))
continue;
Expand Down
3 changes: 2 additions & 1 deletion src/Npgsql/SqlQueryParser.cs
Original file line number Diff line number Diff line change
Expand Up @@ -501,10 +501,11 @@ void MoveToNextBatchCommand()
{
batchCommand = batchCommands[statementIndex];
batchCommand.Reset();
batchCommand._parameters = parameters;
}
else
{
batchCommand = new NpgsqlBatchCommand();
batchCommand = new NpgsqlBatchCommand { _parameters = parameters };
batchCommands.Add(batchCommand);
}
}
Expand Down
18 changes: 0 additions & 18 deletions test/Npgsql.Tests/BatchTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -70,24 +70,6 @@ public async Task Positional_parameters()
Assert.That(await reader.NextResultAsync(), Is.False);
}

[Test]
public async Task Out_parameters_are_not_allowed()
{
await using var conn = await OpenConnectionAsync();
await using var batch = new NpgsqlBatch(conn)
{
BatchCommands =
{
new("SELECT @p1")
{
Parameters = { new("p", 8) { Direction = ParameterDirection.InputOutput } }
}
}
};

Assert.That(() => batch.ExecuteReaderAsync(Behavior), Throws.Exception.TypeOf<NotSupportedException>());
}

#endregion Parameters

#region NpgsqlBatchCommand
Expand Down
28 changes: 0 additions & 28 deletions test/Npgsql.Tests/CommandTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -547,34 +547,6 @@ public async Task CloseConnection_with_exception()

#endregion

[Test]
public async Task StoredProcedure_positional_parameters_works()
{
if (IsMultiplexing)
return;

await using var connection = await DataSource.OpenConnectionAsync();
await using var transaction = await connection.BeginTransactionAsync(IsolationLevel.Serializable);
await using var batch = new NpgsqlBatch(connection, transaction)
{
BatchCommands =
{
new("unknown_procedure")
{
CommandType = CommandType.StoredProcedure,
Parameters =
{
new() { Value = "" },
new() { DbType = DbType.Int64, Direction = ParameterDirection.Output }
}
},
new ("COMMIT")
}
};

Assert.ThrowsAsync<PostgresException>(() => batch.ExecuteNonQueryAsync());
}

[Test]
public async Task SingleRow([Values(PrepareOrNot.NotPrepared, PrepareOrNot.Prepared)] PrepareOrNot prepare)
{
Expand Down
82 changes: 82 additions & 0 deletions test/Npgsql.Tests/StoredProcedureTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,88 @@ LANGUAGE plpgsql
Assert.That(reader[1], Is.EqualTo(11));
}

[Test]
public async Task Batch_positional_parameters_works()
{
var tempname = await GetTempProcedureName(DataSource);
await using var connection = await DataSource.OpenConnectionAsync();
await using var transaction = await connection.BeginTransactionAsync(IsolationLevel.Serializable);
await using var batch = new NpgsqlBatch(connection, transaction)
{
BatchCommands =
{
new(tempname)
{
CommandType = CommandType.StoredProcedure,
Parameters =
{
new() { Value = "" },
new() { DbType = DbType.Int64, Direction = ParameterDirection.Output }
}
},
new ("COMMIT")
}
};

Assert.ThrowsAsync<PostgresException>(() => batch.ExecuteNonQueryAsync());
}

[Test]
public async Task Batch_StoredProcedure_output_parameters_works()
{
// Proper OUT params were introduced in PostgreSQL 14
MinimumPgVersion(DataSource, "14.0", "Stored procedure OUT parameters are only support starting with version 14");
var sproc = await GetTempProcedureName(DataSource);

await using var connection = await DataSource.OpenConnectionAsync();
await using var transaction = await connection.BeginTransactionAsync(IsolationLevel.Serializable);
var c = connection.CreateCommand();
c.CommandText = $"""
CREATE OR REPLACE PROCEDURE {sproc}
(
p_username TEXT,
OUT p_user_id BIGINT
)
LANGUAGE plpgsql
AS $$
BEGIN
p_user_id = 1;
return;
END;
$$;
""";
await c.ExecuteNonQueryAsync();

await using var batch = new NpgsqlBatch(connection, transaction)
{
BatchCommands =
{
new(sproc)
{
CommandType = CommandType.StoredProcedure,
Parameters =
{
new() { Value = "" },
new() { NpgsqlDbType = NpgsqlDbType.Bigint, Direction = ParameterDirection.Output }
}
},
new(sproc)
{
CommandType = CommandType.StoredProcedure,
Parameters =
{
new() { Value = "" },
new() { NpgsqlDbType = NpgsqlDbType.Bigint, Direction = ParameterDirection.Output }
}
}
}
};

await batch.ExecuteNonQueryAsync();
Assert.AreEqual(1, batch.BatchCommands[0].Parameters[1].Value);
Assert.AreEqual(1, batch.BatchCommands[1].Parameters[1].Value);
}

#region DeriveParameters

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

0 comments on commit be50041

Please sign in to comment.