-
Notifications
You must be signed in to change notification settings - Fork 30
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
consider alternatives to query language embedded in method names #226
Comments
FWIW Micronaut Data also supports this same syntax it will first try and match the
Work where Book has a title and type property. This doesn't appear to be something that has been considered in the spec as far as I can see, but I guess the spec was always about trying to identify the common functionality between the different implementations of this pattern (Spring Data, Micronaut Data, Apache DeltaSpike etc.). Also note that for more complex queries it is always better to fallback the native querying API of whatever datastore you are using such as a templated JPA-QL query (https://micronaut-projects.github.io/micronaut-data/latest/guide/#hibernateExplicitQueries) or with JPA specification API https://micronaut-projects.github.io/micronaut-data/latest/guide/#hibernateSpecifications |
Nice! It's a much more natural approach (at least in Java). I was not aware that there was prior precedent for it.
Strongly agree. |
Spring Data takes credit for having proposed such ideas first in a widely used implementation, but also I believe it's fairly old now? We should certainly look at it but I believe that after so much time and experience, we should also try to do better nowadays. That's to say that I don't think we necessarily want to stick to a "common denominator" approach, but rather to a critical assesment of what worked well and what could be done even better. So I'd strongly endorse @gavinking and Micronaut Data 's direction to explore alternatives; on the other hand, I'd prefer to see them battle-tested in production for a while before being cast into stone into a Jakarta specification. |
Well Spring Data's original implementation was based on GORM for Hibernate (from Grails, see http://gorm.grails.org/latest/hibernate/manual/index.html) I know since I was involved at the beginning of the Spring Data project and helped define some of the MappingContext/PersistentEntity APIs which were largely lifted from GORM. GORM was the first widely used project on the JVM to support this pattern, but clearly not for Java developers (it was limited to Groovy). GORM itself can't take credit for the idea as all of this is traced back to ActiveRecord and Ruby on Rails 😉 As for doing better sure that can happen, but it would be nice if the spec was actually implementable by existing implementations rather than being something completely new with an untried and untested specification. |
Well, it sounds like this particular feature is already implemented by both Micronaut and Hibernate, so here we have two existence proofs of implementability. |
So, thinking this through a bit further, I came to a realization. According to my current proposal, a repository would have
Currently:
On the other hand, the "Query by Method Name" stuff that is currently in the spec defines some sort of very limited query language which is portable, but unfortunately comes with all the limitations of having to encode the query language into a method name, where you have neither whitespace not punctuation characters to work with, and where the user is not permitted to freely name the operations of their API. (It's really a pain to try and express queries without the use of parentheses or operators like So it struck me that a very reasonable sort of "compromise" solution would be to take something approximately like the BNF Grammar for Query Methods (at least as a starting point) and let you use that mini-query language not as a method name, but inside the So you could write a method like: @Query("find by title like ?1 and publicationDate.year = ?2")
Book bookByTitleAndYear(String title, int year); and have that be portable across all Jakarta Data implementations. (The actual final form of the query language would likely be different, I'm still just thinking out loud here.) Of course this mini-language would have nothing like the power of say SQL or JPQL, but in this context, that's a feature: it's precisely the limited nature of this language that would enable portability between datastores. I anticipate the objection that we would be defining yet another query language here. But, well, the BNF Grammar for Query Methods is already defining such a creature. All I'm proposing is to move that language into strings, where it has much more "room to grow" into something more meaningful. |
Note that at first you would not be able to do much more with this mini query language than what you can do with "automatic" query methods, but you can do some extra things, and the mini language, if successful, would be enhanced over time. |
A few of the other spec participants and myself had some of these same thoughts earlier this year, in response to which we had written up #109 to consider if Query by Method Name could be expressed with a combination of basic method annotations. It was only in draft form and needed more work, but unfortunately it led to such a heated debate that we had to defer even thinking about it to a future spec version. In the following version I would like to see us revisit some of those ideas along with what you have proposed here. |
See section 4.3 of the spec. A portable query language for Jakarta Data is being proposed in #458. So I'm closing this issue as complete. |
Section 2.3.2 defines "query by method". Well, "defines" is probably too strong. Let's say it lists a number of "keywords" that you can use in method names, and apparently they do stuff. This doesn't look to me like anything approaching a proper specification of a query language, but I don't really want to pull on that thread right now, because I don't think Jakarta Data should even have this feature.
You see, the idea of embedding a query language in method names is an idea that Spring Data stole from Ruby, and it's a terrible idea. (Ruby is Ruby, a dynamically-typed language. Java is not Ruby.)
For anything not entirely trivial, this approach leads to completely unnatural method names: for example, from the Jakarta Data spec,
findByYearRetiredNull
,findByNameOrderByAgeAscNameDescYearAsc
. Ugh. [There are much worse examples in the documentation for Spring Data.]Now, in Hibernate 6, after literally years of carefully considering the problem, we introduced a much cleaner and simpler approach which you can read about here.
The idea is that one writes a method signature like this:
The return type of the method determines the entity to be queried, and the names of the method parameters determine the fields of the entity that we're querying. The name of the method itself can be anything you like, and has no semantics. [The annotation could even be made optional.]
In Hibernate, the names and types of the parameters are checked against the entity at compile time so you see errors immediately in your IDE, but one could of course implement this idea in a worse way, using runtime reflection or whatever, so that developers would find out about the errors at runtime.
Now, of course this approach doesn't allow for many sorts of queries. It's quite difficult to (ab)use it to express more complicated things. But this is actually a feature. By having a natural limitation to just the simplest cases we naturally guide users to use
@Query
to express more complicated queries, and that's exactly the guidance we want to give them.Because, even if we don't love embedding queries in string literals, at least a string literal can have whitespace, and punctuation characters like parentheses and operators.
Well, OK, OK, we could in principle let you abuse it to write stuff like:
which reads a lot better than:
but I'm resisting this idea for now, because I want users to use
@Query
.I think that Jakarta Data should switch lanes here and follow this new approach.
The text was updated successfully, but these errors were encountered: