Skip to content
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.

Sign up for GitHub

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

Support for java.time #37

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
Open

Support for java.time #37

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
678b0c1
changes to support java.time and and override to account for H2 not s…
graynk Dec 7, 2018
559a93e
Added forced conversions to sql types for DBs that are non-compliant …
graynk Dec 11, 2018
e86d9c8
Override of new missing methods
graynk Dec 12, 2018
ed1adcb
Delete JdbcDatabaseConnection.java
graynk Dec 12, 2018
3eb089b
Whoops.
graynk Dec 12, 2018
11d1d9f
whoops
graynk Dec 12, 2018
5569c60
Typo
graynk Dec 13, 2018
2f47ff5
Enum change
graynk Dec 18, 2018
4805f2e
Merge branch 'master' into jsr310-persister-pr
graynk Feb 9, 2019
ab77ff7
Draft for FieldConverter wrapper for Offset types
graynk Mar 19, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions src/main/java/com/j256/ormlite/db/Db2DatabaseType.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@

import java.util.List;

import com.j256.ormlite.field.DataPersister;
import com.j256.ormlite.field.DataType;
import com.j256.ormlite.field.FieldConverter;
import com.j256.ormlite.field.FieldType;

/**
Expand Down Expand Up @@ -74,4 +77,22 @@ public boolean isOffsetSqlSupported() {
// there is no easy way to do this in this database type
return false;
}

@Override
public FieldConverter getFieldConverter(DataPersister dataType, FieldType fieldType) {
// we are only overriding certain types
switch (dataType.getSqlType()) {
case LOCAL_DATE: // db2 doesn't support JDBC 4.2
return DataType.LOCAL_DATE_SQL.getDataPersister();
case LOCAL_TIME:
return DataType.LOCAL_TIME_SQL.getDataPersister();
case LOCAL_DATE_TIME:
return DataType.LOCAL_DATE_TIME_SQL.getDataPersister();
case OFFSET_TIME: // db2 doesn't seem to support TIME/STAMP WITH TIME ZONE
case OFFSET_DATE_TIME:
return null;
Copy link
Collaborator

@Bo98 Bo98 Dec 19, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This pattern is going to cause problems. Returning null leads to hard-to-debug NullPointerExceptions within ORMLite.

We have two options here I can think of:

  1. Throw a friendly exception. It'll be a bit messy to take this into account in the tests though.

  2. Make a best effort at converting. We should be able to easily emulate the behaviour of PostgreSQL and H2. They, at database level, just convert the timestamp to UTC and store it as such. On converting back it uses the system timezone. The timezone is never actually stored, so the WITH TIMEZONE didn't actually mean much in the first place. We could do the same here using a FieldConverter to LocalTime/LocalDateTime. We want to take Instant into account too (which doesn't have its own SQL type because it rightly doesn't need one) so perhaps a FieldConverter wrapper around the data persister would work better than just creating a new DataPersister. It gives more flexibility for custom persisters too (say if you wanted to create persisters for ThreeTen for example). See Fixed exceptions in some JDBC drivers when working with chars #40 for an approach I took in another situation. I think this approach will work best. We won't be doing any worse than PostgreSQL and H2's efforts.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, haven't had a lot of free time on my hands. I'll be taking a vacation somewhere in the end of January/beginning of February and will do my best to catch up on both PRs then.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, no problem. I’ll test everything again when you manage to take a look.

default:
return super.getFieldConverter(dataType, fieldType);
}
}
}
10 changes: 10 additions & 0 deletions src/main/java/com/j256/ormlite/db/DerbyEmbeddedDatabaseType.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
import com.j256.ormlite.field.FieldConverter;
import com.j256.ormlite.field.FieldType;
import com.j256.ormlite.field.SqlType;
import com.j256.ormlite.field.DataType;
import com.j256.ormlite.misc.IOUtils;
import com.j256.ormlite.misc.SqlExceptionUtil;
import com.j256.ormlite.support.DatabaseResults;
Expand Down Expand Up @@ -73,6 +74,15 @@ public FieldConverter getFieldConverter(DataPersister dataType, FieldType fieldT
serializableConverter = new SerializableFieldConverter();
}
return serializableConverter;
case LOCAL_DATE: // derby doesn't support JDBC 4.2
return DataType.LOCAL_DATE_SQL.getDataPersister();
case LOCAL_TIME:
return DataType.LOCAL_TIME_SQL.getDataPersister();
case LOCAL_DATE_TIME:
return DataType.LOCAL_DATE_TIME_SQL.getDataPersister();
case OFFSET_TIME: // derby doesn't seem to support TIME/STAMP WITH TIME ZONE
case OFFSET_DATE_TIME:
return null;
default:
return super.getFieldConverter(dataType, fieldType);
}
Expand Down
18 changes: 18 additions & 0 deletions src/main/java/com/j256/ormlite/db/H2DatabaseType.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,10 @@
import java.util.List;

import com.j256.ormlite.field.FieldType;
import com.j256.ormlite.field.SqlType;
import com.j256.ormlite.field.DataType;
import com.j256.ormlite.field.DataPersister;
import com.j256.ormlite.field.FieldConverter;

/**
* H2 database type information used to create the tables, etc..
Expand Down Expand Up @@ -52,6 +56,20 @@ public void appendLimitValue(StringBuilder sb, long limit, Long offset) {
sb.append(limit).append(' ');
}

@Override
public void appendOffsetTimeType(StringBuilder sb, FieldType fieldType, int fieldWidth) {
sb.append("TIMESTAMP WITH TIME ZONE");
}

@Override
public FieldConverter getFieldConverter(DataPersister dataPersister, FieldType fieldType) {
// H2 doesn't support TIME WITH TIME ZONE
if (dataPersister.getSqlType() == SqlType.OFFSET_TIME)
return DataType.OFFSET_TIME_SQL.getDataPersister();
Copy link
Collaborator

@Bo98 Bo98 Dec 17, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Following the change in -core, this should now be OFFSET_TIME_COMPAT. Similarly in Postgres too.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

// default is to use the dataPersister itself
return dataPersister;
}

@Override
public boolean isOffsetLimitArgument() {
return true;
Expand Down
21 changes: 21 additions & 0 deletions src/main/java/com/j256/ormlite/db/NetezzaDatabaseType.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@

import java.util.List;

import com.j256.ormlite.field.DataPersister;
import com.j256.ormlite.field.DataType;
import com.j256.ormlite.field.FieldConverter;
import com.j256.ormlite.field.FieldType;

/**
Expand Down Expand Up @@ -80,4 +83,22 @@ public void appendSelectNextValFromSequence(StringBuilder sb, String sequenceNam
// this is word and not entity unfortunately
appendEscapedWord(sb, sequenceName);
}

@Override
public FieldConverter getFieldConverter(DataPersister dataType, FieldType fieldType) {
// we are only overriding certain types
switch (dataType.getSqlType()) {
case LOCAL_DATE: // netezza doesn't seem to support JDBC 4.2
return DataType.LOCAL_DATE_SQL.getDataPersister();
case LOCAL_TIME:
return DataType.LOCAL_TIME_SQL.getDataPersister();
case LOCAL_DATE_TIME:
return DataType.LOCAL_DATE_TIME_SQL.getDataPersister();
case OFFSET_TIME:
case OFFSET_DATE_TIME:
return null;
default:
return super.getFieldConverter(dataType, fieldType);
}
}
}
18 changes: 18 additions & 0 deletions src/main/java/com/j256/ormlite/db/PostgresDatabaseType.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,10 @@
import java.util.List;

import com.j256.ormlite.field.FieldType;
import com.j256.ormlite.field.SqlType;
import com.j256.ormlite.field.DataType;
import com.j256.ormlite.field.DataPersister;
import com.j256.ormlite.field.FieldConverter;

/**
* Postgres database type information used to create the tables, etc..
Expand Down Expand Up @@ -127,4 +131,18 @@ public boolean isCreateIfNotExistsSupported() {
return super.isCreateIfNotExistsSupported();
}
}

@Override
public void appendOffsetTimeType(StringBuilder sb, FieldType fieldType, int fieldWidth) {
sb.append("TIMESTAMP WITH TIME ZONE");
}

@Override
public FieldConverter getFieldConverter(DataPersister dataPersister, FieldType fieldType) {
// H2 doesn't support TIME WITH TIME ZONE
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor: this should refer to Postgres - not H2.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed, thanks

if (dataPersister.getSqlType() == SqlType.OFFSET_TIME)
return DataType.OFFSET_TIME_SQL.getDataPersister();
// default is to use the dataPersister itself
return dataPersister;
}
}
23 changes: 23 additions & 0 deletions src/main/java/com/j256/ormlite/db/SqliteDatabaseType.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
package com.j256.ormlite.db;

import com.j256.ormlite.field.DataPersister;
import com.j256.ormlite.field.DataType;
import com.j256.ormlite.field.FieldConverter;
import com.j256.ormlite.field.FieldType;

/**
* Sqlite database type information used to create the tables, etc..
*
Expand Down Expand Up @@ -52,4 +57,22 @@ public boolean isNestedSavePointsSupported() {
public void appendOffsetValue(StringBuilder sb, long offset) {
throw new IllegalStateException("Offset is part of the LIMIT in database type " + getClass());
}

@Override
public FieldConverter getFieldConverter(DataPersister dataType, FieldType fieldType) {
// we are only overriding certain types
switch (dataType.getSqlType()) {
case LOCAL_DATE: // sqlite doesn't support JDBC 4.2
return DataType.LOCAL_DATE_SQL.getDataPersister();
case LOCAL_TIME:
return DataType.LOCAL_TIME_SQL.getDataPersister();
case LOCAL_DATE_TIME:
return DataType.LOCAL_DATE_TIME_SQL.getDataPersister();
case OFFSET_TIME: // sqlite doesn't seem to support TIME/STAMP WITH TIME ZONE
case OFFSET_DATE_TIME:
return null;
default:
return super.getFieldConverter(dataType, fieldType);
}
}
}
42 changes: 42 additions & 0 deletions src/main/java/com/j256/ormlite/jdbc/JdbcDatabaseResults.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,13 @@
import java.sql.ResultSetMetaData;
import java.sql.SQLException;
import java.sql.Timestamp;
import java.sql.Time;
import java.sql.Date;
import java.time.LocalDate;
import java.time.LocalTime;
import java.time.LocalDateTime;
import java.time.OffsetTime;
import java.time.OffsetDateTime;

import com.j256.ormlite.dao.ObjectCache;
import com.j256.ormlite.misc.IOUtils;
Expand Down Expand Up @@ -175,6 +182,41 @@ public Timestamp getTimestamp(int columnIndex) throws SQLException {
return resultSet.getTimestamp(columnIndex + 1);
}

@Override
public Date getDate(int columnIndex) throws SQLException {
return resultSet.getDate(columnIndex + 1);
}

@Override
public Time getTime(int columnIndex) throws SQLException {
return resultSet.getTime(columnIndex + 1);
}

@Override
public LocalDate getLocalDate(int columnIndex) throws SQLException {
return resultSet.getObject(columnIndex + 1, LocalDate.class);
}

@Override
public LocalTime getLocalTime(int columnIndex) throws SQLException {
return resultSet.getObject(columnIndex + 1, LocalTime.class);
}

@Override
public LocalDateTime getLocalDateTime(int columnIndex) throws SQLException {
return resultSet.getObject(columnIndex + 1, LocalDateTime.class);
}

@Override
public OffsetTime getOffsetTime(int columnIndex) throws SQLException {
return resultSet.getObject(columnIndex + 1, OffsetTime.class);
}

@Override
public OffsetDateTime getOffsetDateTime(int columnIndex) throws SQLException {
return resultSet.getObject(columnIndex + 1, OffsetDateTime.class);
}

@Override
public BigDecimal getBigDecimal(int columnIndex) throws SQLException {
return resultSet.getBigDecimal(columnIndex + 1);
Expand Down
13 changes: 13 additions & 0 deletions src/main/java/com/j256/ormlite/jdbc/TypeValMapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,21 @@ public class TypeValMapper {
values = new int[] { Types.LONGVARCHAR };
break;
case DATE:
case LOCAL_DATE_TIME:
values = new int[] { Types.TIMESTAMP };
break;
case LOCAL_DATE:
values = new int[] { Types.DATE };
break;
case LOCAL_TIME:
values = new int[] { Types.TIME };
break;
case OFFSET_TIME:
values = new int[] { Types.TIME_WITH_TIMEZONE };
break;
case OFFSET_DATE_TIME:
values = new int[] { Types.TIMESTAMP_WITH_TIMEZONE };
break;
case BOOLEAN:
values = new int[] { Types.BOOLEAN };
break;
Expand Down