SQLWarnings returned on successful SQL statements

[yrodiere]

Describe the issue

For some SQL statements (e.g. DROP TABLE IF EXISTS ...), after they execute successfully pgjdbc attaches SQLWarning objects to the statement/connection.

These warnings have an SQLState starting with 00, which makes them technically "success" messages, not "warnings".

This leads to Hibernate ORM faithfully logging these warnings-but-not-quite, which is considered too noisy by some: quarkusio/quarkus#16204 , https://hibernate.atlassian.net/browse/HHH-18296

Driver Version?
42.7.3

Java Version?
11 or 17 or 21

OS Version?
Fedora 40

PostgreSQL Version?
16 in my test, but doesn't seem to matter.

To Reproduce
Steps to reproduce the behaviour:

# Clone https://github.com/yrodiere/jdk-playground , branch postgres-success-as-warning
git clone -b postgres-success-as-warning https://github.com/yrodiere/jdk-playground.git
./mvnw clean test

Expected behaviour

I would expect no SQLWarning when an SQL statement executes succesfully, since the database didn't return any warnings (only success messages).

Instead, a SQLWarning is generated.

Logs
N/A

[davecramer]

org.hibernate.jdk.playground.MyTest Time elapsed: 0.111 s <<< ERROR!
java.lang.IllegalStateException: Could not find a valid Docker environment. Please see logs and check configuration

Surely you could provide a simpler example ?

[yrodiere]

Surely you could provide a simpler example ?

Surely I can, assuming you have postgres running on port 5432 with user test and password test.

Here's another branch:

# Clone https://github.com/yrodiere/jdk-playground , branch postgres-success-as-warning-2
git clone -b postgres-success-as-warning-2 https://github.com/yrodiere/jdk-playground.git
./mvnw clean test

[yrodiere]

And here's code if you'd rather use that:

package org.hibernate.jdk.playground;

import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;

import java.sql.Connection;
import java.sql.DriverManager;
import java.sql.SQLException;
import java.sql.Statement;

public class MyTest {

    @Test
    void test() {
        try (
                Connection conn = DriverManager.getConnection("jdbc:postgresql://localhost:5432/test", "test", "test");
                Statement stmt = conn.createStatement();
        ) {
            stmt.execute("DROP TABLE IF EXISTS tableThatDoesNotexist");
            Assertions.assertNull(stmt.getWarnings());
        } catch (SQLException e) {
            Assertions.fail("SQLException caught", e);
        }
    }

}

[davecramer]

Agreed, we should not send these to the client.

[vlsi]

The driver is not in a position to judge useful vs useless messages. If the app does not want messages from the backend, one could consider adjusting client_min_messages

[davecramer]

In this case they are not SQLWarnings.

Seems it is not a warning ... /* Class 00 - Successful Completion */
#define ERRCODE_SUCCESSFUL_COMPLETION MAKE_SQLSTATE('0','0','0','0','0')

[vlsi]

JDBC has only sqlwarning interface, so it looks fine we funnel all the messages via the only available api.

[davecramer]

Right, but it's not a warning, so I think we can safely ignore it.

[vlsi]

Ignoring the messages would break backward compatibility

[davecramer]

Ignoring the messages would break backward compatibility

Yes, I'm concerned what we might break.

[gavinking]

I don't see how this could possibly break any sensible code.

Currently the driver is sending us a SQLWarning representing a condition that does not rise to the severity of it being a warning. So that's semantically wrong just to begin with.

But worse, the driver is warning me that something happened which I explicitly stated can happen, and should be ignored in the text of the SQL statement being executed.

So this warning is completely useless to the client in a very strict sense. The client knows that the table might not have existed, having stated that explicitly up front. And after execution of the statement, the end result is that the statement was successful either way: the table is now inexistent, whether it previously existed or not.

What possible use could that SQLWarning be?

I realize that you probably don't get many complaints about this behavior. Why? Well, because typical JDBC code simply ignores SQLWarnings completely. Hibernate is exceptional in the sense that it actually checks for these and reports them.

[vlsi]

@gavinking ,

Currently the driver is sending us a SQLWarning representing a condition that does not rise to the severity of it being a warning. So that's semantically wrong just to begin with.

Please suggest how the driver should return messages emitted by the backend.

[vlsi]

So this warning is completely useless to the client in a very strict sense

Please ask PostgreSQL developers so they remove the message in the first place. If you are sure the message is completely useless, sure you could convince the backend devs about it

[gavinking]

Well I guess the SQL State tells the story: 00 is a success code and no warnings should be reported.

Warnings should be reported if the code starts with 01.

No?

[vlsi]

Warnings should be reported if the code starts with 01.

Would you please clarify the corresponding JDBC specification that says that?

--

The server reports a notice message along with the corresponding SQLState, position, severity and so on.
The driver makes no assumptions on the contents, and it conveys the data to Java applications.
The driver is not in a position to judge and decide if the data should be discarded.

There might be applications that use raise notice within triggers as described in the official PostgreSQL documentation: https://www.postgresql.org/docs/16/plpgsql-trigger.html#PLPGSQL-EVENT-TRIGGER, so discarding messages from the database can break applications.

[davecramer]

I don't see how this could possibly break any sensible code.

Currently the driver is sending us a SQLWarning representing a condition that does not rise to the severity of it being a warning. So that's semantically wrong just to begin with.

But worse, the driver is warning me that something happened which I explicitly stated can happen, and should be ignored in the text of the SQL statement being executed.

Yes, I understand the issue.

So this warning is completely useless to the client in a very strict sense. The client knows that the table might not have existed, having stated that explicitly up front. And after execution of the statement, the end result is that the statement was successful either way: the table is now inexistent, whether it previously existed or not.

What possible use could that SQLWarning be?

See Vladimir's example of a trigger that just reports NOTICE's

I realize that you probably don't get many complaints about this behavior. Why? Well, because typical JDBC code simply ignores SQLWarnings completely. Hibernate is exceptional in the sense that it actually checks for these and reports them.

Which is why assuming applications are sane is a challenge.

I think the simplest solution to solve this is for Hibernate to issue set client_min_messages to WARNING as part of the initialization of the session and you won't get the NOTICE messages

[gavinking]

@visi

Would you please clarify the corresponding JDBC specification that says that?

I mean, sadly the JDBC spec and javadoc are completely silent on the question of which conditions should result in a SQLWarning, though it does refer to them as "warnings" and as "database access warnings". From that, and from the name of the class, I think that we can be pretty confident in inferring that a SQLWarning is intended to represent a warning.

From the PostgreSQL documentation here:

https://www.postgresql.org/docs/current/errcodes-appendix.html

and here:

https://www.postgresql.org/docs/current/ecpg-errors.html

it's clear that SQLSTATE 00000 is not a warning.

On the contrary, it's described as "Successful Completion" and as "Indicates no error".

On the other hand:

• class 01 is described as "Warning" and
• class 02 is described as "No Data (this is also a warning class per the SQL standard)".

(Note that there is no such parenthesized phrase in the description of class 00.)

Even more clearly, if I run drop table if exists whatever; in psql, I receive:

NOTICE:  table "asdfcasdfa" does not exist, skipping

That's NOTICE, not WARNING.

But look, just relying on common sense. If I send drop table if exists whatever to the server, I don't need or want a SQLWarning when the table doesn't exist. A warning is supposed to be telling me that something might be wrong. It's crystal clear that nothing is wrong here. If it were wrong that the table did not already exist, then I would not have written if exists.

[gavinking]

See Vladimir's example of a trigger that just reports NOTICE's

A NOTICE is not a WARNING, by definition. And so it appears wrong to represent it as a SQLWarning, which is intended to represent, well, a warning.

I think the simplest solution to solve this is for Hibernate to issue set client_min_messages to WARNING as part of the initialization of the session and you won't get the NOTICE messages

Hibernate doesn't in general have control over configuration of the JDBC connection. That's usually handled by other framework/server code.

[vlsi]

But look, just relying on common sense. If I send drop table if exists whatever to the server, I don't need or want a SQLWarning when the table doesn't exist.

@gavinking , would you please ask PostgreSQL developers so they remove the notice message or reduce its severity to DEBUG or something like that?

---

it appears wrong to represent it as a SQLWarning, which is intended to represent, well, a warning.

Please provide a link to JDBC specification that says SQLWarning must only represent warnings.

If JDBC spec had something like SqlInfoMessage, SqlWarnMessage, we could use them accordingy.
Unfortunately, the JDBC specification has only SQLWarning class to represent messages emitted by the backend.
Fortunately, there's java.sql.SQLException#getSQLState and java.sql.SQLException#getErrorCode, so SQLWarning class can represent messages of various severities out-of-the-box.

---

Here's a use case: imagine multiple application threads drop a table, and they want to perform a cleanup.
However, they want to perform the cleanup only once. How would they do that?

The application might implement the following:

• Issue drop table if exists
• If there's table "asdfcasdfa" does not exist notice, then the cleanup is not required
• If the warning was not there, the cleanup is needed

It would implement "single cleanup" semantics even in case multiple concurrent threads attempt dropping the table (e.g. a historical partition)

[davecramer]

See Vladimir's example of a trigger that just reports NOTICE's

A NOTICE is not a WARNING, by definition. And so it appears wrong to represent it as a SQLWarning, which is intended to represent, well, a warning.

I'm not debating NOTICE vs WARNING on that we agree, however drivers have to thread the needle between the SPEC and the server they are written for. We do lot's of things that are not in the SPEC to allow users to exploit the peculiarities of PostgreSQL. I don't see another way to report a NOTICE back to the client.

I think the simplest solution to solve this is for Hibernate to issue set client_min_messages to WARNING as part of the initialization of the session and you won't get the NOTICE messages

Hibernate doesn't in general have control over configuration of the JDBC connection. That's usually handled by other framework/server code.

