Support for inline SET — change a run-time parameter

[ng-galien]

Please read https://stackoverflow.com/help/minimal-reproducible-example

Describe the issue
When executing a query preceded by a PostgreSQL SET command

SET myapp.param1 = 'value1'; SELECT * FROM test;
SET myapp.param1 = 'value1'; INSERT INTO test(name) VALUES (?);

the executeQuery() method throws an org.postgresql.util.PSQLException: No results were returned by the query, and executeUpdate() returns 0.

This behavior is inconsistent and unexpected as the SET command is often used for audit purposes, RLS, etc., and should not interfere with the subsequent query execution.

As a concrete use case consider the need to inject session parameter with Hibernate, we can expect solve the problem with a StatementInspector

public class SqlCommentStatementInspector implements StatementInspector {

    private static final Logger LOGGER = LoggerFactory.getLogger(SqlCommentStatementInspector.class);
    @Override
    public String inspect(String sql) {
        LOGGER.info("SQL: {}", sql);
        var effectiveSql = mustSetProperty(sql) ? "SET app.comment = 'test'; " + sql
                                    : sql;
        LOGGER.info("Effective SQL: {}", effectiveSql);
        return effectiveSql;
    }

    boolean mustSetProperty(String sql) {
        return isInsert(sql) || isUpdate(sql) || isDelete(sql);
    }

    boolean isInsert(String sql) {
        return sql.startsWith("insert");
    }

    boolean isUpdate(String sql) {
        return sql.startsWith("update");
    }

    boolean isDelete(String sql) {
        return sql.startsWith("delete");
    }
}

Driver Version?
42.6.0

Java Version?
21

OS Version?
OSX arm64

PostgreSQL Version?
15.4

To Reproduce
Steps to reproduce the behavior:

• Create a test table and insert some data into it.
• Try executing a SELECT or INSERT statement preceded by a SET command using executeQuery() or executeUpdate(), respectively.
• Observe that executeQuery() throws an org.postgresql.util.PSQLException: No results were returned by the query, and executeUpdate() returns 0.

Expected behaviour
The SET command should set the session or local configuration parameter as expected and allow the subsequent query to execute normally. executeQuery() and executeUpdate() should work as expected, returning the results or the number of affected rows respectively.

Logs

Using the following template code make sure the bug can be replicated in the driver alone.

public class SetAndQueryTest {

    private Connection connection;

    @BeforeEach
    public void setup() throws SQLException {
        String url = "jdbc:postgresql://localhost/postgres?user=postgres&password=postgres";
        connection = DriverManager.getConnection(url);
        try (var statement = connection.createStatement()) {
            statement.execute("""
                    CREATE TABLE IF NOT EXISTS test(
                        id SERIAL PRIMARY KEY,
                        name VARCHAR(255) NOT NULL
                    );
                    """);
            statement.execute("""
                    INSERT INTO test(name) VALUES 
                    ('test1'),
                    ('test2'),
                    ('test3');
                    """);
        }
    }

    @AfterEach
    public void teardown() throws SQLException {
        try (var statement = connection.createStatement()) {
            statement.execute("DROP TABLE IF EXISTS test;");
        }
        connection.close();
    }
    
    @Test
    void testSelectWithSet() throws SQLException {
        try (var statement = connection.createStatement()) {
            assertThrows(PSQLException.class, () -> {
                statement.executeQuery("""
                    SET myapp.param1 = 'value1';
                    SELECT * FROM test;
                    """);
            });
        }
    }

    @Test
    void testSelect() throws SQLException {
        try (var statement = connection.createStatement()) {
            try(var rs = statement.executeQuery("""
                    SELECT * FROM test;
                    """)) {
                while (rs.next()) {
                    assertNotNull(rs.getString("name"));
                }
            }
        }
    }

    @Test
    void tesInsertWithSet() throws SQLException {
        try (var preparedStatement = connection.prepareStatement("""
                    SET myapp.param1 = 'value1';
                    INSERT INTO test(name) VALUES (?);
                    """)) {
            preparedStatement.setString(1, "test4");
            int nb = preparedStatement.executeUpdate();
            assertEquals(0, nb);
        }
    }

