Bug 535431 - Fix support of JSR-310 date/time types with PostgreSQL

[jpsyri]

Fix for https://bugs.eclipse.org/bugs/show_bug.cgi?id=535431

This should probably include some kind of test, but I'm not sure where it should be located. There's no much point testing the changes against non-PostgreSQL server and I couldn't easily locate Postgres specific tests. Additionally at least on my own environment, I have problems running test-lrg against Postgres even without changes, so I'm not sure if tests are supposed to work with Postgres.

If anyone can provide guidance how tests should be implemented, I can add those to this request.

[IAmHopp]

Shouldn't this be done for all Platform files?

[jpsyri]

According to discussion on bug report this should already exists in Oracle9Platform, although I myself can't find it there. MySQLPlatform already seems to have similar code. For others I'm guessing something similar might be required for support, but I don't understand enough to know for sure. (I also don't know if on some of the other platforms would require platform-specific type mappings).

[IAmHopp]

I see.

In my opinion, what you're doing here is important. Not only for PostgreSQL, but equally across all vendors. I know I myself miss support for these types on SQL Server and, looking at the file, the work you did on PostgreSQLPlatform would be analogous to the work that'd have to be done on SQLServerPlatform. Namely, finding buildFieldTypes, adding lines for the new classes and basing the column names on the pre-existing mappings for the old date types.

It seems doable and I believe the advancements you'll make with the PostgreSQL integration by doing this shouldn't be restricted to it, as you're solving a problem that exists with other integrations as well.

Just my two cents.

[jpsyri]

I certainly agree that this is something that should also be fixed on the other platforms and I feel your pain; the reason for me making this request is that I myself encountered this problem with postgres.

My problem is that I'm basically just some random guy with no previous experience on the eclipse-link code, so I'm very hesitant to do any changes where I can't check effects of those changes. I don't have easy access to most of those other systems to do testing, so I couldn't test those myself and, as far as I can see, they might not be tested on any automatic manner either.

[IAmHopp]

as far as I can see, they might not be tested on any automatic manner either.

Someone from the team will have to review and approve this, though, so I think you're covered there. Whoever jumps in here to review your code is exactly the expert on the EclipseLink code you're looking for to make sure nothing goes wrong. The approval process itself will take care of preventing the damage you're worried about doing.

I understand what you're saying, though. Thanks for taking the time to reply either way. I hope you'll reconsider.

[jpsyri]

It seems that #415 is doing basically the same thing, but perhaps more generically

[Flugtiger]

Hi, in #415 I wanted to fix a bug that prevents NULL to be stored in a column mapped to a java.time class. This might be related to bug 535431. (First I even thought both bugs are identical but later I decided against it and opened a new one.) Maybe we should write a test for it and see if #415 already fixes it.

[jpsyri]

I think those two bugs are at least very closely related. Your versions seems like it would also apply to other platforms that Postgres. As @IAmHopp above pointed out, the null-saving problem seems to exist also on SQL Server, and your fix would probably fix also that. I think only thing that would need to be checked is if your fix produces proper data types on auto-generated schemas.

I couldn't myself get the existsing tests to pass on Postgres even without any changes, so I'm not sure if we have a good way to add test that would conclusively test the fix on Postgres (or SQL Server).