Use proper inner join in Linq instead of implicit join and where clauses #2037

hazzik · 2019-03-03T22:55:39Z

hazzik · 2019-03-03T23:01:51Z

src/NHibernate/Hql/Ast/HqlTreeBuilder.cs

@@ -363,6 +363,11 @@ public HqlJoin Join(HqlExpression expression, HqlAlias @alias)
 			return new HqlJoin(_factory, expression, @alias);
 		}

+		public HqlInnerJoin InnerJoin(HqlExpression expression, HqlAlias @alias)
+		{
+			return new HqlInnerJoin(_factory, expression, @alias);


Unfortunately new join feature does not support implicit join types, so have to be explicit.

hazzik · 2019-03-03T23:05:31Z

The description of #1128 is vague, so this change will make that issue obsolete. Technically the previous behaviour is correct despite being sub-optimal.

hazzik · 2019-03-04T09:10:10Z

So are you sure that this PR depends on this issue? As from my understanding existing tests shouldn't suffer from this issue.

Somehow.

Dlinq9 test:

var actual =
	(from o in db.Orders
	 from p in db.Products
	 join d in db.OrderLines
		on new {o.OrderId, p.ProductId} equals new {d.Order.OrderId, d.Product.ProductId}
		into details
	 from d in details
	 select new {o.OrderId, p.ProductId, d.UnitPrice}).ToList();

Is generating following SQL:

select
    order0_.OrderId as col_0_0_,
    product1_.ProductId as col_1_0_,
    orderline2_.UnitPrice as col_2_0_
from
    Orders order0_,
    Products product1_
inner join OrderLines orderline2_
    on (orderline2_.OrderId=order0_.OrderId and orderline2_.ProductId=product1_.ProductId)

However I'm not sure how the SQL should look like.

bahusoid · 2019-03-04T09:12:34Z

Shouldn't your change eliminate theta-style joins?

hazzik · 2019-03-04T09:14:38Z

Shouldn't you change eliminate theta-style joins?

Only second join in this query.

bahusoid · 2019-03-04T09:18:17Z

Well from ordering standpoint (which #2031 is about) everything is correct. Isn't it? So if it throws it's something else..

hazzik · 2019-03-04T09:19:17Z

It throws:

"System.Data.SqlClient.SqlException : The multi-part identifier "order0_.OrderId" could not be bound."

bahusoid · 2019-03-04T09:30:30Z

It seems SQL Server doesn't like such mix of theta-style joins and inner joins. If convert to bogus inner join it works:

select
    order0_.OrderId as col_0_0_,
    product1_.ProductId as col_1_0_,
    orderline2_.UnitPrice as col_2_0_
from
    Orders order0_
inner join Products product1_ on 1=1
inner join OrderLines orderline2_
    on (orderline2_.OrderId=order0_.OrderId and orderline2_.ProductId=product1_.ProductId)

Maybe we should completely get rid of theta-style joins in Linq?

hazzik · 2019-03-04T09:31:59Z

Maybe we should completely get rid of theta-style joins in Linq?

What would be SQL for that query?

hazzik · 2019-03-04T09:34:35Z

SQL server is not alone there, actually. Postgres throws a similar exception:

----> Npgsql.PostgresException : 42P01: invalid reference to FROM-clause entry for table "order0_"

hazzik · 2019-03-04T09:40:30Z

Actually, these are called not "theta join" but "implicit join"

hazzik · 2019-03-04T10:05:48Z

Ok, found correct issue: HHH-1480 + HHH-11337

bahusoid · 2019-03-04T10:46:20Z

I'm not sure it's a good fix. It surely breaks mix of implicit and right joins in hql - see https://hibernate.atlassian.net/browse/HHH-5352

bahusoid · 2019-03-04T11:28:21Z

There is still ordering issue with "cross join" fix for 'CrossJoinAndWithClause' :(
And if you need I can provide failing RIGHT join test due to "cross join" fix which works on master.

IMHO this Cross join behavior should be configurable per query and be enabled by default only from linq as HHH-5352 is a valid concern.

hazzik · 2019-03-04T11:32:03Z

I can provide failing RIGHT join test due to "cross join" fix which works on master.

Did it work in 5.2? ;-)

bahusoid · 2019-03-04T12:00:15Z

Did it work in 5.2? ;-)

Actually yes. You can mix implicit join and right join on association in 5.2:

select a, b, c
from a, b
right join b.c with ..

This cross join fix breaks such queries

bahusoid · 2019-03-04T12:17:13Z

Hm.. There is also interesting commit which might fix my right join issue. Though not sure..

maca88 · 2020-03-08T20:28:54Z

As I am trying to add support for left joins for Linq (#864) and this issue is related, I want to give my thoughts about this.

Maybe we should completely get rid of theta-style joins in Linq?

I do agree. We shouldn't use comma syntax for doing a join, as it is an old way to join tables and it is not flexible, a simple explanation why can be found in the mariadb documentation.

What would be SQL for that query?

The query should look like this:

select
    order0_.OrderId as col_0_0_,
    product1_.ProductId as col_1_0_,
    orderline2_.UnitPrice as col_2_0_
from
    Orders order0_
cross join Products product1_
inner join OrderLines orderline2_
    on (orderline2_.OrderId=order0_.OrderId and orderline2_.ProductId=product1_.ProductId)

Using a cross join allows us to reference the joined table in the later joins which is not possible when using the comma syntax.

I'm not sure it's a good fix. It surely breaks mix of implicit and right joins in hql

I do agree, I think that we shouldn't port the "fix" from Hibernate that replaces comma joins with cross joins as already mentioned, it is not the same when additional joins are added afterwards.

There is still ordering issue with "cross join" fix for 'CrossJoinAndWithClause'

The query in that test is not valid as the left join references the table that is not available due to the JOIN precedence rules in SQL-99. IMO we shouldn't try to fix such queries, I think it is the user responsibility to know what are the limitations when using the comma syntax for joining tables.

There is also interesting commit which might fix my right join issue

I don't think that we need to fix anything when the user mixes comma joins with right/left/inner joins, for the above query we should convert it to sql as it is, without trying to convert the comma joins into cross joins.

bahusoid · 2020-03-09T08:27:38Z

The query in that test is not valid

Yes we figured it out in the issue discussion. See pretty much the same thoughts here #2031 (comment)

Also I did realize that entity join implementation in hql that I ported from hibernate have some flaws when mixed with associations. If I remember correctly entity joins are simply appended to the end disregarding it's actual usage in HQL. See example here: #2031 (comment)

P.S. Sorry I don't really have time to actively participate now.

bahusoid · 2020-03-22T09:55:00Z

Is this PR still valid? Isn't this functionality already implemented in #2327?

hazzik · 2020-03-22T18:43:15Z

I don’t know now:)

maca88 · 2020-03-22T19:08:39Z

Isn't this functionality already implemented in #2327?

Yes it is, this PR can be closed.

hazzik commented Mar 3, 2019

View reviewed changes

hazzik added t: Improvement c: Linq Provider p: Minor labels Mar 3, 2019

hazzik force-pushed the proper-inner-join branch from dbc4485 to 40bd25e Compare March 3, 2019 23:25

This comment has been minimized.

Sign in to view

hazzik mentioned this pull request Mar 4, 2019

Combining an implicit JOIN with a LEFT JOIN which requires a WITH clause triggers an exception. #2031

Closed

nhibernate deleted a comment from bahusoid Mar 4, 2019

Use proper inner join in Linq instead of cross-join and where clauses

405a0d3

hazzik force-pushed the proper-inner-join branch from b87f858 to 405a0d3 Compare April 15, 2019 02:19

nhibernate deleted a comment from autorebase bot Jun 14, 2019

hazzik added the r: Replaced label Mar 22, 2020

hazzik closed this Mar 22, 2020

bahusoid changed the title ~~Use proper inner join in Linq instead of cross-join and where clauses~~ Use proper inner join in Linq instead of implicit join and where clauses Mar 22, 2020

RockNHawk mentioned this pull request Aug 13, 2020

How to correctly make join by IQueryable with NHibernate #2478

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use proper inner join in Linq instead of implicit join and where clauses #2037

Use proper inner join in Linq instead of implicit join and where clauses #2037

hazzik commented Mar 3, 2019 •

edited

Loading

hazzik Mar 3, 2019

hazzik commented Mar 3, 2019 •

edited

Loading

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

hazzik commented Mar 4, 2019

bahusoid commented Mar 4, 2019 •

edited

Loading

hazzik commented Mar 4, 2019

bahusoid commented Mar 4, 2019 •

edited

Loading

hazzik commented Mar 4, 2019

bahusoid commented Mar 4, 2019

hazzik commented Mar 4, 2019

hazzik commented Mar 4, 2019

hazzik commented Mar 4, 2019

This comment has been minimized.

hazzik commented Mar 4, 2019 •

edited

Loading

bahusoid commented Mar 4, 2019

bahusoid commented Mar 4, 2019

hazzik commented Mar 4, 2019

bahusoid commented Mar 4, 2019 •

edited

Loading

bahusoid commented Mar 4, 2019

maca88 commented Mar 8, 2020

bahusoid commented Mar 9, 2020

bahusoid commented Mar 22, 2020

hazzik commented Mar 22, 2020

maca88 commented Mar 22, 2020

Use proper inner join in Linq instead of implicit join and where clauses #2037

Use proper inner join in Linq instead of implicit join and where clauses #2037

Conversation

hazzik commented Mar 3, 2019 • edited Loading

hazzik Mar 3, 2019

Choose a reason for hiding this comment

hazzik commented Mar 3, 2019 • edited Loading

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

hazzik commented Mar 4, 2019

bahusoid commented Mar 4, 2019 • edited Loading

hazzik commented Mar 4, 2019

bahusoid commented Mar 4, 2019 • edited Loading

hazzik commented Mar 4, 2019

bahusoid commented Mar 4, 2019

hazzik commented Mar 4, 2019

hazzik commented Mar 4, 2019

hazzik commented Mar 4, 2019

This comment has been minimized.

hazzik commented Mar 4, 2019 • edited Loading

bahusoid commented Mar 4, 2019

bahusoid commented Mar 4, 2019

hazzik commented Mar 4, 2019

bahusoid commented Mar 4, 2019 • edited Loading

bahusoid commented Mar 4, 2019

maca88 commented Mar 8, 2020

bahusoid commented Mar 9, 2020

bahusoid commented Mar 22, 2020

hazzik commented Mar 22, 2020

maca88 commented Mar 22, 2020

hazzik commented Mar 3, 2019 •

edited

Loading

hazzik commented Mar 3, 2019 •

edited

Loading

bahusoid commented Mar 4, 2019 •

edited

Loading

bahusoid commented Mar 4, 2019 •

edited

Loading

hazzik commented Mar 4, 2019 •

edited

Loading

bahusoid commented Mar 4, 2019 •

edited

Loading