Server default DateStyle is being overwritten

[olavloite]

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

Describe the issue
The new default ISO, MDY startup parameter added in #2925 potentially overwrites any custom default datestyle that has been set for the server:

1. Configure a PostgreSQL server with default datestyle = 'iso, dmy'
2. Open a JDBC connection and execute show datestyle
3. The result in 42.6.0 is ISO, DMY
4. The result in 42.7.0 is ISO, MDY

cc @JelteF

The reason for the above change in behavior is that setting only the first part of the date style does not overwrite the second part. 42.6.0 only set DateStyle=ISO, which left the 'ambiguous input part' of the DateStyle intact. The new value that is used in 42.7.0 will overwrite the entire default.

Driver Version?
42.7.0

Java Version?
17

OS Version?
Distributor ID: Debian
Description: Debian GNU/Linux 11 (bullseye)
Release: 11
Codename: bullseye

PostgreSQL Version?
16.1

To Reproduce

1. Configure a PostgreSQL server with default datestyle = 'iso, dmy'
2. Open a JDBC connection and execute show datestyle
3. The result in 42.7.0 is ISO, MDY

Expected behaviour
The result should be the server default ISO, DMY

Logs

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

import java.sql.Connection;
import java.sql.DriverManager;
import java.sql.ResultSet;
import java.sql.Statement;
import java.util.Properties;

public class TestNullsFirst {
    public static void main(String []args) throws Exception {


        String url = "jdbc:postgresql://localhost:5432/test";

        Properties props = new Properties();
        props.setProperty("user", "test");
        props.setProperty("password", "test");
        try ( Connection conn = DriverManager.getConnection(url, props) ){
      try (ResultSet resultSet = conn.createStatement().executeQuery("show datestyle")) {
        assertTrue(resultSet.next());
        // NOTE: This check only succeeds in 42.6.0 if the server default has been set to ISO, DMY
        assertEquals("ISO, DMY", resultSet.getString(1));
        assertFalse(resultSet.next());
      }
        }
    }
}

[vlsi]

I'm puzzled the way we can deal with both #3008 and #2925

It looks like the way out is to make datestyle configurable in pgjdbc, and use old ISO by default, so the users who use "imperfect pgbouncer" can adjust the default "datestyle" sent by pgjdbc based on their needs.

Any thoughts on that?

/cc @JelteF

[olavloite]

Any thoughts on that?

I don't know if you asked me (too), but for clarity: We're not really hit by this, but it showed up as a test failure in a couple of our tests that assumed that the default would be ISO, so I reported it here as it was an unexpected change of behavior. We don't depend on it, but I could imagine that someone who has set a different server default could be surprised by this.

[OrangeDog]

This breaks for most non-American users who parse date inputs in their SQL.
It doesn't break in a nice way either. It silently sets your date values to be wrong.

This 100% should have been configurable. Even better, read the server default and use that.

[vlsi]

@OrangeDog , could you suggest a test case that reproduces the failure?

[vlsi]

As the key reason for #2925 was performance optimization, I suggest we revert the change in 42.7.1 until we have a workable solution that does not break clients.

[OrangeDog]

With the server set to DMY, this used to return "2023-02-01":

import java.sql.Connection;
import java.sql.DriverManager;
import java.sql.ResultSet;
import java.sql.Statement;
import java.util.Properties;

public class TestDefaultDateStyle {
    public static void main(String []args) throws Exception {


        String url = "jdbc:postgresql://localhost:5432/test";

        Properties props = new Properties();
        props.setProperty("user", "test");
        props.setProperty("password", "test");
        try ( Connection conn = DriverManager.getConnection(url, props) ){
            try ( Statement statement = conn.createStatement() ) {
                try ( ResultSet rs = statement.executeQuery("SELECT '01/02/2023'::date") ){
                    if (rs.next())
                        System.out.println("Get String: " + rs.getString(1));
                }
            }
        }
    }
}

[vlsi]

We'll need to add regression tests like #3008 (comment) as we release 42.7.1

[JelteF]

(back from my honeymoon) Seems reasonable that this was reverted. I'll try to get the PgBouncer fix into the next release: pgbouncer/pgbouncer#776

To be clear while the PgBouncer fix fixes the majority of the performance issue, it's not possible to not completely resolve it at pgbouncer its side.

I think the only fix for the perf issue on JDBC side would be to allow JDBC users to specify if they want, ISO, ISO, DMY or ISO, MDY as their DateStyle. And default to the first one (like you've done in the past). Then at least people would be to choose the more performant one.

[JelteF]

FYI I released PgBouncer 1.22.0 which includes the PgBouncer side fix.