clickhouse-java icon indicating copy to clipboard operation
clickhouse-java copied to clipboard

Cannot use ResultSet.getObject to retrieve java.sql.Date (JDBC specification violation?)

Open LDVSOFT opened this issue 2 years ago • 10 comments

Describe the bug

At some point ClickHouse JDBC driver switched to using java.time. types to represent temporal values. While I salute this change and would prefer too to abandon java.sql.Date, the way it was done, unfortunately, breaks some end consumers that expect old types to be returned by default. While originally we detected it by trying to read data from ClickHouse to Spark (by using the latter), I was able to provide a more direct counterexample.

Steps to reproduce

  1. Setup a JDBC connection using driver version 0.4.6,
  2. Perform a SELECT d FROM some_table where d has type Date or similar (nullable or array),
  3. Access it via resultSet.getObject("d") or resultSet.getObject("d", java.sql.Date.class).
    • First will give java.time.LocalDate, that could be correct under some type maps but not the default one,
    • Second one fails with a ClassCastException, that in my eyes violates JDBC specification directly.

To be exact, I am referencing to JDBC 4.3 specification, appendix B (quotation with my style):

B.3 JDBC Types Mapped to Java Object Types

ResultSet.getObject and CallableStatement.getObject use the mapping shown in TABLE B-3 for standard mappings.

JDBC Type Java Object Type
skipped skipped
DATE java.sql.Date
skipped skipped
All ResultSet.getObject methods that don't take a type map directly tell in their documentation that they follow the mapping above.

Related:

  • ResultSet.getDate works as expected.
  • Arrays of dates (accessed via resultSet.getArray("ds").getArray()) are broken the same way (java.time.LocalDate[] instead of java.sql.Date[]).
  • I've not tested this yet but I'd expect the same problem to arise for JDBC TIMESTAMP with java.time.Instant vs java.sql.Timestamp.
  • I've not tested other JDBC methods, like setting PreparedStatement parameters, but I would check their behaviour too.

I've checked the behaviour of other JDBC drivers against the respectful databases I use, including PostgreSQL, and they work as expected. Older ClickHouse JDBC drvier (I guess around 0.3.1) worked for us.

Expected behaviour

I guess the right thing to do would be one or more of:

  • Add java.time. support for getObject(<label or number>, java.lang.Class<*>) and getObject(<label or number>, <mapping>) methods while preserving the old defaults,
  • Add explicit opt-in for java.time. to be returned by default, if wanted (I guess it could be achieved by this option changing the connection-default type map),

Either way, don't change behaviour from specification until explicit opt-in for one.

Code example

When we detected the issue, after applying the hotfix I was working on a reproducer to understand if the issue was consistent with other API usage or not, here it goes. It uses JUnit 5, TestContainers and Kotlin.

package com.jetbrains.jetstat.oneshot

import org.junit.jupiter.api.assertAll
import org.testcontainers.containers.ClickHouseContainer
import org.testcontainers.junit.jupiter.Container
import org.testcontainers.junit.jupiter.Testcontainers
import java.sql.Connection
import java.sql.DriverManager
import java.time.LocalDate as JavaTimeDate
import java.sql.Date as JavaSqlDate
import kotlin.test.*

@Testcontainers
class ClickhouseJdbcTest {
    @Container
    private val container = ClickHouseContainer("clickhouse/clickhouse-server:22.2")

    private inline fun withConnection(block: (Connection) -> Unit) =
        DriverManager
            .getConnection(container.jdbcUrl, container.username, container.password)
            .use(block)

    @BeforeTest
    fun `setup database`() {
        withConnection { conn ->
            conn.createStatement().use { statement ->
                statement.execute("CREATE TABLE example (a_date Date, b_dates Array(Date)) ENGINE = Log")
                statement.executeUpdate("INSERT INTO example(a_date, b_dates) VALUES ('2022-01-01', ['2022-01-02', '2022-01-03'])")
            }
        }
    }

