Blazebit / blaze-persistence

Rich Criteria API for JPA providers
https://persistence.blazebit.com
Apache License 2.0
741 stars 90 forks source link

Which CriteriaBuilder can work with EntityView? #1954

Open guofengzh opened 1 week ago

guofengzh commented 1 week ago

For issue issues 1943, as @jwgmeligmeyling answered in issues 1644 that select statement should be removed from the query. I think this is because the select statement will be added by applying an entity view. The entity view is for projection.

So I make changes to the code to see what will happen. I add a method in CriteriaBuilderImpl (this is the simple way to do) to clear CriteriaBuilderImpl.selectManager.selectInfos, then after calling BlazeCriteriaBuilderRenderer.render(), I call this method to clear selectInfos. What surprised me was, using this approach, the examples I tested in 21. Querydsl integration (I did not test fromValue statement and Querydsl's tranform in EntityView. Note that I tested samples in 21.7 and 21.8 about CTE[1]) worked successfully using EntityView.

This inspired me that if I could pay attention to the fact that I was going to use EntityView for projection when constructing CriteriaBuilder, perhaps the current implementation of Querydsl integration would be good enough to be used in EntityView. (I tested that if I use Blaze Persistence API to create a CriteriaBuilder with select statement, the same issue occurred too.)

Things may not be that simple, because issue 1644 was reported two years ago, but it has not been resolved yet. And its resolution time has been postponed from 1.6.13 to 2.0.1. So this issue may be complicated to resolve, not as simple as I did.

So, what I want to ask is, will my code changes cause any problems I don't know about? I highly recommend EntityView's solution and want to use it in a limited scope, waiting for such issues to be resolved.

Then there is another case about the following example in the documentation.

        BlazeJPAQuery<Tuple> query = new BlazeJPAQuery<Tuple>(em, cbf).from(cat)
                .select(cat.name,
                        JPQLNextExpressions.rowNumber().over().orderBy(cat.name),
                        JPQLNextExpressions.lastValue(cat.name).over().partitionBy(cat.id));

The generated query is

SELECT cat.name, row_number() OVER (ORDER BY cat.name ASC NULLS LAST), last_value(cat.name) OVER (PARTITION BY cat.id) FROM Cat cat

But after applying view, the query is

SELECT cat.id AS CatView_id, UPPER(cat.name) AS CatView_name FROM Cat cat

This result might be expected, because the select statement should follow what the entity view defined.

So I think this kind use of window function might be required to be defined in the entity view, like what Using CTEs in entity views about CTE. But the entity view documentation does not describe how to use window function in enity view.

My question is, will EntityView support such window function usage?

The last part about 21.10. Lateral joins

When I change the following code to use entity view:

QRecursiveEntity t = new QRecursiveEntity("t");
QRecursiveEntity subT = new QRecursiveEntity("subT");
QRecursiveEntity subT2 = new QRecursiveEntity("subT2");

List<Tuple> fetch = new BlazeJPAQuery<>(entityManager, cbf)
    .select(t, subT2)
    .from(t)
    .leftJoin(JPQLNextExpressions.select(subT).from(t.children, subT).orderBy(subT.id.asc()).limit(1), subT2)
    .lateral()
    .fetch();

even if I change select(t, subT2) to select(t), I still get

java.lang.IllegalArgumentException: No or multiple query roots, can't find single root!

Maybe LATERAL is treated as an explicit join, but I only care about t, so I add some code to keep the first element in CriteriaBuilderImpl.joinManager.rootNodes and remove the others. The code runs well and the result is the same as the original sample.

Of course, this is not the correct way to handle it, but I want to know in which scenarios this approach will lead to errors?

After exploring the cause of this issue (i.e. issue 1943) for some time, I think the quality of Blaze source code is very good.

So, my idea is, if solving this issue is more complicated, can we first have a simpler solution, and then, through iteration, gradually solve this problem perfectly?

I'm new to Blaze, so my thinking may be a bit naive, but I really want to try using Querydsl in EntityView, because currently the Blaze API is not as fluent as Querydsl, especially when using an IDE.

[1] I specially mention CTE, because @jwgmeligmeyling said that casing to CriteriaBuilder is not safe for set operations and queries with common table expressions. I'm watching and keeping an eye out for this issue, but haven't encountered it yet.

beikov commented 1 week ago

I don't know what the trouble was that @jwgmeligmeyling encountered when looking into #1644, but you can use window functions in mapping expressions e.g.

@EntityView(Cat.class)
interface CatView {
  @IdMapping
  Long getId();
  @Mapping("row_number() OVER (ORDER BY name ASC NULLS LAST)")
  Long getRowNumber();
}
guofengzh commented 1 week ago

@beikov

Your example runs just fine.

Thanks.

jwgmeligmeyling commented 6 days ago

I am not entirely sure why changes to CriteriaBuilderImpl would be required to pull this off or how window functions are relevant, but just responding to the two questions directly asked towards me:

casting to CriteriaBuilder is not safe for set operations and queries with common table expressions

When using the set operations or CTE operations the builder type changes. See for example the code from the following test case from the repository: https://github.com/Blazebit/blaze-persistence/blob/e16c5c0a1744ec36db649908228fc761ebc52ebf/core/testsuite/src/test/java/com/blazebit/persistence/testsuite/SetOperationTest.java#L92-L104

It returns a FinalSetOperationCriteriaBuilder. Trying to cast that to a CriteriaBuilder will lead to a ClassCastException. The BlazeCriteriaBuilderRenderer for Querydsl uses the Blaze-Persistence Criteria Builder API to construct the query, so if you will create the exact same query using the Querydsl integration will result in a builder of the exact same return type ( FinalSetOperationCriteriaBuilder).AFAIK there is no common abstraction between those builders that can be passed to evm.applySetting. At the time, the only common denominator was Fetchable, which is what the Querydsl integration returns.

2.

that select statement should be removed from the query. I think this is because the select statement will be added by applying an entity view. The entity view is for projection.

When using the EntityViewManager it indeed does expect the built query does not have a projection already. However, the Querydsl integration currently requires a projection for the query renderer to even work. Null checks need to be added to prevent trying to render a select statement when there is no projection. I pointed out the exact locations here: https://github.com/Blazebit/blaze-persistence/issues/1644#issuecomment-1490076808 .

These are just pointers however, you may just run into the next query generation error after this 😄 If there's a draft PR with some example test cases, I can have a deeper dive.

guofengzh commented 3 days ago

I gotten it.

For the above 2, I also debug into the location you point out at #1644. I think BlazeCriteriaBuilderRenderer.render() just transforms Querydsl's AST into Blaze's (I haven't fully understood the code yet), and may not use the information provided by the select statement, so I added the following method to CriteriaBuilderImpl:

    public void resetSelectInfos() {
        this.selectManager.getSelectInfos().clear();
    }

Then, after calling render(), I call this method to sidestep the problem you pointed out (using the NULL check)

        Queryable<Cat, ?> catQueryable = bbr.render(query);
        CriteriaBuilderImpl<Cat> cb = (CriteriaBuilderImpl<Cat>)catQueryable;
        cb.resetSelectInfos();

The full code example is here.

The examples in the documentation (core and entity view module) that I tried work now, so, I think, this may be a temporary solution to the problem. As long as we are careful when constructing the CriteriaBuilder, it may not be a big problem to apply Querydsl to Entityview.

This is the purpose of raising this question here. I have no other deeper ideas yet.

Thanks for your help very much!