Skip to content

Add bindTimestamp with non-default Calendar to JdbcPreparedStatement #5629

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

vdshb
Copy link

@vdshb vdshb commented Jan 30, 2025

The suggested (See: https://stackoverflow.com/a/6627999) way to save Instant into Postgresql with JDBC is:

val instant = java.time.Instant.now()
val ts = java.sql.Timestamp.from(instant);
val tzUTC = java.util.Calendar.getInstance(TimeZone.getTimeZone("UTC"));
ps.setTimestamp(col, ts, tzUTC);   // column is TIMESTAMPTZ!

Currently it can't be done with JdbcPreparedStatement.

This PR is meant to improve this aspect of JdbcPreparedStatement.

@vdshb
Copy link
Author

vdshb commented Jan 30, 2025

I've found a way to save Instant by converting it into OffsetDateTime and use bindObject.

I still think, that if we have bindTimestamp we should be able to put Calendar in it.

I am also confused, that every bind method except date/time binding methods increased index by one, so I've added it to my last commit (let me know if it wasn't a bug and I'll revert it).

@griffio
Copy link
Contributor

griffio commented Feb 6, 2025

You would also need to update the sqldelight jdbc driver api spec this is checked during the gradle build for breaking changes to the public api

public final fun bindTimestamp (ILjava/sql/Timestamp;)V

The java.time.Instance type is currently not supported in Postgres JDBC https://jdbc.postgresql.org/documentation/query/#table51-supportedjava-8-date-and-time-classes

As a Java Instant has to be stored as a timezone type - You must ensure that the storage column type is TIMESTAMPTZ

Rather than resorting to legacy types, a SqlDelight Adapter could be used to convert between Instant and OffsetDateTime as you may prefer Instant in your data class model rather than OffsetDateTime e.g

import java.time.Instant;

CREATE TABLE Requests (
   ping TIMESTAMPTZ AS Instant,
   ipaddress TEXT
);
val pingAdapter = object : ColumnAdapter<Instant, OffsetDateTime> {
    override fun decode(databaseValue: OffsetDateTime): Instant = databaseValue.toInstant()
    override fun encode(value: Instant): OffsetDateTime = value.atOffset(ZoneOffset.UTC)
}

I am also confused, that every bind method except date/time binding methods increased index by one, so I've added it to my last commit (let me know if it wasn't a bug and I'll revert it).

Yes - JDBC is position based (1..n) so that would be correct to update it

@vdshb
Copy link
Author

vdshb commented Mar 8, 2025

You would also need to update the sqldelight jdbc driver api spec

I really don't know neither the syntax of the spec nor what it is based on. Isn't it somehow generated?

@griffio
Copy link
Contributor

griffio commented Mar 9, 2025

You would also need to update the sqldelight jdbc driver api spec

I really don't know neither the syntax of the spec nor what it is based on. Isn't it somehow generated?

You can use the tasks ./gradlew :drivers:jdbc-driver:apiCheck to validate and ./gradlew :drivers:jdbc-driver:apiDump to update the spec

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants