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

Issue of Repository output sql [DATAJPA-942] #1293

Closed
spring-projects-issues opened this issue Aug 3, 2016 · 12 comments
Closed

Issue of Repository output sql [DATAJPA-942] #1293

spring-projects-issues opened this issue Aug 3, 2016 · 12 comments
Assignees
Labels
in: core Issues in core support

Comments

@spring-projects-issues
Copy link

Ben Li opened DATAJPA-942 and commented

I found an issue of spring data jpa. Details are as follow:

  1. There are 3 Entities. Entity A (member variable: protected C c), EntityB (which extends A), and Entity C.
  2. BRepository. List<B> findAllByCAndName(C c, String name);

When execute the repo “findAllByCAndName(c,name)”, output sql like this:
“select b0_.id as id1_0_, b0_.c_id as c_id3_0_, b0_.name as name2_0_ from b b0_ left outer join c c1_ on b0_.c_id=c1_.id where c1_.id=? and b0_.name=?”

However, my expectation is “select b0_.id as id1_0_, b0_.c_id as c_id3_0_, b0_.name as name2_0_ from b b0_ left outer join c c1_ on b0_.c_id=c1_.id where b0_.c_id=? and b0_.name=?”

The difference between two sql might reduce the sql efficiency greatly. How to change the repo output sql from “where c1_.id=?” to “where b0_.c_id=?” ? Any suggestion will be appreciated.

Here is my demo project, please run it to recur the issue. We are looking forward to your reply. Thanks a lot


Affects: 1.9.4 (Gosling SR4)

Attachments:

Issue Links:

  • DATAJPA-1076 Use foreign key instead of joining the full table when possible
    ("is duplicated by")

1 votes, 3 watchers

@spring-projects-issues
Copy link
Author

Oliver Drotbohm commented

SQL generation is handled by your persistence provider which I assume is Hibernate in this case. Did you already check with them?

@spring-projects-issues
Copy link
Author

Ben Li commented

hi, good to hear your reply. Our persistence provider is Hibernate, and we have checked and confirmed that SQL generation is fine. But the issue occurred as my description. We have no idea about that, any suggestion? Thanks!!

@spring-projects-issues
Copy link
Author

Oliver Drotbohm commented

Call me puzzled. Your original description states that you expect other SQL generated. Now you say SQL generation is fine. If the latter is the case, then what is the ticket about?

@spring-projects-issues
Copy link
Author

Frank Tang commented

SQL generation fine means following situation:

@Query("select b from #{#entityName} b where b.c=?1 and b.name=?2")
List<B> findAllByCAndName(C c, String name);

this will generate the right sql when add an expicit @Query annotation ... the right sql will exactly be:"select b0_.id as id1_0_, b0_.c_id as c_id3_0_, b0_.name as name2_0_ from b b0_ left outer join c c1_ on b0_.c_id=c1_.id where b0_.c_id=? and b0_.name=?"

@spring-projects-issues
Copy link
Author

Ben Li commented

yes, SQL generation fine means: although the sql generated, not as my expectation.

Such as:
List<B> findAllByCAndName(C c, String name);

wrong sql generation:
SQL generation: “select b0_.id as id1_0_, b0_.c_id as c_id3_0_, b0_.name as name2_0_ from b b0_ left outer join c c1_ on b0_.c_id=c1_.id where c1_.id=? and b0_.name=?”

my expectation
SQL generation should be : “select b0_.id as id1_0_, b0_.c_id as c_id3_0_, b0_.name as name2_0_ from b b0_ left outer join c c1_ on b0_.c_id=c1_.id where b0_.c_id=? and b0_.name=?”

@spring-projects-issues
Copy link
Author

Oliver Drotbohm commented

I can't reproduce your claims on an example. From the Spring Data JPA test cases:

class User {
  @ManyToOne User manager;
  String lastname;
}

A manually defined query select u from User u where u.manager = ?1 and u.lastname = ?2 yields select … from SD_User user0_ where user0_.manager_id=? and user0_.lastname=?. I tried replicating what Spring Data does using the JPA Criteria API:

CriteriaBuilder builder = em.getCriteriaBuilder();
		CriteriaQuery<User> query = builder.createQuery(User.class);
		Root<User> root = query.from(User.class);

		Join<User, User> join = root.join(User_.manager, JoinType.LEFT);
		ParameterExpression<User> managerExpression = builder.parameter(User.class);
		ParameterExpression<String> lastnameExpression = builder.parameter(String.class);

		Predicate managerEquals = builder.equal(join, managerExpression);
		Predicate lastnameEquals = builder.equal(root.get(User_.lastname), lastnameExpression);

		query = query.select(root).where(managerEquals, lastnameEquals);
		// query = query.select(root).where(builder.and(managerEquals, lastnameEquals));

		TypedQuery<User> createQuery = em.createQuery(query);

		createQuery.setParameter(managerExpression, null);
		createQuery.setParameter(lastnameExpression, "Foo");

		createQuery.getResultList();

That just generates the SQL you mentioned to be improvable, but it looks like that's just what Hibernate generates

@spring-projects-issues
Copy link
Author

Frank Tang commented

Your provided test case is not the same as ours in the demo attachment.In our demo,following 2 points must be noticed:
1.Entity A has an annotation @MappedSuperclass and contains a @ManyToOne association C
2.Entity B extends A so that B also has an association C too.
which are missed in your test case..

We defined a method in repository B like:
List<B> findAllByCAndName(C c, String name);

this method makes association C which extends from a MappedSuperclass as a where condition.

When add an annotation @Query("select b from B b where b.c=?1 and b.name=?2") on this method, it works fine. So i think persistence provider(hibernate) should works fine.However,if we don't add @Query,only keep the method name,the wrong sql will be generated!

In our product, entity A is a base entity, and many repositories contain many method names make C as a filter query condition.As db data grows,this wrong sql will give a bad impact on performance..

@spring-projects-issues
Copy link
Author

Oliver Drotbohm commented

The point it, it doesn't make any difference. I've adapted my test case to use your domain types and I still get:

select b0_.id as id1_0_, b0_.c_id as c_id3_0_, b0_.name as name2_0_ from b b0_ left outer join c c1_ on b0_.c_id=c1_.id where c1_.id=? and b0_.name=?

The manually declared query select b from B b where b.c=?1 and b.name=?2 results in select b0_.id as id1_0_, b0_.c_id as c_id3_0_, b0_.name as name2_0_ from b b0_ where b0_.c_id=? and b0_.name=?. So it doesn't even use an explicit join.

My point is: there's no way for us to use the Criteria API in any other way, if we define the join explicitly as it's not under our control how HIbernate translates that criteria API setup into an actual query. And as you can see it doesn't make any difference whether there's inheritance involved or the like.

Here's the test case I used:

@RunWith(SpringJUnit4ClassRunner.class)
@ContextConfiguration(classes = DemoApplication.class)
public class FooTest {

	@PersistenceContext EntityManager em;

	@Test
	public void buildQueryUsingCriteriaApi() {

		CriteriaBuilder builder = em.getCriteriaBuilder();
		CriteriaQuery<B> query = builder.createQuery(B.class);
		Root<B> root = query.from(B.class);

		Join<B, C> join = root.join("c", JoinType.LEFT);
		ParameterExpression<C> managerExpression = builder.parameter(C.class);
		ParameterExpression<String> lastnameExpression = builder.parameter(String.class);

		Predicate managerEquals = builder.equal(join, managerExpression);
		Predicate lastnameEquals = builder.equal(root.get("name"), lastnameExpression);

		query = query.select(root).where(builder.and(managerEquals, lastnameEquals));

		TypedQuery<B> createQuery = em.createQuery(query);

		createQuery.setParameter(managerExpression, null);
		createQuery.setParameter(lastnameExpression, "Foo");

		createQuery.getResultList();
	}

	@Test
	public void buildQueryFromString() {

		Query query2 = em.createQuery("select b from B b where b.c=?1 and b.name=?2");

		query2.setParameter(1, null);
		query2.setParameter(2, "foo");

		query2.getResultList();
	}
}

@spring-projects-issues
Copy link
Author

Frank Tang commented

I just checked your test code,and modified test method "buildQueryUsingCriteriaApi" like following:

public void buildQueryUsingCriteriaApi() {

        CriteriaBuilder builder = em.getCriteriaBuilder();
        CriteriaQuery<B> query = builder.createQuery(B.class);
        Root<B> root = query.from(B.class);

        //Join<B, C> join = root.join("c", JoinType.LEFT);
        //ParameterExpression<C> managerExpression = builder.parameter(C.class);
        ParameterExpression<String> lastnameExpression = builder.parameter(String.class);

        //Predicate managerEquals = builder.equal(join, managerExpression);
        Predicate managerEquals = builder.equal(root.get("c"),(C)null);
        Predicate lastnameEquals = builder.equal(root.get("name"), lastnameExpression);

        query = query.select(root).where(builder.and(managerEquals, lastnameEquals));

        TypedQuery<B> createQuery = em.createQuery(query);

        //createQuery.setParameter(managerExpression, null);
        createQuery.setParameter(lastnameExpression, "Foo");

        createQuery.getResultList();
   }

and the generated sql is exactly our expected,it's fine! The difference is i commented using join...So! Can spring data jpa construct criteria without join C in this situation?!

@spring-projects-issues
Copy link
Author

Ben Li commented

HELLO ?????

@spring-projects-issues
Copy link
Author

Frank Tang commented

After dig the source code of Spring Data Jpa...i found the root cause of 'join' ,it's because of method org.springframework.data.jpa.repository.query.QueryUtils#requiresJoin:( ,any association member except anotated with value 'optional = false' will create a join sql phrase...unfortunately, our developers wrote many findByAssociationMember methods and most entity association member
is optional=true..this generate bad performance sql ...generally the main entity has the foreign key id, findByAssociationMember can be implemented without join, only need compare the passed in parameter id value with foreign key id! I don't know why method 'requiresJoin' need judge the non-optional association, i've tested it will make a cross join automatically if we try to compare association embed property and make requiresJoin method return false.
Conclusions:
1.This issue is not a bug,it should be an improvement.
2.If we don't need the cross join, we can find another way like using specifications not findByXXX, this case happened less than usual
3.findByAssociationMember without join should be the default case whether association is optional or not, because without join is the normal case

@Oliver Gierke
Hope to get your reply! thanks:)

@gregturn
Copy link
Member

If you modeled using optional=true, then as the javadocs read, "If set to false then a non-null relationship must always exist," this infers that null CAN be the case.

And this is what left outer join is for. Essentially, an optional match.

Indeed, left outer joins CAN be more expensive. You have the chance of retrieving more data, which costs more in time and memory consumption. But fundamentally, you have to model things as they are and work on optimization after that.

If you wish to use the Criteria API to write your own hand-optimized queries, that's fine. But we can't optimize in this fashion AGAINST the modeling annotations of JPA.

@gregturn gregturn self-assigned this Jun 29, 2022
@gregturn gregturn removed the type: bug A general bug label Jun 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core Issues in core support
Projects
None yet
Development

No branches or pull requests

3 participants