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

Left vs Inner joins issue #81

Open
bukajsytlos opened this issue Oct 19, 2022 · 14 comments
Open

Left vs Inner joins issue #81

bukajsytlos opened this issue Oct 19, 2022 · 14 comments

Comments

@bukajsytlos
Copy link
Contributor

I didn't find any follow-up of #39.
How should one handle this issue? I don't think that join hints are way to go, as whether to use left or right join depends on query parameters being used and whether a relationship is optional somewhere on the path.

i.e if we have relationship1.value==abc or relationship1.value=na=''
if relationship1 is not optional, there is no point in left join.
if relationship1 is optional it should be joined left.
if relationship1 is optional but there is no "or is null" it should be inner join.
there is also usecase mentioned in last comment of linked PR.

afaiu this would need some preprocessing, cause join type can't be changed on the fly, right?

@perplexhub
Copy link
Owner

Do you have any sample codes?

@bukajsytlos
Copy link
Contributor Author

bukajsytlos commented Oct 22, 2022

for example this test

	@Test
	public final void testXXX() {
		String rsql = "company.code==demo,city.name==Moon";
		List<User> users = userRepository.findAll(toSpecification(rsql));
		long count = users.size();
		log.info("rsql: {} -> count: {}", rsql, count);
		assertThat(rsql, count, is(3l));
	}

@perplexhub
Copy link
Owner

Can you provide the expected result and actual result? Thanks.

@bukajsytlos
Copy link
Contributor Author

bukajsytlos commented Oct 22, 2022

expected result should be 3 users
2 from company, 1 from city
as a result of this query

select user0_.id          as id1_11_,
       user0_.city_id     as city_id5_11_,
       user0_.company_id  as company_6_11_,
       user0_.create_date as create_d2_11_,
       user0_.name        as name3_11_,
       user0_.status      as status4_11_
from users user0_
         inner join company company1_ on user0_.company_id = company1_.id
         left outer join city city2_ on user0_.city_id = city2_.id
where company1_.code = ?
   or city2_.name = ?

instead of one result as of this actual query

select user0_.id          as id1_11_,
       user0_.city_id     as city_id5_11_,
       user0_.company_id  as company_6_11_,
       user0_.create_date as create_d2_11_,
       user0_.name        as name3_11_,
       user0_.status      as status4_11_
from users user0_
         inner join company company1_ on user0_.company_id = company1_.id
         inner join city city2_ on user0_.city_id = city2_.id
where company1_.code = ?
   or city2_.name = ?;

@perplexhub
Copy link
Owner

Could you try this?

	@Test
	public final void testJoinHintsRelationWithMultipleOrCase() {
		String rsql = "company.code==demo,city.name=='Hong Kong Island'";
		Map<String, JoinType> joinHints = new HashMap<String, JoinType>() {{put("User.city", JoinType.LEFT);put("User.company", JoinType.LEFT);}};
		List<User> users = userRepository.findAll(toSpecification(rsql, true,null, joinHints));
		long count = users.size();
		log.info("rsql: {} -> count: {}", rsql, count);
		assertThat(rsql, count, is(3l));
	}

@bukajsytlos
Copy link
Contributor Author

yes, but as I said I don't find join hints meaningful. I don't know which rsql string will be send by client.
I (as a user of this lib) don't want to care and parse rsql string to determine if I want left or inner join.

@perplexhub
Copy link
Owner

perplexhub commented Oct 23, 2022

Should we always use left join only if the relationship is optional? Otherwise, use inner join.

@bukajsytlos
Copy link
Contributor Author

unfortunately rules are not that simple. I will try to write them down

@bukajsytlos
Copy link
Contributor Author

bukajsytlos commented Nov 6, 2022

Not 100% sure if this is all but this is what I've came up:

  1. checking isNull on relationship path. Left join has to be on from first optional relationship onward
  2. having OR "split", where path joins on any branch have to be left joined from first optional relationship onward on path

@bukajsytlos
Copy link
Contributor Author

although first point is questionable I guess

@perplexhub
Copy link
Owner

Do you have time to implement it? It would be more easy to verify with test cases.

@bukajsytlos
Copy link
Contributor Author

bukajsytlos commented Nov 17, 2022

well, I could try, but do you have an idea how to go about that? afaik we need to determine join type based on data later during visiting.

@perplexhub
Copy link
Owner

Could we first analyze the structure of the query and generate the JoinHint before processing in converter (RSQLVisitor)?

@bukajsytlos
Copy link
Contributor Author

hmm, that's good idea.

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

2 participants