-
Notifications
You must be signed in to change notification settings - Fork 31
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
change '#id' to 'id(this)' #587
Conversation
since Lukas just merged my proposed change to JPQL which introduced id() and version() functions
A question I have for you guys is: should we add |
*/ | ||
String ID = "#id"; | ||
String ID = "id(this)"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we have this function included before?
I mean, if the goal is to have safely contained specifications, or in this case, it is OK?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we have this function included before?
WDYM?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Checking the Note:
Note that {@code id(this)} is the expression in JPQL for the
* unique identifier of an entity with an implicit identification
* variable.
It is a JPQL function and not a Jakarta Data Query Language function.
I remember that you taught us that the specification should be self-contained. That is, the vendor should read only this specification and understand the behavior of Jakarta Data.
We have included this JPQL function right now by note.
Should we also include this function in our query instead of referring to this JPQL query? Or is it acceptable in this case?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah well that's why I asked:
A question I have for you guys is: should we add
id(this)
andversion(this)
as legal expressions to JPQL?
It's not critical in this case (or at least not yet) because right now this is just a magical constant string value.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
at least not yet
Obviously I'm thinking toward the future here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems like a good forward thinking thing to do. We don't currently specify that functions can be supplied to Sort
or OrderBy
, but should we ever decide to add that, this would be consistent with it.
It seems like that could be useful, but it should be fine to wait on that until next version. |
I thought of another use for @Query("WHERE id(this) IN ?1 ORDER BY id(this) ASC")
List<T> findByIdIn(Iterable<K> ids); |
That is correct. Indeed, as far as I recall, this was the real reason I originally wanted it. |
Since Lukas just merged my proposed change to JPQL which introduced
id()
andversion()
functions.