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

Using "OR" with a foreign key returns an inner join when should be an outter join #35

Closed
scottdear opened this issue Mar 20, 2015 · 21 comments

Comments

@scottdear
Copy link

If you use RQB and have two tables, a and b, and you create a foreign key which joins b to a, then you do a query where you specify a where clause that involves table a, then an "OR" and a where clause that involves table b, it should be creating an outer join.

For example, lets say we have company and weather and you link the two on zip code then do something like:

companies.employees > 100
OR
weather.tmp < 50

You get SQL that looks like

SELECT * FROM companies
INNER JOIN weather ON
companies.zip = weather.zip
WHERE
companies.employees > 100
OR
weather.tmp < 50

What it should look like is:

SELECT * FROM companies
OUTTER JOIN weather ON
companies.zip = weather.zip
WHERE
companies.employees > 100
OR
weather.tmp < 50

This is complicated, when using MySQL as it does not support OUTTER JOINS

It can be emulated by doing:

SELECT * FROM companies
LEFT JOIN weather ON
companies.zip = weather.zip
WHERE
companies.employees > 100
UNION
SELECT * FROM companies
RIGHT JOIN weather ON
companies.zip = weather.zip
WHERE
weather.tmp < 50

From a UI/End User perspective, it may be preferrable to do a LEFT JOIN so that you don't get any records from the right, that are not also on the left.

@salk31
Copy link
Owner

salk31 commented Mar 21, 2015

Definitely seems like a sensible option, not sure it should be the default option.

I'm a bit surprised MySql doesn't support full outer joins. I'm not sure about pulling this problem into core RQB.

Options I can think of:
A - RQB provides option per FK to do left our full outer join (not sure about right join)
B - Application developer provides a dumbed down view to hide details from user/RQB.
C - Subquery options WHERE EXISTS
CON - Not the same as outer join

@salk31
Copy link
Owner

salk31 commented Mar 22, 2015

Hi Scott,

Is this really your use case for a full outer join? I know nothing about your domain but are you not really after particular companies in this case?

Cheers

Sam

@scottdear
Copy link
Author

Actually in my use case I want a left join (so I get all the weather columns for the emp > 100 part of the clause) and an inner join on the temperature part, so that gives me all the companies where temp < 50, which from a UI/End User perspective is what I would expect.

After thinking about this for a bit, I think the rub here, is that since step one is picking a main table, and then you select a criteria that uses a foreign key, it is 'JOINED' to that table. In that case, I don't think an outter join is really appropriate, as it would return records from the weather table where there were no matches in the company table, and since we chose the company table at the start, there shouldn't be any records in the results that do not have a company record.

Does that make sense?

I think the resulting SQL (at least for my needs) would be something like:

SELECT * FROM companies
LEFT JOIN weather ON
companies.zip = weather.zip
WHERE
companies.employees > 100
OR
weather.temp < 50

Which should be the same as:

SELECT * FROM companies
LEFT JOIN weather ON
companies.zip = weather.zip
WHERE
companies.employees > 100
UNION
SELECT * FROM companies
INNER JOIN weather ON
companies.zip = weather.zip
WHERE
weather.temp < 50

The problem only presents itself when using an OR and then a Foreign key. If the OR is replaced with an AND, then an INNER JOIN is just what's expected.

If later on we add an AND, like

AND weather.condition = 'rain'

Then we are back to an INNER JOIN because it has to be part of the resulting set.

@salk31
Copy link
Owner

salk31 commented Mar 27, 2015

I think that fits with my use cases. My users are looking for a set of objects of the same type so left join is fine.

How about an option on the FK to say inner or left join?

Does mysql handle nulls according to the standard? ie any comparison involving null is false?

If so then in theory could always use left join the only problem being a performance hit?

@scottdear
Copy link
Author

I don't think it's a matter of the key being left or inner join all the
time, I think it's more; if there's an "OR" on a foreign key it should
be a left join, and if it's "AND" and it should be an inner join.

Does that make sense?

-Scott

On 03/27/2015 12:46 AM, salk31 wrote:

I think that fits with my use cases. My users are looking for a set of
objects of the same type so left join is fine.

How about an option on the FK to say inner or left join?

Does mysql handle nulls according to the standard? ie any comparison
involving null is false?

If so then in theory could always use left join the only problem being
a performance hit?


Reply to this email directly or view it on GitHub
#35 (comment).

@salk31
Copy link
Owner

salk31 commented Mar 27, 2015

Simplest thing would just to always do a left join?

It wouldn't make any difference to AND since all comparisons would be against null so fail anyway?

@salk31
Copy link
Owner

salk31 commented Apr 4, 2015

I get the feeling I'm missing something obvious or we are talking at cross purposes. Sorry if I'm being thick.

@scottdear
Copy link
Author

I think that maybe I'm over complicating this ;)

If do a left join, then use weather.temperature < 50, it SHOULD make the left join act like an inner join because it should not return any rows where weather.temperature is null, i.e. no matching record on the right side.

So I think always doing a left join would work.

Does that make sense?

@salk31
Copy link
Owner

salk31 commented Apr 10, 2015

I think so. So a global change (config or hard coded) to use left joins all the time would suit your needs? It could be revisited if somebody ever has a performance problem.

@scottdear
Copy link
Author

scottdear commented Apr 10, 2015 via email

@salk31
Copy link
Owner

salk31 commented Apr 11, 2015

Downside is just a possible small performance hit (I think). Should be easy to do but I've got a big change that I'm trying to get out of the door.

salk31 added a commit that referenced this issue Apr 18, 2015
@salk31
Copy link
Owner

salk31 commented Apr 18, 2015

Should be there on master now, please could you try it?

@scottdear
Copy link
Author

I've been pulling/installing from the tardis branch, are those
updates/changes in master now?

On 04/18/2015 02:52 AM, salk31 wrote:

Should be there on master now, please could you try it?


Reply to this email directly or view it on GitHub
#35 (comment).

salk31 added a commit that referenced this issue Apr 24, 2015
#35

Conflicts:
	redquerybuilder-core/src/test/java/com/redspr/redquerybuilder/core/client/GwtTestBasics.java
@salk31
Copy link
Owner

salk31 commented Apr 24, 2015

I've picked/merged to tardis now. So should be there for you.

@scottdear
Copy link
Author

Sorry it took so long to get back on this. I dropped in the new code and it is now doing "LEFT OUTER JOIN" instead of "INNER JOIN". Any chance we could make it just "LEFT JOIN" I believe LEFT and LEFT OUTER are the same?

@salk31
Copy link
Owner

salk31 commented May 21, 2015

It does seem to be optional in SQL-92 so I'm with you that I'd rather not see it.

Just out of interest why don't you want it?

I'll work on a fix and commit ASAP.

salk31 added a commit that referenced this issue May 21, 2015
Be minimal and don't generate "OUTER"
@salk31
Copy link
Owner

salk31 commented May 21, 2015

I've put this on tardis, please let me know what you think and I'll merge it to master.

@scottdear
Copy link
Author

We wrote our own SQL database for the back end, and at the time it
didn't support the LEFT OUTER only LEFT. We just added support to our
code to support LEFT OUTER, so if you want, you can leave it in.

On 05/21/2015 08:10 AM, salk31 wrote:

It does seem to be optional in SQL-92 so I'm with you that I'd rather
not see it.

Just out of interest why don't you want it?

I'll work on a fix and commit ASAP.


Reply to this email directly or view it on GitHub
#35 (comment).

@scottdear
Copy link
Author

Will do, I'll pull it down and give it a test today.

-Scott

On 05/21/2015 08:54 AM, salk31 wrote:

I've put this on tardis, please let me know what you think and I'll
merge it to master.


Reply to this email directly or view it on GitHub
#35 (comment).

@salk31
Copy link
Owner

salk31 commented May 23, 2015

OK to close?

@scottdear
Copy link
Author

For sure, stick a fork in this one.

On Fri, May 22, 2015 at 11:44 PM, salk31 notifications@github.com wrote:

OK to close?


Reply to this email directly or view it on GitHub
#35 (comment)
.

@salk31 salk31 closed this as completed May 23, 2015
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