jakartaee / data

Data-API
Apache License 2.0
87 stars 27 forks source link

[Use Case]: JDQL fragments #546

Open gavinking opened 5 months ago

gavinking commented 5 months ago

As a ...

I need to be able to ...

specify JDQL expressions or fragments of JDQL in annotations.

Which enables me to ...

reduce the cognitive dissonance in moving between @Find and @Query.

Additional information

So in thinking through:

I came to the realization that there's a general approach that makes @Find much more flexible, reduces the conceptual gap between @Find and @Query, solves the problem with @OrderBy, and makes the trip to the zoo unnecessary.

[Don't get scared. I'm not proposing to do any of this today, I'm just trying to understand what the future might look like.]

The idea is to have a smaller set of annotations which accept fragments of JDQL. So, for example:

@Find 
@OrderBy("lower(title) desc")
@OrderBy("isbn")
List<Book> booksByTitle(Type type, @Where("title like :pattern") String pattern);

which would be assembled to:

@Query("where title like :pattern and type = :type order by lower(title) desc, isbn")
List<Book> booksByTitle(Type type, String pattern);

You might even be able subsume @Where into @By, since it's pretty easy to distinguish a scalar expression with no parameters from a conditional expression with a parameter. And you might allow @OrderBy to accept a list. So:

@Find 
@OrderBy("lower(title) desc, isbn")
List<Book> booksByTitle(Type type, @By("title like :pattern") String pattern);

Now, one might look at this particular example, and think "but the @Query version is shorter and perhaps even more readable", and that's true because this is a sort-of "extreme" case in the sense that I have three annotations and only two parameters. But if I had four parameters and just one or two annotations, it would look more attractive.

But that's anyway not the point. The point is to reduce the pressure to add lots of special-purpose annotations, and save the developer from getting into the sort of edit war where they find themselves switching back and forth between @Find and @Query.

To reiterate: @Where("title like :pattern") is more verbose than @Like. But it covers many more cases and is somehow, at least to my way of thinking, a much more elegant approach.

Note that the content of these annotations can still be checked at compile time. JDQL has a very a simple expression language that's easy to parse in an annotation processor.

So we would have:

By side effect, @By("id(this)") becomes the much more elegant way to write @By(ID).

Everything here looks backward-compatible with what we have today! That said, we could consider adopting this approach for @OrderBy in this release, if we have time. But it's definitely not critical.

njr-11 commented 5 months ago

JDQL query fragments are something we can consider in the future. They address a very different use case than,

  • the potential for us to grow a whole zoo of similar animals in future (@Like, @IgnoreCase, @In, etc), and

The use case for query fragments is similar to @Query where the user is willing to trade off the risk of errors in string values and the need to learn query language for the benefit of being able to write more complex queries.

The use case for making parameter-based automatic query methods on par with Query by Method Name trades off the ability to have more complex queries for the ability to perform basic operations relying purely on API and API JavaDoc without needing to learn any query language or needing to specify string values that are vulnerable to spelling and other errors.

Another way of having JDQL query fragments might be to include then in the @Query annotation itself rather than introducing new annotations for them,

@Query(where = "title like :pattern and type = :type",
       orderBy = "lower(title) desc, isbn")
List<Book> booksByTitle(Type type, String pattern);

Similarly, having what you refer to as an annotation zoo isn't the only option for the annotative pattern. Another option is for a single annotation, such as @Is, with an enum value to cover everything. For example,

@By(_Product.name) @Is(Like) String pattern,
@By(_Product.category) @Is(In) List<Category> categories,
gavinking commented 5 months ago

The use case for query fragments is similar to @Query where the user is willing to trade off the risk of errors in string values and the need to learn query language for the benefit of being able to write more complex queries. [snip] .... without needing to learn any query language or needing to specify string values that are vulnerable to spelling and other errors.

I mean, that really depends.

In my implementation the tradeoff between @Query and @Find is only a matter of verbosity. Errors in JDQL and JPQL query strings are detected at compilation time, including spelling and even typing errors in field names!

Now, OK, that requires some fairly serious machinery, which I spent some time developing a few years ago, and so I just happened to have it lying around when Jakarta Data came along. And so not all implementations are going to have that. Fine.

But for simpler cases like @By("title like :pattern"), which is really what we're talking about here, any annotation processor should be capable of easily validating that. You don't need my fancy machinery. The expression language of Jakarta Data is really, really simple! You could hand-write a parser for it from scratch in a couple of hours, maybe an hour if you already know what you're doing.

That is to say, @By("title like :pattern") is not much more difficult to validate than @By("title").

Now, I'm not suggesting we can't have @Like, since that's a really super-common one, and @Like is still less verbose than @By("title like :pattern"), I'm just saying it takes away the pressure for this to grow into a whole zoo of these things.

Another way of having JDQL query fragments might be to include then in the @Query annotation itself rather than introducing new annotations for them

Well that wouldn't let me mix JDQL fragments into a @Find method, which is the essential point of this proposal. And it would leave me stuck with @OrderBy(vale="name",descending=true,ignoreCase=true) when I'm writing a @Find method.

What I'm looking for is the ability to mix bits of JDQL into a @Find method. When I'm using @Query, I don't need to break up the query into fragments, and, in fact, it's less readable that way.

Another option is for a single annotation, such as @Is, with an enum value to cover everything. For example,

@By(_Product.name) @Is(Like) String pattern,
@By(_Product.category) @Is(In) List<Category> categories,

This isn't awful; I quite like it, actually. Nice idea!

But on the other hand it sorta undermines the main value of @Find from my point of view which is: (a) its extreme simplicity and (b) its lack of verbosity—and (c) the fact that in the upcoming release of IntelliJ, the IDE will autocomplete it for me. In fact, it's actually slightly more verbose, and a bit harder to type than:

@By("name like :pattern") String pattern,
@By("category in :categories") List<Category> categories,

So, y'know, I'm just not sure I'm sold on the idea of implementing an expression language in annotations.

gavinking commented 5 months ago

If you've never seen it, here, by the way, here's what it looks like when you misspell something or give it the wrong type. It's not the case that JDQL has to be less typesafe than @Find.

Screenshot 2024-03-19 at 3 10 43 PM Screenshot 2024-03-19 at 3 12 16 PM Screenshot 2024-03-19 at 3 13 29 PM
gavinking commented 5 months ago

So I've spent some more time reflecting on this last night and this morning, going back to first principles of what I originally liked about the @Find approach, and what I think is its sweet spot compared to JDQL.

When I first implemented @Find methods in Hibernate, it was pretty clear that you might want to have some way to "adjust" the matching on a per-parameter basis, but that immediately opened up the question of where to draw the line.

I was pretty sure I didn't want to try implementing a whole expression language in annotations. I didn't and still don't want to open the door to the whole open-ended hog of @Contains, @StartsWith, @EndsWith, @LessThan, @LessThanEqual, @GreaterThan, @GreaterThanEqual, @Like, @In, @IgnoreCase, @MemberOf, etc, etc. You might wonder why not, and I guess the answer is simply that I don't think Java annotations are a good language for writing expressions in. Even the above list of ten (!) annotations still covers only a tiny fraction of what's possible in JDQL, let alone JPQL.

On the other hand, it was also pretty clear early on that there was going to be lots of pressure to handle pattern matching (i.e. @Like) and probably also some sort of in condition (i.e. @In). Those things are just really common. But I held off on adding them because it just seemed to open a door I didn't want to open.

But now after a lot of reflection on this, I've realized that there's a perfectly reasonable "ideology" which lets me address those two cases without opening any doors I don't want to open. It's this:

@Find
Book[] booksByIsbn(String[] isbn);

@Find
List<Book> booksByTitle(@Pattern String title);

Notice that there are no "operators" here. In the first case, the type alone requires that this be interpreted as an in condition, and in the second case, an annotation describing the semantics of the string itself, @Pattern, requires that matching is via like. From the right perspective, you can view @Pattern as part of the type declaration.

And that's consistent with the original philosophy that @Find methods are interpreted on the basis of the names and types of their parameters. No need to introduce the notion of an "operator annotation". That's exactly what I was looking for!

So what happens if I do need >? Well, then use JDQL, because it's actually good at expressing these sorts of things via a much more natural syntax than @GreaterThan. And so we arrive at the idea to allow fragments of JDQL to be combined with @Find. I really think that this is the right way forward. With just one or two (rather than ten) new annotations, we can let you easily express stuff as complicated as or even more complicated than:

@Find @Where("deleted = false")
Book[] booksByIsbn(String[] isbn);

@Find @Where("deleted = false")
@OrderBy("lower(title), isbn desc")
List<Book> longBooks(@By("pages > ?1") int minPages, 
                     @By("left(locale, 2) = lower(?2)") String language
                     @Pattern String topic)

That is to say, we can express things which will just never be elegant or even possible using "operator annotations". Now, are these examples obviously better than just writing:

@Query("where deleted = false and isbn in ?1")
Book[] booksByIsbn(String[] isbn);

@Query("where pages > ?1 and left(locale, 2) = lower(?2) and topic like ?3"
       + " and deleted = false" 
       + " order by title, isbn desc")
List<Book> longBooks(int minPages, 
                     String language
                     String topic)

I dunno, I think you could argue that they are slight better, though it's clearly a matter of taste. But what I like above all else is that it unifies @Query and @Find in a pleasing way.

I understand that this looks superficially less typesafe than the "operator annotations". But what I've seen is that it really doesn't need to be. It really is quite straightforward to typecheck these kinds of fragments in an annotation processor. I have been taken a bit by surprise by how well this approach works in practice.

On the other hand, if you're lucky enough to have IntelliJ Ultimate, IntelliJ now does a fantastic job of highlighting/validating/autocompleting JPQL embedded in annotations. Of course, IntelliJ doesn't understand the Jakarta Data annotations yet, it only understands the similar annotations in Hibernate. But the guys working on this stuff have been extremely responsive to my requests in this area, so I expect they'll jump on Jakarta Data. They already have the machinery they need.

I also understand that the reasoning here is maybe a bit hard to digest at first. I'm not suggesting that there's something deeply broken about @GreaterThan. What I'm saying is that it's a sort of dead end that takes you much less interesting places.