    @Test
    void testInsert() throws SQLException {
        try (var stmt = connection.prepareStatement("""
                    INSERT INTO test(name) VALUES (?);
                    """)) {
            stmt.setString(1, "test4");
            int nb = stmt.executeUpdate();
            assertEquals(1, nb);
        }
    }
}

[davecramer]

The issue here is that you cannot execute the SET in an executeQuery() as set does not return anything.

You can execute it in execute() then execute the query. It will take two calls though

[vlsi]

I wonder if we can make such behaviour acceptable. I'm not sure it is the best escape hatch, however, I would really like to have something for "select batching" (like PreparedStatement#executeBatch, but for SELECT statements) and/or "query pipelining" (e.g. ability to issue several queries, and deal with the results later).

We have java.sql.Connection#setClientInfo(java.lang.String, java.lang.String) which issues set application_name=.... We don't pipeline the query (we issue set application_name=... right at setClientInfo call), however, we could defer set ... till the next query comes so we don't stall on network roundtrips.

session parameter with Hibernate, we can expect solve the problem

The drawback of set.. commands is they can't use bind variables, so they have to be parsed on every execution :-/
If only the database provided server-prepared statements for set... statements

[davecramer]

We have java.sql.Connection#setClientInfo(java.lang.String, java.lang.String) which issues set application_name=.... We don't pipeline the query (we issue set application_name=... right at setClientInfo call), however, we could defer set ... till the next query comes so we don't stall on network roundtrips.

This would sort of play havoc with poolers. If you haven't issued the set before the close you would have to do so on close()

This seems pretty hacky

Dave

[ng-galien]

Hello,

I've just submitted a POC to show that the feature won't require a lot of changes in the code.
However, I don't know if this contravenes the JDBC specification

[davecramer]

saw that, thanks! let's see if it passes the tests

[ng-galien]

saw that, thanks! let's see if it passes the tests

I'm not able to run the full test suite at the moment, try to find a way to avoid side effects for now...
In a first iteration I was looking at the parser level in order to concatenate SET statements, but Postgres reject it.

[ng-galien]

@davecramer it's quite difficult to wonder what appends on the CI has I get some test failures on my machine depending on PG version (10 to 12).

gradle clean build

Tests result

• PGV=15 docker/bin/postgres-server

WARNING 816,9sec, 6732 completed, 0 failed, 70 skipped, Gradle Test Run :postgresql:test

• PGV=14 docker/bin/postgres-server

WARNING 232,0sec, 6732 completed, 0 failed, 70 skipped, Gradle Test Run :postgresql:test

• PGV=13 docker/bin/postgres-server

WARNING 226,9sec, 6732 completed, 0 failed, 70 skipped, Gradle Test Run :postgresql:test

• PGV=12 docker/bin/postgres-server

FAILURE 226,1sec, 6732 completed, 1 failed, 70 skipped, Gradle Test Run :postgresql:test.

• PGV=11 docker/bin/postgres-server

FAILURE 226,1sec, 6732 completed, 1 failed, 70 skipped, Gradle Test Run :postgresql:test.

• PGV=10 docker/bin/postgres-server

FAILURE 219,3sec, 6732 completed, 1 failed, 87 skipped, Gradle Test Run :postgresql:test

• PGV=9 SCRAM=false docker/bin/postgres-server

WARNING 186,1sec, 6720 completed, 0 failed, 94 skipped, Gradle Test Run :postgresql:test

• PGV=8.4 SSL=no SCRAM=false docker/bin/postgres-server

Does not start on arm64

The failures are both

FAILURE 1,8sec, 11 completed, 1 failed, 0 skipped, org.postgresql.replication.LogicalReplicationStatusTest

java.lang.AssertionError: Activity in other database will generate WAL but no XLogData  messages. Received LSN will begin to advance beyond of confirmed flushLSN
Expected: not <LSN{0/1729128}>
     but: was <LSN{0/1729128}>

[davecramer]

I doubt your PR caused that failure. You can safely ignore that one I think