Skip to content

Fix #1228 #2146

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

Merged
merged 1 commit into from
Jul 16, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .editorconfig
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,13 @@ insert_final_newline = true
indent_style = tab
dotnet_sort_system_directives_first = true
csharp_space_after_cast = true
csharp_new_line_before_open_brace = true
csharp_new_line_before_else = true
csharp_new_line_before_catch = true
csharp_new_line_before_finally = true
csharp_new_line_before_members_in_object_initializers = true
csharp_new_line_before_members_in_anonymous_types = true
csharp_new_line_between_query_expression_clauses = true

[*.xsd]
indent_style = tab
Expand Down
133 changes: 133 additions & 0 deletions src/NHibernate.Test/NHSpecificTest/GH1228/Fixture.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,133 @@
using NUnit.Framework;

namespace NHibernate.Test.NHSpecificTest.GH1228
{
public class Fixture : BugTestCase
{
[Test]
public void TestOk()
{
using (ISession s = OpenSession())
{
using (ITransaction t = s.BeginTransaction())
{
try
{
{
var queryThatWorks = s.CreateQuery(@"
SELECT ROOT FROM NHibernate.Test.NHSpecificTest.GH1228.Sheet AS ROOT
WHERE (EXISTS (FROM NHibernate.Test.NHSpecificTest.GH1228.Shelf AS inv
, ROOT.Folder AS ROOT_Folder
WHERE ROOT_Folder.Shelf = inv AND inv.Id = 1
) )
AND ROOT.Name = 'SomeName'");
queryThatWorks.List();
}
{
var queryThatWorks = s.CreateQuery(@"
SELECT ROOT FROM NHibernate.Test.NHSpecificTest.GH1228.Shelf AS ROOT
WHERE (EXISTS (FROM NHibernate.Test.NHSpecificTest.GH1228.Sheet AS sheet
, ROOT.Folders AS ROOT_Folder
WHERE ROOT_Folder = sheet.Folder AND sheet.Name = 'SomeName'
) )
AND ROOT.Id = 1");
queryThatWorks.List();
}
}
finally
{
s.Delete("from Sheet");
s.Delete("from Folder");
s.Delete("from Shelf");
t.Commit();
}
}
}
}

[Test]
public void TestWrongSql()
{
using (ISession s = OpenSession())
{
using (ITransaction t = s.BeginTransaction())
{
try
{
{
var queryThatCreatesWrongSQL = s.CreateQuery(@"
SELECT ROOT FROM NHibernate.Test.NHSpecificTest.GH1228.Sheet AS ROOT
WHERE (EXISTS (FROM NHibernate.Test.NHSpecificTest.GH1228.Shelf AS inv
JOIN ROOT.Folder AS ROOT_Folder
WHERE ROOT_Folder.Shelf = inv AND inv.Id = 1
) )
AND ROOT.Name = 'SomeName'");
queryThatCreatesWrongSQL.List();
}
{
// The only assertion here is that the generated SQL is valid and can be executed.
// Right now, the generated SQL is missing the JOIN inside the subselect (EXISTS) to Folder.
var queryThatCreatesWrongSQL = s.CreateQuery(@"
SELECT ROOT FROM NHibernate.Test.NHSpecificTest.GH1228.Shelf AS ROOT
WHERE (EXISTS (FROM NHibernate.Test.NHSpecificTest.GH1228.Sheet AS sheet
JOIN ROOT.Folders AS ROOT_Folder
WHERE ROOT_Folder = sheet.Folder AND sheet.Name = 'SomeName'
) )
AND ROOT.Id = 1");
queryThatCreatesWrongSQL.List();
// The only assertion here is that the generated SQL is valid and can be executed.
// Right now, the generated SQL is missing the JOIN inside the subselect (EXISTS) to Folder.
}
}
finally
{
s.Delete("from Sheet");
s.Delete("from Folder");
s.Delete("from Shelf");
t.Commit();
}
}
}
}

[Test]
public void Test3() {
using (ISession s = OpenSession()) {
using (ITransaction t = s.BeginTransaction()) {
try {
{
// The only assertion here is that the generated SQL is valid and can be executed.
// Right now, the generated SQL is missing the JOIN inside the subselect (EXISTS) to Folder.
var queryThatCreatesWrongSQL = s.CreateQuery(@"
SELECT ROOT FROM NHibernate.Test.NHSpecificTest.GH1228.Shelf AS ROOT
WHERE (EXISTS (FROM NHibernate.Test.NHSpecificTest.GH1228.Sheet AS sheet
JOIN sheet.Folder AS folder
WHERE folder.Shelf = ROOT AND sheet.Name = 'SomeName'
) )
AND ROOT.Id = 1");
queryThatCreatesWrongSQL.List();
// The only assertion here is that the generated SQL is valid and can be executed.
// Right now, the generated SQL is missing the JOIN inside the subselect (EXISTS) to Folder.
}
{
var queryThatCreatesWrongSQL = s.CreateQuery(@"
SELECT ROOT FROM NHibernate.Test.NHSpecificTest.GH1228.Sheet AS ROOT
WHERE (EXISTS (FROM NHibernate.Test.NHSpecificTest.GH1228.Shelf AS inv
JOIN inv.Folders AS folder
WHERE folder = ROOT.Folder AND inv.Id = 1
) )
AND ROOT.Name = 'SomeName'");
queryThatCreatesWrongSQL.List();
}
}
finally {
s.Delete("from Sheet");
s.Delete("from Folder");
s.Delete("from Shelf");
t.Commit();
}
}
}
}
}
}
37 changes: 37 additions & 0 deletions src/NHibernate.Test/NHSpecificTest/GH1228/Mappings.hbm.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
<?xml version="1.0" encoding="utf-8" ?>
<hibernate-mapping xmlns="urn:nhibernate-mapping-2.2"
namespace="NHibernate.Test.NHSpecificTest.GH1228"
assembly="NHibernate.Test"
>
<class name="Shelf" >
<id name="Id">
<generator class="increment"/>
</id>
<property name="Name"/>
<set name="Folders" lazy="true" cascade="all-delete-orphan" inverse="true">
<key column="Shelf"/>
<one-to-many class="Folder"/>
</set>
</class>

<class name="Folder" >
<id name="Id">
<generator class="increment"/>
</id>
<property name="Name"/>
<many-to-one name="Shelf" class="Shelf"/>
<set name="Sheets" lazy="true" cascade="all-delete-orphan" inverse="true">
<key column="Folder"/>
<one-to-many class="Sheet"/>
</set>
</class>

<class name="Sheet" >
<id name="Id">
<generator class="increment"/>
</id>
<property name="Name"/>
<many-to-one name="Folder" class="Folder"/>
</class>

</hibernate-mapping>
26 changes: 26 additions & 0 deletions src/NHibernate.Test/NHSpecificTest/GH1228/Model.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
using System.Collections.Generic;

namespace NHibernate.Test.NHSpecificTest.GH1228
{
public class Shelf
{
public virtual int Id { get; set; }
public virtual string Name { get; set; }
public virtual ISet<Folder> Folders { get; protected set; } = new HashSet<Folder>();
}

public class Folder
{
public virtual int Id { get; set; }
public virtual Shelf Shelf { get; set; }
public virtual string Name { get; set; }
public virtual ISet<Sheet> Sheets { get; protected set; } = new HashSet<Sheet>();
}

public class Sheet
{
public virtual int Id { get; set; }
public virtual Folder Folder { get; set; }
public virtual string Name { get; set; }
}
}
5 changes: 5 additions & 0 deletions src/NHibernate/Hql/Ast/ANTLR/HqlSqlWalker.cs
Original file line number Diff line number Diff line change
Expand Up @@ -736,6 +736,11 @@ void CreateFromJoinElement(

HandleWithFragment(fromElement, with);
}

if (fromElement.Parent == null)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't it be done where the fromElement is created?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, but there are cases where the item is added to the tree explicitly in FromElement.SetOrigin but there is an else branch with comment
// Otherwise, the destination node was implied by the FROM clause and the FROM clause processor // will automatically add it in the right place.

But at least for the implied join in an explicit join no code took care of adding the item to the tree.

I tried to identify the case and go to the previous else if branch (where the comment HHH-276 is located) but that caused one legacy unit test to fail.

As there is no clear responsibility for how the element should be added to the tree, I found it most easy to add it where I did. It's some kind of catch all cases where the item is not added yet which one can find good or bad. But as I said, there is no clear responsibility so I tended towards the feeling: it's a good thing.

{
fromElement.FromClause.AddChild(fromElement);
}
}

if ( log.IsDebugEnabled() )
Expand Down