On Tue, Sep 17, 2024 at 05:01:18PM -0500, Sami Imseih wrote: > > Then, please see attached two lightly-updated patches. 0001 is for a > > backpatch down to v14. This is yours to force things in the exec and > > bind messages for all portal types, with the test (placed elsewhere in > > 14~15 branches). 0002 is for HEAD to add some sanity checks, blowing > > up the tests of pg_stat_statements if one is not careful with the > > query ID reporting. > > These 2 patches look good to me; except for the slight typo > In the commit message of 0002. "backpatch" instead of "backpatck".
Yes, I've noticed this one last Friday and fixed the typo in the commit log after sending the previous patch series. > That leaves us with considering v5-0002 [1]. I do think this is good > for overall correctness of the queryId being advertised after a cache > revalidation, even if users of pg_stat_activity will hardly notice this. > > [1] > https://www.postgresql.org/message-id/DB325894-3EE3-4B2E-A18C-4B34E7B2F5EC%40gmail.com > Yeah. I need more time to evaluate this one. Also, please find one of the scripts I have used for the execute/fetch case, that simply does an INSERT RETURNING with a small fetch size to create a larger window in pg_stat_activity where we don't report the query ID. One can run it like that, crude still on point: # Download a JDBC driver # Create the table to use. psql -c 'create table aa (a int);' postgres CLASSPATH=postgresql-42.7.4.jar java TestReturning.java Then, while running the script, you would notice that pg_stat_activity reports NULL for the query ID with the query text while the batch fetches are processing. I've taken and expanded one of the scripts you have sent for 1d477a907e63. I'd like to get to the point where we are able to test that in core reliably. The sanity checks in the executor paths are a good step forward but they do nothing for the fetch cases. Perhaps Andrew Dunstan work to expose libpq's APIs with the perl TAP tests would help at some point to control the extended protocol queries, but we are going to need more for the fetch case as there are no hooks that would help to grab a query ID. A second option I have in mind would be to set up an injection point that produces a NOTICE if a query ID is set when we end processing an execute message, then check the number of NOTICE messages produced as these can be predictible depending on the number of rows and the fetch size.. This won't fly far unless we can control the fetch size. -- Michael
import java.sql.*;
public class Test {
static final String DB_URL = "jdbc:postgresql://localhost/postgres";
public static void main(String[] args) {
Connection conn = null;
// Open a connection
try {
conn = DriverManager.getConnection(DB_URL);
conn.setAutoCommit(false);
PreparedStatement statement = conn.prepareStatement("INSERT into aa SELECT generate_series(1,10000000) RETURNING a");
statement.setFetchSize(100);
ResultSet rs = statement.executeQuery();
while (rs.next()) {
}
conn.commit();
statement.close();
} catch (SQLException e) {
e.printStackTrace();
try {
conn.rollback();
} catch (SQLException ee) {
ee.printStackTrace();
}
}
}
}
signature.asc
Description: PGP signature