    @Test
    fun `check that ClickHouse driver reports DATEs consistently`() {
        val expectedDateA = JavaTimeDate.of(2022, 1, 1)
        val expectedDatesB = listOf(
            JavaTimeDate.of(2022, 1, 2),
            JavaTimeDate.of(2022, 1, 3)
        )

        withConnection { conn ->
            conn.createStatement().use { statement ->
                val resultSet = statement.executeQuery("SELECT * FROM example")
                resultSet.next() // First
                val aDate = resultSet.getObject("a_date")
                val aDateDirect = resultSet.getDate("a_date")
                val bDatesArr = resultSet.getArray("b_dates").array
                val bDatesObj = resultSet.getObject("b_dates")

                println("Type of bDatesArr: ${bDatesArr.javaClass}")
                println("Type of bDatesObj: ${bDatesObj.javaClass}")

                assertEquals(JavaSqlDate.valueOf(expectedDateA), aDateDirect)

                when (aDate) {
                    is JavaSqlDate -> {
                        println("single date returned as JavaSqlDate")

                        assertEquals(JavaSqlDate.valueOf(expectedDateA), aDate)

                        val expected = expectedDatesB.map(JavaSqlDate::valueOf)
                        assertAll({
                            assertTrue(bDatesArr is Array<*> && bDatesArr.isArrayOf<JavaSqlDate>())
                            assertContentEquals(expected, bDatesArr.asList())                             
                        }, {
                            assertTrue(bDatesObj is Array<*> && bDatesObj.isArrayOf<JavaSqlDate>())
                            assertContentEquals(expected, bDatesObj.asList())
                        })
                    }

                    is JavaTimeDate -> {
                        println("single date returned as JavaTimeDate")
                        assertEquals(expectedDateA, aDate)

                        assertEquals(expectedDateA, aDate)
                        assertAll({
                            assertTrue(bDatesArr is Array<*> && bDatesArr.isArrayOf<JavaTimeDate>())
                            assertContentEquals(expectedDatesB, bDatesArr.asList())
                        }, {
                            assertTrue(bDatesObj is Array<*> && bDatesObj.isArrayOf<JavaTimeDate>())
                            assertContentEquals(expectedDatesB, bDatesObj.asList())
                        })
                    }

                    else -> {
                        error("Returned date as a weird type ${aDate.javaClass.name}")
                    }
                }

                val aDateExplicit = resultSet.getObject("a_date", JavaSqlDate::class.java)
                assertEquals(JavaSqlDate.valueOf(expectedDateA), aDateExplicit)
            }
        }
    }
}

Technically it only fails on the last lines, but I believe the consistent reporting of java.time.LocalDates is not the behaviour I expect.

Configuration

Environment

  • Client version: com.clickhouse, clickhouse-jdbc, 0.4.6, http specifier.
  • Language version: Java 17 (from Ubuntu LTS)
  • OS: Linux (Ubuntu 22.04 LTS, up-to-date), amd64

ClickHouse server

  • ClickHouse Server version: tested on Docker image version 22.2, but I believe affects all supported versions
  • ClickHouse Server non-default settings, if any: irrelevant

LDVSOFT avatar Aug 01 '23 16:08 LDVSOFT

Thanks @LDVSOFT, very well explained! Agree it's an issue to fix. Will add an option for explicit opt-in and change the default mapping to legacy Date/Timestamp.

zhicwu avatar Aug 01 '23 23:08 zhicwu

Thanks @zhicwu! That should work out. I think that even on the new mapping driver should still allow to fetch java.sql. types with .getObject(<id>, java.sql.Date.class).

I've noticed that my test case also checks that resultSet.getObject returns java.lang.Array, and that's not necessary required actually, Postgres uses PgArray there.

LDVSOFT avatar Aug 02 '23 11:08 LDVSOFT

Actually, JDBC spec, the same B.3 table, requires to return java.sql.Array for ARRAYs in getObject. Hm.

LDVSOFT avatar Aug 02 '23 11:08 LDVSOFT

To add some more information on a point raised by @LDVSOFT, there is an issue with PreparedStatement.setObject(int parameterIndex, Object x, int targetSqlType) when x is of type java.sql.Timestamp and targetSqlType is java.sql.Types.Timestamp.

As expected it updates the value in ClickHouseDateTimeValue but since java.sql.Types.Timestamp is not handled, it reverts to the default behavior of the String representation - which is deprecated (and incorrect when the Java time zone is not UTC).

jnd77 avatar Jan 19 '24 07:01 jnd77