-
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
Ability to define query restrictions dynamically #829
Comments
_Product.price.greaterThan(min)
.and(_Product.price.lessThan(max))
.and(_Product.color.equal(BLUE)
.or(_Product.color.equal(GREEN))), This to me is highly ambiguous. Conventionally, |
For dynamically building the query criteria, I would like to consider the JPAStreamer and DotNet Core Entity Framework as references. An example of JPASteamer home page, https://jpastreamer.org/ jpaStreamer.stream(User.class)
.filter(User$.age.equal("22"))
.sorted(User$.name.reversed())
.skip(10)
.limit(5)
.forEach(System.out::println); Especially since Java has a great Use generated metamodels to assist in implementing the The Repository concept is a container for a collection of entities, should be streamable, and can be converted to Stream seamlessly. products.stream().filter(Product$.name.like("").and(...))
.map(result -> another POJO)
.collect()/forEach/toList In the implementation, translate it and execute it on the DB driver API level(Jdbc or NOSQL drivers). |
@hantsy JPAstreamer is very cute and I don't hate it at all. It does indeed have a sort-of approximately similar scope to JDQL, in that it looks like it works well for queries with no joins or cartesian products. So it looks like in principle you could establish a welldefined mapping between the two things. But on the other hand I'm not sure how cleanly it really fits with the notion of repositories and query methods. It's a sort-of different paradigm. If you want to use JPAstreamer for your queries, I'm not clear on why you would want to have a repository to begin with. Anyway, I think it's something we can discuss. If we're already proposing to introduce restrictions like My main concern that we would maybe end up with a worse JPAstreamer with no support for e.g. |
Signed-off-by: Nathan Rauh <[email protected]>
Signed-off-by: Nathan Rauh <[email protected]>
Signed-off-by: Nathan Rauh <[email protected]>
Signed-off-by: Nathan Rauh <[email protected]>
Signed-off-by: Nathan Rauh <[email protected]>
Signed-off-by: Nathan Rauh <[email protected]>
So I finally found some time to think more clearly about this stuff, and to spend some time designing a restrictions API for Hibernate 7. Note that our implementation of Jakarta Data works by generating very trivial code that just calls public APIs of Hibernate, and so if Data has restrictions, then Hibernate needs something equivalent. What I came up with is something that to me feels extremely simple and elegant, and rather powerful.
Now, the lovely thing about this is that a repository method parameter may accept:
So I could write: @Find List<Book> books(Restriction<Book> restriction); and call it like: List<Book> books = library.books(Restriction.startsWith(Book_.title, "Jakarta"); List<Book> books = library.books(Restriction.like(Book_.title, "Hibernate%"); List<Book> books = library.books(Restriction.in(Book_.id, List.of(1, 2, 3)); Or I can write: @Find List<Book> books(Range<String> title); and call it like: List<Book> books = library.books(Range.prefix("Jakarta"); List<Book> books = library.books(Range.pattern("Hibernate%"); List<Book> books = library.books(Range.valueList(List.of(1, 2, 3)); Notice that there's not a single annotation anywhere. Nor is there any construct representing any kind of operator! There's no This solution is extremely expressive, highly customizable, extremely programmable, and completely type-safe. It's also a really quite tiny amount of code, since I just made Anyway, Jakarta Data doesn't have to go down this path, but I'm throwing it out there as an option. I think it's both simpler and more powerful than what we have right now. |
I think the first pattern,
I got confused by the above combination, but I think the latter one is just a typo and you meant to have String values in the list to match up with the
Doing this without an annotation worked because it assumes In any case, the usefulness of this pattern with Range/EmptyRange/FullRange/Interval/LowerBound/UpperBound/NotNull/Pattern/Value/ValueList/CaseInsensitiveValue seems to be when the repository method author wants to enforce that the repository user must add query restrictions on specific predefined entity attributes (the names encoded in to the method parameter names) instead of any combinations of restrictions on entity attributes that the repository user wants. For example, a repository can have a method that requires its user to query on restrictions of the book title and length, @Find List<Book> booksWithTitleAndLength(Range<String> title, Range<Integer> length);
...
List<Book> books = library.booksWithTitleAndLength(Range.containing("Jakarta EE"),
Range.closed(100, 500)); But if they want restrictions on a different combination, like None of what I have stated above is arguing for or against the proposal. It is only exploring it at this point. |
When you want completely generic restrictions, it's exactly the same from the point of view of the user. But from the point of view of of how this is modeled and represented and how it's implemented in Java code it is, in my opinion, significantly more elegant than what we have right now.
Yes, sorry, that last example is wrong, of course.
Yes of course. The idea is that:
Note that those are all package-private implementation classes, I don't think I said that explicitly.
Correct. And we know 100% for sure that this is something people often want to do:
Compare: @Find
List<Records> records(long tenant, Range<LocalDate> creationDate); with something like: @Find
List<Records> records(long tenant);
@Find
List<Records> records(long tenant,
@By(_Record.CREATION_DATE) @Is(GREATER_THAN_OR_EQUAL) LocalDate startDate);
@Find
List<Records> records(long tenant,
@By(_Record.CREATION_DATE) @Is(LESS_THAN_OR_EQUAL) LocalDate endDate);
@Find
List<Records> records(long tenant,
@By(_Record.CREATION_DATE) @Is(LESS_THAN_OR_EQUAL) LocalDate startDate,
@By(_Record.CREATION_DATE) @Is(GREATER_THAN_OR_EQUAL) LocalDate endDate); The solution with Now, of course, you might object that "well, nobody would write code like that, they would just use
Now, I get it, these benefits might not seem all that compelling to you (though to me I find them quite convincing). And so if I were proposing a more complex solution, I can imagine that some people might think it just not worth the extra complexity. But I'm emphasizing here that this proposal is fundamentally simpler. You get this extra flexibility for negative cost. |
I saw that when I was trying write an example. I think that works for your implementation where you are interpreting them internally and able to return JPA Predicate off of your API. In a spec where the API is separate from a provider implementation, there would still need to be a way for the provider to obtain from the Range (or Restriction) information about what it represents. Currently, we are using
This point is misleading. It implies that the pattern that uses "metacharacters" does not offer library.books(_Book.title.startsWith("Jakarta EE"));
library.books(_Book.title.contains("Jakarta EE")); versus the Range proposal, library.booksTitled(Range.startsWith("Jakarta EE"));
library.booksTitled(Range.contains("Jakarta EE")); Your point here would be more valid stating a user would like to use
This one I don't agree with at all. You have switched from comparing Range: @Find
List<Record> records(long tenant, Range<LocalDate> creationDate);
tenantInfo.records(tenantId, Range.greaterThanOrEqual(date));
tenantInfo.records(tenantId, Range.lessThanOrEqual(date));
tenantInfo.records(tenantId, Range.closed(firstDate, lastDate));
tenantInfo.records(tenantId, Range.full(LocalDate.class)); Restriction: @Find
List<Record> records(long tenant, Restriction<Record> restrictions);
tenantInfo.records(tenantId, _Record.creationDate.greaterThanEqual(date));
tenantInfo.records(tenantId, _Record.creationDate.lessThanEqual(date));
tenantInfo.records(tenantId, _Record.creationDate.between(firstDate, lastDate));
tenantInfo.records(tenantId, null); These are about the same. In both cases, I would say that ignoring/skipping/full-ranging the Range or Restriction parameter is awkward and we should look into better ways of allowing that if needed. Probably there should be a Restrict.ignore() or something with a better name rather than passing in null, but we haven't added that yet. If we go further with the example where the user wants to add restrictions on more entity attributes, then the greater flexibility of the Restriction pattern gives an advantage, tenantInfo.records(tenantId, Restrict.all(_Record.description.notNull(),
_Record.description.contains(searchText),
_Record.creationDate.greaterThanEqual(date))); With Restriction, the same repository method can be reused for flexible search criteria, whereas Range will require additional repository methods for every different combination of entity attributes possible in flexible search criteria. This is not to say we can't have Range as well. It isn't a replacement for Restriction and it isn't a replacement for It should be noted that it might be an option for someone who doesn't want to type a static metamodel class to do a static import, and then you have, tenantInfo.records(tenantId, creationDate.greaterThanEqual(date)); but that won't be desirable in all cases, especially if there are multiple entity classes to use with an overlap in attribute names. |
Yes, right, those types might need to be exposed as an SPI. In fact it's just about a perfect use-case for
You're misunderstanding, what I'm saying is that:
What I'm saying is that
Correct. It's a replacement for |
The "competition" here is between: @Find
List<Records> records(long tenant,
@By(_Record.CREATION_DATE) @Is(LESS_THAN_OR_EQUAL) LocalDate startDate,
@By(_Record.CREATION_DATE) @Is(GREATER_THAN_OR_EQUAL) LocalDate endDate); And: @Find
List<Records> records(long tenant, Range<LocalDate> creationDate); But by providing |
(One of my favorite tricks is to provide very simple but generic primitives and build up more complex higher-semantic-level things out of them. |
I don't know why there is discussion of If you want to discuss The patterns for |
OK, Nathan, stop. Take a step back. I'm asking you to clear your mind of your preconceptions and of the way you have been thinking about these problems up until now. In your head there are two distinct things here, and they call for completely separate solutions. I fundamentally disagree with this. And so by insisting that we conceptualize these things separately, you're asking me to concede the main part of my argument. Not so fast! I've been trying to push you guys in the direction of thinking that even though there might sorta be two things, that they can actually be solved more elegantly by one more unified thing. I know you like The point is that |
And of course, if I'm wrong and you're right, and we ultimately figure out that What's not so easy is to retrofit |
When asked to consider |
What possible legitimate purpose might I, as a repository author, have in wanting to restrict the kind of comparison that the client can perform?
Why would I as a repository author possibly want to let the client restrict a date by I mean, sure, previous inferior repository frameworks did force people to do it that way, and so we've all certainly seen lots of examples like that. But that doesn't mean there's a good reason to do it like that, or that it's what people would choose to do if we gave them the choice. But perhaps I'm wrong. Perhaps you can think of some motivation that I've not thought of? Anyway, this is great. Now we've really arrived at the root of the disagreement. I was struggling to see where the disconnect was. Now I see it. |
Consider: var books = library.byTitle(like("%Hibernate%")); // (1) var books = library.byTitle(contains("Hibernate")); // (2) These are not in my opinion more generic, nor lacking in clear intent than: var books = library.byTitleLike("%Hibernate%"); // (3) In fact, examples (1) and (2) express the intent much more clearly, since they express it through the type system. If I didn't add Similarly: var records = archive.byCreationDate(greaterThan(startDate)); // (4) Expresses its intent, more clearly, in my opinion, than: var records = archive.byCreationDateAfter(startDate); // (5) Again, the type system forces me to express the intent clearly, instead of me having to name my method carefully. Hell, we (or the user) can even add trivial convenience methods for var records = archive.byCreationDate(after(startDate)); |
The following example was in the other issue, mentioned earlier, overdueBooks = library.booksDueBefore(today); I can quickly think of others like, children = people.children(maxChildAge); potentialDrivers = people.ofDrivingAge(minDriverAge); candidatesForBonus = employees.highPerformers(minRating, Set.of(Role.SOFTWARE_ENGINEER,
Role.SOFTWARE_DEVELOPER,
Role.SOFTWARE_ARCHITECT)); Although I haven't been recording examples that I see, I've run into enough of them to know the pattern is quite useful, and would pretty much always be the starting point I would take when writing a repository method of my own: First, what do I know of for restrictions and sort criteria that cannot change? If there is any, then encode it into the method upfront. I would say that Query by Method Name actually was actually working against this pattern by forcing less meaningful names that become exponentially more cumbersome to use as the number of restrictions increases linearly and thus discouraging use. Moving away from that and being able to use concise names that are meaningful to the business logic will driver more usage, which will be good because it should be encouraged as a best practice for the repository author to encode upfront everything that cannot change, sparing the repository user from needing to do extra work of figuring out how to use a more generally named method with which Ranges to wrap their values in to get what they really wanted. I know you'll argue against some of my simple examples: Why not make them more flexible with Range (or even Restriction) and the repository users can do whatever they want with them? In cases, where that is what you want, then yes, go for it. But in cases where there are specific business requirements and usage patterns, the repository author should be able to take advantage of encoding them and exposing things to the repository user more precisely and meaningfully in terms of the model that they have. |
That's ... um, well, yeah ... pretty much exactly what I think, actually. I'm kind of a big fan of abstraction, reusability, and composabilty.
I do agree with this. But from this point of view
This is the bit where you lose me. I don't really understand what you're trying to achieve here. Uncharitably, it seems your goal here is to deliberately make your code less generic than it could be. That's, like, the exact opposite of everything I'm aiming for when I write a program . |
I guess the point is that the reason I changed my mind about repositories is NOT that I liked the fact that I was trading a single generic On the contrary, that's what I still don't like about repositories. (For the exact same reason that I don't write The reason I changed my mind about repositories was because we figured out a way to achieve a level of typesafety that was impossible with |
If you all could humor the following question: For my own understanding, does the following example illustrate the type of problem the API would be designed to address? Assume that the data model for this example GUI pane is comprised of multiple tables.
|
@montanajava Yes, that is an excellent example. In Data 1.0, the only part of this that was possible was the Sorting. This issue/feature will make everything else in your example possible, except Group By. I'm not sure I properly understand the meaning of Group By in this example. If it is only meant as a higher level of sorting, then it would be already possible as well, by using Jakarta Data's Sort. If it is anything beyond that, then it's a feature we don't have and isn't solved by this issue. In that case, open a new issue for discussion of it and to go over the use case in more detail. |
@njr-11 Thanks for your reply. A higher level of sorting is indeed the most common use case I had in mind. While there are arguments for including "group by", I agree it would be best to create a separate issue for that. I'm really glad to see you confirm the problem domain this issue addresses! It tackles a real-world need, I can assure you. |
Besides the fluent APIs to assemble the query criteria, we can port the |
Big -1. We had this in Hibernate for many, many years, long before any of those other things you mention existed and:
This doesn't belong in the Data spec. If you like, it's fairly trivial to build your own query by example facility using the |
Signed-off-by: Nathan Rauh <[email protected]>
Replaces #825
Users will sometimes need to define queries dynamically instead of statically in response to what they see happen at run time. For example, a user specifies various customizable search options and in response, the application dynamically adds restrictions to a query to filter for the matching results.
An application ought to be able to define the same kind of query restrictions dynamically that it can already define statically (with patterns like Query by Method Name).
Query restrictions should make use of the Jakarta Data Static Metamodel and generally be agnostic to the database as much as possible, other than the same sort of limitations that Query By Method Name and the other static patterns have on which operations certain classes of NoSQL databases are incapable of.
The Jakarta Data Static Metamodel will be expanded to represent conditions that are possible per attribute type. These conditions can be supplied as an extra repository method parameter, similar to what is currently possible with Order.by/Sorts.
Example with one static and one dynamic restriction:
For multiple dynamic restrictions,
One possible way suggested by Otavio in 825
Another possible way (slightly more wordy, possibly more readable)
Or we could think of another way to cover and/or/not. That part can be figured out later. The basic idea here will be to add the restrictions to the static metamodel and decide how type safe to make them. For example, requiring
Number
for numeric attributes, or possibly more precise, or just Object for everything, ...The text was updated successfully, but these errors were encountered: