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.

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

Joda LocalDate / LocalDateTime mapping #691

Closed
dmiorandi opened this issue Mar 31, 2014 · 23 comments
Closed

Joda LocalDate / LocalDateTime mapping #691

dmiorandi opened this issue Mar 31, 2014 · 23 comments
Labels

Comments

@dmiorandi
Copy link

Hi,
I'm using this mapping to use Joda instead Java Date.

        configuration.register(new LocalDateTimeType());
        configuration.register(new LocalDateType());
        configuration.register(new LocalTimeType());

java classes and accessors are generated in right way, mapped on
Date==>LocalDate
DateTime==>LocalDateTime
but if i try to passing a filter based on a costant LocalDate (with no time information)
it's converted in a wrong value using zone that make hours field wrong (+1) instead of 0, but LocalDate have no zone in joda at all.
In joda LocalDate and LocalDateTime have no zone info, Date and DateTime have zone info.

By debugging I suspect that the mapping of this two classes is right to Date,DateTime not for LocalDate, LocalDatetime.

com.mysema.query.sql.types.LocalDateType
com.mysema.query.sql.types.LocalDateTimeType

Take a look here to the doc.
http://joda-time.sourceforge.net/apidocs/org/joda/time/LocalDate.html

My suggestion is to patch these two classes by removing zone info.
If you are agree I can send to you the patched classes...

(The zone classes should be DateType and DateTimeType according to joda names
but these two classes are already in use, so I don't know....)

@timowest
Copy link
Member

These were changed in this ticket #538

What is your opinion on these arguments http://mnmlst-dvlpr.blogspot.de/2013/11/joda-time-to-jdbc-in-three-hard-lessons.html

@timowest
Copy link
Member

Could you provide maybe a code snippet that shows your problem?

timowest added a commit that referenced this issue Mar 31, 2014
timowest added a commit that referenced this issue Mar 31, 2014
timowest added a commit that referenced this issue Mar 31, 2014
@timowest timowest added the bug label Mar 31, 2014
@dmiorandi
Copy link
Author

Hi Timo,
the article refere to default timezone and use it to convert values, so that the result conversion (i.e. in UTC+1 that is my time) give you something like 01/04/2014 01:00:00.
but in joda LocalDate should be converted always in 01/04/2014 00:00:00 by don't care about timezone, this is by definition of the joda LocalDate so in jdbc type conversion that is something that I'm concerning about.

We are using oracle 11.2.01 with querydsl-sql (no hibernate). The fields on db is of type Date (that is Date + Time with no locale support afaik)

Below the example code. The insertRsaRTariffe method add a date value (from another select, but is the same if I use an insert from a bean with LocalDate) in the right way (01/04/2014 00:00:00), but the select +where filter generate a value 01/04/2014 01:00:00 (see SQL) so
there is no match non filter.

Please feel free to ask me others details.

    public Boolean isRsaRTariffeSovrapposta(Long idStruttura, Long contatore, LocalDate validaARicerca) {
        RsaRTariffe_QSQL    rsaRTariffe_QSQL    = RsaRTariffe_QSQL.rsaRTariffe;

        SQLQuery selectClause    = queryDslJdbcTemplateViaHibernateSession.getQuery().from(rsaRTariffe_QSQL);

        BooleanBuilder            booleanBuilder            = new BooleanBuilder();
        booleanBuilder.and(rsaRTariffe_QSQL.idStruttura.eq(idStruttura).and(rsaRTariffe_QSQL.contatore.ne(contatore)));

        selectClause.where(booleanBuilder);

        BooleanBuilder            booleanBuilder1            = new BooleanBuilder();

        booleanBuilder1.or(rsaRTariffe_QSQL.validaA.isNull());
        booleanBuilder1.or(rsaRTariffe_QSQL.validaA.isNotNull().and(rsaRTariffe_QSQL.validaA.after(validaDaRicerca)));
        selectClause.where(booleanBuilder1);

        selectClause.singleResult(rsaRTariffe_QSQL.idStruttura);

        Boolean result = selectClause.exists();

        return result;
    }   


    @Test
    public void testRsaRTariffeBusinessImpl() {
        Long        idStruttura        = 9L;
        Long        contatore        = 2L;
        Calendar cal2 = Calendar.getInstance();
        cal.set(2014, 1, 1);
        LocalDate    validaARicerca    = new LocalDate(cal2);
        rsaRTariffeBusinessImpl.isRsaRTariffeSovrapposta(idStruttura, contatore, validaARicerca);
    }
  @Override
    @Transactional(readOnly = false, propagation = Propagation.REQUIRED)
    public Long insertRsaRTariffe(RsaRTariffe rsaRTariffe) {
        RsaRTariffe_QSQL    rsaRTariffe_QSQL1    = new RsaRTariffe_QSQL("rsaRTariffe_QSQL1");

        SQLQuery selectClause = queryDslJdbcTemplateViaHibernateSession.getQuery()
                .from(rsaRTariffe_QSQL1)
                .where(rsaRTariffe_QSQL1.idStruttura.eq(rsaRTariffe.getIdStruttura()));

        Long result = selectClause.uniqueResult(rsaRTariffe_QSQL1.contatore.max());

        rsaRTariffe.setContatore(result + 1);
        rsaRTariffe.setPostiLetto(0L);
        RsaRTariffe_QSQL    rsaRTariffe_QSQL    = RsaRTariffe_QSQL.rsaRTariffe;
        SQLInsertClause        insertClause        = queryDslJdbcTemplateViaHibernateSession.insertSql(rsaRTariffe_QSQL).populate(rsaRTariffe,DefaultMapper.WITH_NULL_BINDINGS);

        Long                rows                = insertClause.execute();
        return rows;
    }
SELECT *
FROM (
    SELECT 1
    FROM RSA_R_TARIFFE RSA_R_TARIFFE
    WHERE RSA_R_TARIFFE.ID_STRUTTURA = 9
        AND RSA_R_TARIFFE.CONTATORE != 2
        AND (
            RSA_R_TARIFFE.VALIDA_A IS NULL
            OR RSA_R_TARIFFE.VALIDA_A IS NOT NULL
            AND RSA_R_TARIFFE.VALIDA_A > to_date('02/01/2013 01:00:00', 'mm/dd/yyyy hh24:mi:ss')
            AND RSA_R_TARIFFE.VALIDA_A = to_date('02/01/2013 01:00:00', 'mm/dd/yyyy hh24:mi:ss')
            )
    )
WHERE rownum <= 1

@timowest
Copy link
Member

timowest commented Apr 1, 2014

@oehme Could you also comment on the proposed changes to LocalDate and LocalDateTime serialization in Querydsl SQL?

It appears that the enforced UTC conversions don't work well if timezoneless date representation is used in the database.

@oehme
Copy link
Contributor

oehme commented Apr 2, 2014

The fact that inserts work for @dmiorandi shows that the Joda handlers are working fine.

There is always a conversion. The problem is that the JDBC API uses java.util.Date. This is an absolute value (what Joda calls an Instant). So while both the LocalDate and your database column have no zone information, JDBC insists on using an Instant. It goes like this:

Joda ==(+Zone)==> java.util.Date ==(-Zone)==> Database

The only thing I did was specify a zone that doesn't have DST and all its problems. If you don't do that, you end up with the system default zone. It should not have any impact on the query result, as it's a NoOp.

I don't quite get the SQL statement posted above. This

AND RSA_R_TARIFFE.VALIDA_A > to_date('02/01/2013 01:00:00', 'mm/dd/yyyy hh24:mi:ss')
AND RSA_R_TARIFFE.VALIDA_A = to_date('02/01/2013 01:00:00', 'mm/dd/yyyy hh24:mi:ss')

will always return false, it cannot be greater and equal at the same time. Also, how did the literal strings get in there? Shouldn't this be a prepared statement? Maybe there is some java.util.Date.toString() going on in the code? That would definitely lead to such results, because that uses the system default zone.

@oehme
Copy link
Contributor

oehme commented Apr 2, 2014

Okay, I have found where dates are converted to literal Strings

What confuses me is the fact that @dmiorandi passes in a LocalDate, but it seems to be formatted as a DateTime. So maybe the method I found is not the one that's used? Or maybe @dmiorandi uses an older version of QueryDSL. The method above looks fine and should produce the correct results.

This was edited, I first thought I found the problem in @dmiorandi test code, but it should actually work.

@timowest
Copy link
Member

timowest commented Apr 2, 2014

@oehme Thanks for your comments

Concerning the LocalDateType I wonder whether this logic is valid

st.setDate(startIndex, new Date(value.toDateTimeAtStartOfDay(DateTimeZone.UTC).getMillis()), utc());

The conversion of LocalDate to an instant should be probably to the start of day of the default timezone like this

st.setDate(startIndex, new Date(value.toDateTimeAtStartOfDay().getMillis()), utc());

For LocalDateTimeType and LocalTimeType there don't seem to comparable issues.

@dmiorandi
Copy link
Author

Hi @oehme,
I've made the change you suggest, but the result is the same.
I'm always concerned about com.mysema.query.sql.types.LocalDateType class on setValue method.
The setValue always set DefaultTimeZone imho there is why I can't obtain a midnight Value.
The result is always 01:00 cause I'm GMT+1.
I've reproduced the setValue in a test below. If I use an empty constructor the behavior is as expected.
As expected I mean if i pass a LocalDate as parameter a midnight value is generated in sql.

So all in all, I definitely agree with Timo about solution proposal in LocalDateType class.

    @Test
    public void testRsaRTariffeBusinessImpl() {
        Long        idStruttura     = 9L;
        Long        contatore       = 2L;
        LocalDate   validaDaRicerca = new LocalDate(2013,2,1);
        rsaRTariffeBusinessImpl.isRsaRTariffeSovrapposta(idStruttura, 
                                                            contatore, 
                                                            validaDaRicerca, 
                                                            null);
    }
select * from ( select 1 from RSA_R_TARIFFE RSA_R_TARIFFE where RSA_R_TARIFFE.ID_STRUTTURA 
= 9 and RSA_R_TARIFFE.CONTATORE != 2 and (RSA_R_TARIFFE.VALIDA_A is null or RSA_R_TARIFFE.VALIDA_A 
is not null and RSA_R_TARIFFE.VALIDA_A > to_date('01/01/2013 01:00:00', 'mm/dd/yyyy hh24:mi:ss') 
and RSA_R_TARIFFE.VALIDA_A = to_date('01/01/2013 01:00:00', 'mm/dd/yyyy hh24:mi:ss')) ) where 
rownum <= 1 
    @Test
    public void testJoda() {
        Calendar cal = Calendar.getInstance();
        cal.set(2013, 0, 1);
        LocalDate   byCalendar  = new LocalDate(cal);
        LocalDate   direct  = new LocalDate(2013,1,1);

        System.out.println(new Date(byCalendar.toDateTimeAtStartOfDay(DateTimeZone.UTC).getMillis()));
        System.out.println(new Date(direct.toDateTimeAtStartOfDay(DateTimeZone.UTC).getMillis()));
        System.out.println(new Date(byCalendar.toDateTimeAtStartOfDay().getMillis()));
        System.out.println(new Date(direct.toDateTimeAtStartOfDay().getMillis()));

    }   

@oehme
Copy link
Contributor

oehme commented Apr 2, 2014

@timowest As I said, it does not matter which zone you use, it will always be a NoOp because it is converted to the timezone and immediately back. Also, @dmiorandi already said that selecting and inserting again works fine, so the type handlers are correct.

This has to be caused by some other place in the code where Dates are converted to Strings. The fact that a full date-time String is generated even though @dmiorandi is passing only a LocalDate is highly suspicious to me. What version of QueryDSL are you using? Have you checked that it works when you change the two type handlers? Or was that just a guess?

@dmiorandi Your println statements use the toString method of java.util.Date, which uses the current time zone for formatting. This causes you to see +1h.

@timowest
Copy link
Member

timowest commented Apr 2, 2014

@oehme It's not a NoOp, the start of day conversion creates different timestamps depending on which timezone is used. The conversion is symmetrical, but the behaviour is semantically not correct.

Try this

@Test
public void test() {
    LocalDate localDate = new LocalDate(2013, 1, 1);
    DateTime dateTime1 = localDate.toDateTimeAtStartOfDay();
    DateTime dateTime2 = localDate.toDateTimeAtStartOfDay(DateTimeZone.UTC);
    assertEquals(dateTime1, dateTime2);
}

Output

java.lang.AssertionError: 
Expected :2013-01-01T00:00:00.000+02:00
Actual   :2013-01-01T00:00:00.000Z

@oehme
Copy link
Contributor

oehme commented Apr 2, 2014

@timowest The combination of converting with UTC and then telling the JDBC driver to use UTC is a NoOp. Of course the intermediate object is not the same, but it does not matter.

I've been using these handlers in a big project for 2 years now (also on Oracle) and they work fine. Also @dmiorandi said that select+insert works. Just searching with ".after()" does not work, as that does not seem to use the type handler, but some string manipulation instead.

@timowest
Copy link
Member

timowest commented Apr 2, 2014

@oehme You might be right. I will prepare more DB tests to verify how the JDBC level transformation actually works.

@oehme
Copy link
Contributor

oehme commented Apr 2, 2014

Could you first look whether ".after(LocalDate date)" uses this method for serialization? I don't think the type handler is used at all there.

@dmiorandi
Copy link
Author

To be more focused when I sad insert is right i was wrong. I've got GMT+1 also in insert. Take a look here

            LocalDate direct = new LocalDate(2013, 1, 1);
        RsaRTariffe rsa= new RsaRTariffe();
        rsa.setAccreditataA(direct);
        rsa.setAccreditataDa(direct);
        rsa.setContatore(1L);
        rsa.setIdStruttura(1L);
        RsaRTariffe_QSQL    rsaRTariffe_QSQL1   = new RsaRTariffe_QSQL("rsaRTariffe_QSQL1");
        SQLInsertClause insertClause =  queryDslJdbcTemplateViaHibernateSession.insertSql(rsaRTariffe_QSQL1).populate(rsa,DefaultMapper.WITH_NULL_BINDINGS);
        insertClause.execute();
insert into RSA_R_TARIFFE (VALIDA_A, ACCREDITATA_A, TARIFFA_LIV_2, TARIFFA_LIV_1, VALIDA_DA, POSTI_LETTO_LIV2, POSTI_LETTO_LIV3, CONTATORE, POSTI_LETTO, ID_STRUTTURA, ACCREDITATA_DA, TARIFFA_LIV_3)
values (NULL, to_date('01/01/2013 01:00:00', 'mm/dd/yyyy hh24:mi:ss'), NULL, NULL, NULL, NULL, NULL, 1, NULL, 1, to_date('01/01/2013 01:00:00', 'mm/dd/yyyy hh24:mi:ss'), NULL)

So is true that the behavior is symmetrical like @oehme sad but the mapping on joda types is wrong. This behavior should be associated to joda Date and joda DateTime that have timezone inside not to joda LocalDate and joda LocalDateTime that have no timezone by definition. This is why I agree with @timowest patch.

I know: joda class name is not so clear....

To put it in a nutshell this should be the joda mapping (ignoring the fact some class already have that names...)

joda LocalDate==>LocalDateType (always midnight, no timezone, use empty constructor)
joda LocalDateTime==>LocalDateTimeType (always midnight, no timezone, use empty constructor)
joda Date==>DateType (take care about timezone, like actually in LocalDateType)
joda DateTime==>DateTimeType (take care about timezone, like actually in LocalDateTimeType)

@timowest
Copy link
Member

timowest commented Apr 2, 2014

@dmiorandi Are the to_date('01/01/2013 01:00:00', 'mm/dd/yyyy hh24:mi:ss') literals generated by Querydsl or shown by your JDBC driver?

I will look later today into this issue again.

@oehme By default Querydsl uses prepared statement bindings for all constants, @dmiorandi might be able to answer where the literals come from.

@oehme
Copy link
Contributor

oehme commented Apr 2, 2014

@dmiorandi If you use the default constructor, then it will use your system default time zone. It always uses a time zone because it has to convert from the LocalDate (a relative time) to a java.util.Date (an absolute time). There is no way around this, because JDBC only allows java.util.Date as a parameter.

As @timowest requested: Please tell us where those to_date statements are coming from. I definitely don't see them when I use querydsl-sql.

@dmiorandi
Copy link
Author

By jdbc driver. I'm using a jdbc proxy driver to printout sql as I don't know how to printout query sql with parameters values in querydsl directly.

net.sf.log4jdbc.DriverSpy
jdbc:log4jdbc:jtds:sqlserver://192.168.30.244;DatabaseName=ClesiusICEF_PREPROD

If I need to generate from querydsl with no jdbc interference how can I achieve this?

@dmiorandi
Copy link
Author

@oehme about default constructor using, if I've made a LocalDateType patched using empty constructor and it works like I excpect (always Midnight I'm GMT+1), so I'm confused about this.

    @Override
    public void setValue(PreparedStatement st, int startIndex, LocalDate value) throws SQLException {
        //st.setDate(startIndex, new Date(value.toDateTimeAtStartOfDay(DateTimeZone.UTC).getMillis()), utc());
        st.setDate(startIndex, new Date(value.toDateTimeAtStartOfDay().getMillis()), utc());
    }

@oehme
Copy link
Contributor

oehme commented Apr 2, 2014

@dmiorandi The code you posted should never work, because it is asymmetric (use default zone first, then utc). I strongly suggest that the DriverSpy is just printing out wrong queries, especially since the "to_date" stuff cannot come from QueryDSL. There is also a DriverSpy bug that supports my suspicions.

I think that the real problem is this:

AND RSA_R_TARIFFE.VALIDA_A > ?
AND RSA_R_TARIFFE.VALIDA_A = ?

The actual date does not matter. This will always be false, it cannot be equal(=) and greater(>) at the same time.

timowest added a commit that referenced this issue Apr 2, 2014
@timowest
Copy link
Member

timowest commented Apr 2, 2014

@oehme & @dmiorandi I can confirm that the current version of LocalDateType seems to be valid. Look at my latest commit for the test. I tested will all databases that Querydsl SQL supports.

I assume that the DriverSpy might also not take the third argument to setDate into account.

@dmiorandi You can enable literal serialization via query.setUseLiterals(true)

@dmiorandi
Copy link
Author

Hi @oehme and @timowest.
About query yes, it's wrong, but I was checking just jdbc output so.........
About jdbc proxy there is the mentioned bug because trying query and verifing on the db result seems to be ok. I've tried insert+select reading the field + select filtering on field (eq and after) and it works. See below.