I haven't looked in a while, but you do have a PostgreSQL dialect. Sending an SQL set at the beginning doesn't seem out of place to me ?

[gavinking]

Please ask PostgreSQL developers so they remove the message in the first place. If you are sure the message is completely useless, sure you could convince the backend devs about it

I mean that's the thing. The message isn't completely useless when I'm working in psql, for example. It's completely fine for this to be reported as a NOTICE in that context.

So the problem is not with the backend.

The thing that's wrong is that the JDBC driver is reporting a NOTICE as a SQLWarning to a Java client, thus escalating the severity of an informational message to a "something wrong" message.

I'm not debating NOTICE vs WARNING on that we agree, however drivers have to thread the needle between the SPEC and the server they are written for

Yeah, sure, OK, I understand your point that JDBC doesn't give you a way to report NOTICEs.

But then reporting them as warnings by default also doesn't seem right. [If this was something you could opt into, that would be completely fine, but it's the default behavior.]

I mean, let's take a step back: JDBC doesn't give you a way to report LOG or DEBUG1 messages either. But I definitely wouldn't want you to send these back as SQLWarnings when client_min_messages=DEBUG1.

Well, actually, wait up, what does happen in this case? If I set client_min_messages=DEBUG1, do you filter out DEBUG1 messages or do you send them back to the client?

Sending an SQL set at the beginning doesn't seem out of place to me ?

Oh apologies, I read too fast, and I thought you meant it was a JDBC connection property. Indeed, we could easily run a SET statement before executing DDL and that would be a reasonable workaround for us.

[It doesn't really solve the problem that NOTICEs are being reported as warnings, of course.]

[davecramer]

Please ask PostgreSQL developers so they remove the message in the first place. If you are sure the message is completely useless, sure you could convince the backend devs about it

I mean that's the thing. The message isn't completely useless when I'm working in psql, for example. It's completely fine for this to be reported as a NOTICE in that context.

Well not everyone is using JDBC for CRUD apps. People do write monitoring and psql like apps in JDBC. https://www.jackdb.com/ for instance.

So the problem is not with the backend.

The thing that's wrong is that the JDBC driver is reporting a NOTICE as a SQLWarning to a Java client, thus escalating the severity of an informational message to a "something wrong" message.

I'm not debating NOTICE vs WARNING on that we agree, however drivers have to thread the needle between the SPEC and the server they are written for

Yeah, sure, OK, I understand your point that JDBC doesn't give you a way to report NOTICEs.

But then reporting them as warnings by default also doesn't seem right. [If this was something you could opt into, that would be completely fine, but it's the default behavior.]

I mean, let's take a step back: JDBC doesn't give you a way to report LOG or DEBUG1 messages either. But I definitely wouldn't want you to send these back as SQLWarnings when client_min_messages=DEBUG1.

Well, actually, wait up, what does happen in this case? If I set client_min_messages=DEBUG1, do you filter out DEBUG1 messages or do you send them back to the client?

No we don't again the spec doesn't have a SQLDebug message or the like.

Sending an SQL set at the beginning doesn't seem out of place to me ?

Oh apologies, I read too fast, and I thought you meant it was a JDBC connection property. Indeed, we could easily run a SET statement before executing DDL and that would be a reasonable workaround for us.

[It doesn't really solve the problem that NOTICEs are being reported as warnings, of course.]

Agreed

[gavinking]

No we don't

You don't filter them out, or you don't send them to the client?

[davecramer]

No we don't

You don't filter them out, or you don't send them to the client?

We don't filter them out.

[gavinking]

We don't filter them out.

OK, so that really doesn't seem like the right default behavior.

Well not everyone is using JDBC for CRUD apps. People do write monitoring and psql like apps in JDBC. https://www.jackdb.com/ for instance.

Yeah, I mean, like, I think it's fine that people who're doing something other than OLTP have an opt-in feature to gain access to this stuff. (As a JDBC parameter or whatever.) I just don't think it's the right default.

[davecramer]

Ironically this whole thing is caused because PostgreSQL ships with client_min_messages=NOTICE

We don't filter them out.

OK, so that really doesn't seem like the right default behavior.

Users would have to change client_min_messages to receive them, so they are explicitly asking for them and we don't have another method with which to return them

Well not everyone is using JDBC for CRUD apps. People do write monitoring and psql like apps in JDBC. https://www.jackdb.com/ for instance.

Yeah, I mean, like, I think it's fine that people who're doing something other than OLTP have an opt-in feature to gain access to this stuff. (As a JDBC parameter or whatever.) I just don't think it's the right default.

As noted above had Postgres chosen to ship with client_min_messages=WARNING this would be a non-issue. Again we try to be as agnostic as possible.

[yrodiere]

An update: @gavinking followed your recommendation to use client_min_messages in Hibernate ORM, and early tests indicate this can indeed work in our case.

So regardless of what you decide on this topic, Hibernate ORM users should be fine.

Thank you for the discussion!

[davecramer]

You're welcome
Glad we could find some reasonable solution