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

JPQL date methods #30

Open
aVolpe opened this issue Jul 14, 2016 · 5 comments
Open

JPQL date methods #30

aVolpe opened this issue Jul 14, 2016 · 5 comments

Comments

@aVolpe
Copy link

aVolpe commented Jul 14, 2016

In JPQL we can use date functions (see this doc).

In Jinq, we can't use this because new Date() is not recognized, and in the JPQL class there is not functions to handle this case.

What are the steps required to implement something like this? Or this is not in the scope of Jinq?

Thanks for the wonderful library.

@my2iu
Copy link
Owner

my2iu commented Jul 14, 2016

I remember looking at this, but I don't fully remember my reasoning for not including it.

I think it was that JPQL supplied functions for getting the current date/time, but I couldn't find a way to actually manipulate the date/time. As a result, the functions seemed useless to me. So if you wanted to run a query to find all the sales within the last 7 days, I couldn't find a way to subtract 7 days from the current date, so I couldn't figure out how those functions would actually be used in a query.

In order to do useful queries involving the current date/time in JPQL, it seemed like you had to do all the math on the Java side anyway, making it pointless to support the CURRENT_DATE, etc. functions. Jinq does support that. Just pull the date calculation out of the query and pass it in as a query parameter.

Date sevenBefore = Date.valueOf(LocalDate.now().minusDays(7));
salesStream.where( s -> s.date().after(sevenBefore) )

or something like that.

aVolpe added a commit to aVolpe/Jinq that referenced this issue Jul 15, 2016
The CURRENT_TIME, CURRENT_TIMESTAMP, and CURRENT_DATE are added, as part
of the JPQL utility class.

ref my2iu#30
@aVolpe
Copy link
Author

aVolpe commented Jul 15, 2016

@my2iu there are valid cases to user the CURRENT_DATE, etc functions.

I create a pull request, #31 with the implementation of the functions, and a Test to try it, I don't know if this is the best way to approach this with JINQ, but it's a try.

In the class SymbExToColumns exists a assert that can't be validated with the new functions, so I add a XXX and a ugly solution to this, I cannot find a way to check hierarchy with the Type class.

The new API of Java is very convenient, but this is a part of the specification and I think there are cases where the date precision of the database are needed.

One mor time, thanks for your wonderful library.

My TODO is to add the documentation to the changes I made if you like the change.

@my2iu
Copy link
Owner

my2iu commented Jul 15, 2016

Thanks for the code changes.

It seems that the CURRENT_DATE etc. functions are mostly used for updates or for people using proprietary variants of JPA that have been extended with date manipulation functions. These types of things aren't currently supported in Jinq. As such, I'm a little reluctant to include support for it because if people see the functions in the documentation, they might spend a long time trying to use them when they aren't actually useful. They should have instead been spending their time using dates as parameters, which is the proper JPQL way of doing things at the moment.

As such, I'm going to keep your changes around for future use, but I won't merge them in at the moment. I'm going to hold off until

  1. I get around to adding update support to Jinq, which is where having a CURRENT_DATE function would be mostly useful
  2. or when JPA adds support for the new Java 8 time stuff, and I have to do a more thorough reworking of Java 8's time support

@aVolpe
Copy link
Author

aVolpe commented Jul 15, 2016

If I create a pull request with the fix to your test regards the java.sql API you will accept that? These changes are not related to the current_date issue and only fix a minor bug.

As you say, the CURRENT_DATE and related are not useful for the main case of Jinq, the best use case is to create the ranges in java.

Thanks for your library.

@my2iu
Copy link
Owner

my2iu commented Jul 15, 2016

Thanks for finding that bug! I'll actually go fix that myself. I'll try to fix it using more Java 8 stuff because apparently all that old java.sql.Date stuff is just very ambiguous about time zones anyway so should generally be avoided.

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

No branches or pull requests

2 participants