I've got to offer you a couple of beer :-)

Last question: in Oracle reverse class gen map all OracleDate to joda LocalDate. In wich case oracle should generate LocalDateTime? On Timestamp? So To work with hour I've got to use timestamp? But in oracle time can be in a date field. How about it?

I've seen on mapper these ones
{91=class org.joda.time.LocalDate, 92=class org.joda.time.LocalTime, 93=class org.joda.time.LocalDateTime}

Tks

        Configuration configuration = new Configuration(new OracleTemplates());
        // Registro per le date i tipi joda in modo da poterli usare nelle query
        configuration.register(new DateTimeType());
        configuration.register(new DateType());
        configuration.register(new TimeType());
        configuration.register(new LocalDateTimeType());
        configuration.register(new LocalDateType());
        configuration.register(new LocalTimeType());
TransactionTemplate transactionTemplate = queryDslJdbcTemplateViaHibernateSession
                .getTransactionTemplate();
        transactionTemplate.setPropagationBehavior(Propagation.REQUIRES_NEW
                .value());
        transactionTemplate.execute(new TransactionCallback<Object>() {
            @Override
            public Object doInTransaction(TransactionStatus status) {
                LocalDate   dd  = new LocalDate(2014, 3, 31);               
                RghAnagrafica rghAnagrafica=new RghAnagrafica();
                rghAnagrafica.setCodiceUtente(new BigInteger("9999"));
                rghAnagrafica.setDataNascita(dd);
                RghAnagrafica_QSQL rghAnagrafica_QSQL=RghAnagrafica_QSQL.rghAnagrafica;

                SQLInsertClause query=queryDslJdbcTemplateViaHibernateSession.insertSql(rghAnagrafica_QSQL)
                        .populate(rghAnagrafica);
                        int aa=(int) query.execute();
                return query;
            }
        });

        RghAnagrafica_QSQL rghAnagrafica_QSQL=RghAnagrafica_QSQL.rghAnagrafica;
        LocalDate querySel=queryDslJdbcTemplateViaHibernateSession.getQuery()
                .from(rghAnagrafica_QSQL)
                .where(rghAnagrafica_QSQL.codiceUtente.eq(new BigInteger("9999")))
                .uniqueResult(rghAnagrafica_QSQL.dataNascita); 

        querySel=queryDslJdbcTemplateViaHibernateSession.getQuery()
                .from(rghAnagrafica_QSQL)
                .where(rghAnagrafica_QSQL.dataNascita.after(new LocalDate(2014, 3, 30)))
                .uniqueResult(rghAnagrafica_QSQL.dataNascita); 

@timowest
Copy link
Member

timowest commented Apr 2, 2014

Good we got the LocalDateType issue settled.

How do you do the code generation? Via maven or in Java? I think the DateTime semantics are simpler than LocalDateTime.

If you need to map DateTime to SQL DATE types, than register it like this

configuration.registeri(new DateTimeType(Types.DATE));

@timowest
Copy link
Member

timowest commented Apr 2, 2014

I'll close this now, since the original issue has been settled.

@timowest timowest closed this as completed Apr 2, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants