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

[BUG] JoinFetch alias while pagination #138

Closed
yarikwest opened this issue Nov 25, 2022 · 5 comments
Closed

[BUG] JoinFetch alias while pagination #138

yarikwest opened this issue Nov 25, 2022 · 5 comments

Comments

@yarikwest
Copy link

yarikwest commented Nov 25, 2022

Hi.
First of all, thanks for great library.

I have found bug while creating query with JoinFetch and filtering by fetched entity. Select query is ok, but when using pagination and results is more than page size, then count query (where is bug) also gets created, and it doesn't contain join.

Example:

@Entity
public class ServicePoint {
    @Id
    @GeneratedValue(strategy = GenerationType.IDENTITY)
    private Long id;

    private String abbr;
    private String name;

    @ManyToOne(cascade = {CascadeType.PERSIST, CascadeType.MERGE}, fetch = FetchType.LAZY)
    @JoinColumn(name = "address_id")
    private Address address;
}

@Entity
public class Address {
    @Id
    @GeneratedValue(strategy = GenerationType.IDENTITY)
    private Long id;

    private String city;
    private String code;
    private String street;

    @Column(name = "street_no")
    private String streetNo;
}

image

Generated sql:

select
        servicepoi0_.id as id1_29_0_,
        address1_.id as id1_1_1_,
        servicepoi0_.abbr as abbr2_29_0_,
        servicepoi0_.address_id as address_7_29_0_,
        servicepoi0_.name as name4_29_0_,
        address1_.city as city2_1_1_,
        address1_.code as code3_1_1_,
        address1_.street as street4_1_1_,
        address1_.street_no as street_n5_1_1_
    from
        service_points servicepoi0_ 
    left outer join
        addresses address1_ 
            on servicepoi0_.address_id=address1_.id 
    where
        upper(servicepoi0_.abbr) like '%A%'
        or upper(servicepoi0_.name) like '%A%' 
        or upper(address1_.city) like '%A%' 
        or address1_.code like 'a%' 
        or upper(address1_.street) like '%A%' 
    order by
        servicepoi0_.abbr asc limit 3

Exception:

org.hibernate.hql.internal.ast.QuerySyntaxException: Invalid path: 'generatedAlias1.city' [select count(generatedAlias0) from ServicePoint as generatedAlias0 where ( ( ( ( upper(generatedAlias0.abbr) like :param0 ) or ( upper(generatedAlias0.name) like :param1 ) ) or ( upper(generatedAlias1.city) like :param2 ) ) or ( generatedAlias1.code like :param3 ) ) or ( upper(generatedAlias1.street) like :param4 )]

But when using like this, than everything ok:
image
And generated count query looks like this:

select
        count(servicepoi0_.id) as col_0_0_ 
    from
        service_points servicepoi0_ cross 
    join
        addresses address1_ 
    where
        servicepoi0_.address_id=address1_.id 
        and (
            upper(servicepoi0_.abbr) like '%A%' 
            or upper(servicepoi0_.name) like '%A%' 
            or upper(address1_.city) like '%A%' 
            or address1_.code like 'a%'  
            or upper(address1_.street) like '%A%'
        )
@jradlica
Copy link
Collaborator

jradlica commented Nov 25, 2022

The minimal code which reproduces the bug:

(Issue138.java - it should be placed in /src/test/java/...)

import net.kaczmarzyk.E2eTestBase;
import net.kaczmarzyk.spring.data.jpa.Customer;
import net.kaczmarzyk.spring.data.jpa.CustomerRepository;
import net.kaczmarzyk.spring.data.jpa.domain.In;
import net.kaczmarzyk.spring.data.jpa.web.annotation.JoinFetch;
import net.kaczmarzyk.spring.data.jpa.web.annotation.Spec;
import org.junit.Test;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.data.domain.Pageable;
import org.springframework.data.jpa.domain.Specification;
import org.springframework.http.MediaType;
import org.springframework.web.bind.annotation.RequestMapping;
import org.springframework.web.bind.annotation.ResponseBody;
import org.springframework.web.bind.annotation.RestController;

import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get;
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath;
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;

public class Issue138 extends E2eTestBase {

	@RestController
	public static class TestControllerIssue138 {

		@Autowired
		private CustomerRepository customerRepository;

		@RequestMapping(value = "/issue138/join-fetch-interface/customers", params = { "orderIn" })
		@ResponseBody
		public Object findByOrderIn(@JoinFetch(paths = "orders", alias = "o")
									@Spec(path = "o.itemName", params = "orderIn", spec = In.class) Specification<Customer> spec, Pageable pageable) {
			return customerRepository.findAll(spec, pageable);
		}

	}

	@Test
	public void resolveSpecBasedOnJoinFetchAliasForPagedRequest() throws Exception {
		mockMvc.perform(get("/issue138/join-fetch-interface/customers")
				.param("orderIn", "Pizza")
						.param("order", "Duff Beer")
						.param("page", "0")
						.param("size", "1")
						.param("sort", "id")
				.accept(MediaType.APPLICATION_JSON))
			.andExpect(status().isOk())
			.andExpect(jsonPath("$").isArray())
			.andExpect(jsonPath("$[0].firstName").value("Homer"))
			.andExpect(jsonPath("$[1]").doesNotExist());
	}

}

@tkaczmarzyk
Copy link
Owner

@yarikwest Thanks for reporting. After short analysis I understand the issue and prepared a working PoC of a fix. But it needs more testing and refactoring.

In a nuthshell:

  1. JoinFetch had an explicit skip of joinin in count queries -- this was done for 2 reasons: 1) fetch is very often used for just initiualizing lazy collections, not filtering so it would be superfluous for count query 2) hibernate seems to forbid join fetch in count query (i.e. when root entity is not included in the result list)
  2. I added a quick hackish fragment that in case of a count query converts join fetch to a join...
  3. ... and the test posted by @jradlica seems to be working fine -- see related PR

I think this is the way to go, but:

  1. More tests must be added (e.g. for multi-level joins)
  2. JoinFetch to Join conversion must be skipped if there is no actual filtering on the joined entity (in such a case it would be superfluous in a count query)
  3. JoinFetch allows multiple paths in a single annoatation, in the PoC I used just the first one

So it requires more work. You can compiling the version from the branch and see if it solves your issue in the meantime

@tkaczmarzyk
Copy link
Owner

(Marking as good first issue because sb could at least write more tests e.g. with nested joins (i.e. join fetching on a joined entity). The actual implementation requires more deep knowledge of the internals)

@tkaczmarzyk
Copy link
Owner

(uploaded as 2.9.1-SNAPSHOT to my private Maven repo for easier evaluation, see bottom of the README for the location details)

@tkaczmarzyk
Copy link
Owner

@yarikwest fix included in v2.10.0 (just released)

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

No branches or pull requests

3 participants