NpgsqlBatch does not properly set ConnectorPreparedOn property on its BatchCommands

[stojancho-jefremov]

Steps to reproduce

Add following two tests to https://github.com/npgsql/npgsql/blob/main/test/Npgsql.Tests/PrepareTests.cs file and run them.

[Test]
public void Batch_sets_properly_connector_prepared_on_for_its_batch_commands_auto_prepare()
{
    using var dataSource = CreateDataSource(csb =>
    {
        csb.MaxAutoPrepare = 5;
        csb.AutoPrepareMinUsages = 1;
    });
    using var conn = dataSource.OpenConnection();
    using (var batch = new NpgsqlBatch(conn) { BatchCommands = { new("SELECT 1"), new("SELECT 2") } })
    {
        AssertNumPreparedStatements(conn, 2);

        foreach (var batchCommand in batch.BatchCommands)
            Assert.IsNotNull(((NpgsqlBatchCommand)batchCommand).ConnectorPreparedOn);
    }
}

[Test]
public void Batch_sets_properly_connector_prepared_on_for_its_batch_commands()
{
    using var conn = OpenConnectionAndUnprepare();
    using (var batch = new NpgsqlBatch(conn) { BatchCommands = { new("SELECT 1"), new("SELECT 2") } })
    {
        batch.Prepare();
        AssertNumPreparedStatements(conn, 2);
        foreach (var batchCommand in batch.BatchCommands)
            Assert.IsNotNull(((NpgsqlBatchCommand)batchCommand).ConnectorPreparedOn);
    }
    conn.UnprepareAll();
}

The issue

After bumping Npgsql version from 7.0.1 to 7.0.4 we experienced a serious performance degradation in our performance benchmarks. We did an investigation, and the exact culprit seems to be this fix: #4821
It seems that preparation (for a given batch with batch commands) is unnecessary reset because of ConnectorPreparedOn property being incorrectly (almost) always null.

Further technical details

Please note that the issue is still there for the latest versions of npgsql and PostgresSql. Versions used for the tests from above:
Npgsql version: latest from main (8.0.0)
PostgreSQL version: 15.4
Operating system: Windows 10

[vonzshik]

Hello. I'm not sure whether your first test is correct (since you never execute the batch none of the queries are going to be auto prepared, so ConnectorPreparedOn will indeed be null), the second test does show an issue with explicit prepare which I'm going to fix.

[stojancho-jefremov]

Hello @vonzshik! Regarding my first test, it seems that you are right. After I changed it to this:

[Test]
public void Batch_sets_properly_connector_prepared_on_for_its_batch_commands_auto_prepare()
{
    using var dataSource = CreateDataSource(csb =>
    {
        csb.MaxAutoPrepare = 5;
        csb.AutoPrepareMinUsages = 1;
    });
    using var conn = dataSource.OpenConnection();
    using (var batch = new NpgsqlBatch(conn) { BatchCommands = { new("SELECT 1"), new("SELECT 2") } })
    {
        using var reader = batch.ExecuteReader();

        foreach (var batchCommand in batch.BatchCommands)
            Assert.IsNotNull(((NpgsqlBatchCommand)batchCommand).ConnectorPreparedOn);
    }

    AssertNumPreparedStatements(conn, 2);
}

the test stopped failing.

Frankly, we don't use the auto prepare, so I reported that one because I thought there is similar issue as with the explicit prepare. I also looked a bit into the code, but it seems that I didn't get it completely right. Anyway. you can consider adding this test if you find it useful. Thanks!