Skip to content
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

Which CriteriaBuilder can work with EntityView? #1954

Open
guofengzh opened this issue Nov 9, 2024 · 4 comments
Open

Which CriteriaBuilder can work with EntityView? #1954

guofengzh opened this issue Nov 9, 2024 · 4 comments

Comments

@guofengzh
Copy link
Contributor

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
Copy link
Member

beikov commented Nov 10, 2024

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
Copy link
Contributor Author

@beikov

Your example runs just fine.

Thanks.

@jwgmeligmeyling
Copy link
Collaborator

jwgmeligmeyling commented Nov 15, 2024

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:

FinalSetOperationCriteriaBuilder<String> cb = cbf.create(em, String.class)
.from(Document.class, "d1")
.select("d1.name")
.where("d1.name").eq("D1")
.intersect()
.from(Document.class, "d2")
.select("d2.name")
.where("d2.name").notEq("D2")
.except()
.from(Document.class, "d3")
.select("d3.name")
.where("d3.name").eq("D3")
.endSet();

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.

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: #1644 (comment) .

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
Copy link
Contributor Author

guofengzh commented Nov 19, 2024

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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants