[CALCITE-2743] Remove MySQL-specific Date & Time shifting

[marcprux]

A workaround for a legacy MySQL bug where Date/Time/Timestamp fields are being shifted against the UTC offset is being applied regardless of the backend store. This PR removes that code, since the can better be worked around using the MySQL "useLegacyDatetimeCode" connection parameter.

Fixes: https://issues.apache.org/jira/browse/CALCITE-2743

[marcprux]

I didn't see any tests in place that tested this specific (mis)behavior. I won't be able to add any tests for this, but I agree they would be nice to have.

[eolivelli]

Maybe you can create a very simple unit test, create and instance of such class and call the method, you know.

It will be very useful for the future in order to prevent that someone else reverts your change

[marcprux]

Here's a test case that demonstrates the failure using nothing but the ReflectiveSchema (i.e., there are no DB-specific timezone shenanigans getting in the way):

public class TimeZoneOffsetTest extends TestCase {
    public void testTimeZoneOffsets() throws Exception {
        testTimestampWithTimeZone("GMT"); // works: 1999-12-31 23:00:00.0
        testTimestampWithTimeZone("America/Chicago"); // fails: 1999-12-31 23:00:00.0 != 2000-01-01 05:00:00.0
    }

    public void testTimestampWithTimeZone(String tz) throws Exception {
        TimeZone curtz = TimeZone.getDefault();
        try {
            TimeZone.setDefault(TimeZone.getTimeZone(tz));
            try (var c = createReflectiveConnection(new TSSchema(), "adhoc")) {
                try (var rs = c.createStatement().executeQuery("SELECT TSTAMP FROM \"TABLE\"")) {
                    assertTrue(rs.next());
                    assertEquals("TimeZone comparison in " + tz, STAMP, rs.getObject(1));
                }
            }
        } finally {
            // restore previous TZ
            TimeZone.setDefault(curtz);
        }
    }

    static java.sql.Timestamp STAMP = new java.sql.Timestamp(946702800000l);
    public static class TSSchema {
        public static TSTable[] TABLE = new TSTable[] { new TSTable() };
        public static class TSTable { public java.sql.Timestamp TSTAMP = STAMP; }
    }

    Connection createReflectiveConnection(Object ob, String schemaName) throws ClassNotFoundException, SQLException {
        var info = new Properties();
        info.put("lex", "MYSQL_ANSI");
        var conn = DriverManager.getConnection("jdbc:calcite:", info);
        conn.unwrap(CalciteConnection.class).getRootSchema().add(schemaName, new ReflectiveSchema((ob)));
        conn.setSchema(schemaName); // use this schema name for queries
        return conn;
    }

}

[eolivelli]

Great. Can you add it to the PR?

[F21]

@marcprux Can you also update the commit message to: [CALCITE-2743] Remove MySQL-specific Date & Time shifting (Marc Prud'hommeaux) ?

[dgloeckner]

Hi,

can someone from the core Calcite team please comment what is needed to have this PR merged?

I'm happy to help. From my pov this bug hinders the adoption of Calcite quite significantly and it just feels bad to receive incorrect timestamps from Calcite.

[rubenada]

@dgloeckner apart from resolving the conflicts, a unit test should be provided in the PR: a test that fails with the current Calcite master (i.e. it shows the issue) and gets fixed with the patch.