Skip to content

NH-3435 - Ability to select the root entity in a criteria projection #302

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

Closed
wants to merge 1 commit into from
Closed

Conversation

rjperes
Copy link
Member

@rjperes rjperes commented Aug 14, 2014

@oskarb
Copy link
Member

oskarb commented Aug 17, 2014

I'm afraid I don't follow... This appears to be only new code - what is being cleaned up?

@hazzik
Copy link
Member

hazzik commented Aug 17, 2014

There was a broken pull request #299

@oskarb oskarb changed the title Cleaning up pull request NH-3435 Ability to select the root entity in a criteria projection Sep 2, 2014
@rjperes rjperes mentioned this pull request Sep 21, 2014
@hazzik
Copy link
Member

hazzik commented Sep 21, 2014

Hi @rjperes,

I've cleaned up your old pull request with:

D:\Projects\NHibernate\nhibernate-core>git checkout NH-3435
Branch NH-3435 set up to track remote branch NH-3435 from rjperes.
Switched to a new branch 'NH-3435'

D:\Projects\NHibernate\nhibernate-core>git rebase upstream/master -Xrenormalize
First, rewinding head to replay your work on top of it...
Auto-merging src/NHibernate/NHibernate.csproj
Auto-merging src/NHibernate/Criterion/Projections.cs
Auto-merging src/NHibernate.Test/Criteria/ProjectionsTest.cs
[detached HEAD c267a58] Cleaning up pull request
Author: Ricardo Peres [email protected]
6 files changed, 264 insertions(+)
create mode 100644 src/NHibernate/Criterion/BaseEntityProjection.cs
create mode 100644 src/NHibernate/Criterion/EntityProjection.cs
create mode 100644 src/NHibernate/Criterion/RootEntityProjection.cs
Committed: 0001 Cleaning up pull request
All done.

D:\Projects\NHibernate\nhibernate-core>git push rjperes NH-3435:NH-3435 -f
Counting objects: 22, done.
Delta compression using up to 8 threads.
Compressing objects: 100% (12/12), done.
Writing objects: 100% (13/13), 2.84 KiB | 0 bytes/s, done.
Total 13 (delta 10), reused 3 (delta 1)
To [email protected]:rjperes/nhibernate-core.git

@hazzik
Copy link
Member

hazzik commented Oct 9, 2014

Restarting tests

@rjperes
Copy link
Member Author

rjperes commented Oct 10, 2014

BTW: can you lower the priority from Major to something else? And will it be in 4.1?

@hazzik hazzik added this to the 4.1.0 milestone Nov 18, 2014
@hazzik hazzik changed the title NH-3435 Ability to select the root entity in a criteria projection NH-3435 - Ability to select the root entity in a criteria projection Apr 22, 2015
@hazzik hazzik closed this Apr 22, 2015
@hazzik hazzik reopened this Apr 22, 2015
@danspam
Copy link
Contributor

danspam commented Mar 1, 2016

Looks to me like this new code only selects the identifier of the root entity and not all the properties.

@rjperes
Copy link
Member Author

rjperes commented Mar 1, 2016

Did you try it?

@danspam
Copy link
Contributor

danspam commented Mar 1, 2016

Yes I did, and the SQL produced only selects the identifier. The way I read it, the code that retrieves the column names and alias name in IProjection.ToSqlString() just retrieves the identifier column name and alias. The pull request #299 mentioned earlier appears to have code that iterates and retrieves all properties of the entity but this pr does not have this.

Unfortunately my knowledge of this code base is non-existent and I would not dare hazard a guess at the fix for this.

@rjperes
Copy link
Member Author

rjperes commented Mar 1, 2016

This works as expected, AFAIK.
Did you see the unit tests? Are they failing?

@danspam
Copy link
Contributor

danspam commented Mar 2, 2016

The unit tests are not failing. However, I thought that the purpose of this change was to be able to create a projection of all the properties of the specified entity . This is useful if you have multiple joins in the query and only want to retrieve e.g. the root entity without creating a sql select statement with every property of all the joined entities.

When I run the unit test UseRootProjection(), the generated SQL statement of the select query using Projections.RootEntity() is as follows:

`SELECT
    this_."studentId" as studentId2_ 
FROM
    Student this_ 
WHERE
    this_.studentId = @p0;
@p0 = 667 [Type: Int64 (0)]`

This means that if the entity is not in cache, accessing any of the entities properties will cause an additional query to retrieve the rest of the properties.

@hazzik hazzik modified the milestones: 4.2.0, 4.1.0 Aug 12, 2016
@hazzik hazzik modified the milestones: 4.2.0, 5.0.0 Dec 1, 2016
@hazzik hazzik modified the milestones: 5.0, 5.1 Aug 25, 2017
@bahusoid
Copy link
Member

I also really miss the ability to eager fetch entities via QueryOver/Criteria API.
And as @danspam pointed out - this promising pr doesn't allow it either :(.

Regarding unit tests - they check object loading that's already present in session. So tests don't really check that all properties are fetched.

I updated tests to show the issue:

Updated tests
[Test]
public void UseRootProjection()
{
	//NH-3435
	using (ISession session = Sfi.OpenSession())
	using (ITransaction tx = session.BeginTransaction())
	{
		Course course = new Course();
		course.CourseCode = "HIB";
		course.Description = "Hibernate Training";
		session.Save(course);

		Student gavin = new Student();
		gavin.Name = "Gavin King";
		gavin.StudentNumber = 667;
		session.Save(gavin);

		Enrolment enrolment = new Enrolment();
		enrolment.Course = course;
		enrolment.CourseCode = course.CourseCode;
		enrolment.Semester = 1;
		enrolment.Year = 1999;
		enrolment.Student = gavin;
		enrolment.StudentNumber = gavin.StudentNumber;
		gavin.Enrolments.Add(enrolment);
		session.Save(enrolment);
		session.Flush();
				
		//Clear session to make sure entity is eager fetched
		session.Clear();

		Student g = session.CreateCriteria(typeof(Student))
			.Add(Expression.IdEq(gavin.StudentNumber))
			.SetFetchMode("Enrolments", FetchMode.Join)
			.SetProjection(Projections.RootEntity())
			.UniqueResult<Student>();

		Assert.That(NHibernateUtil.IsInitialized(g), Is.True, "object must be initialized");
		Assert.That(g, Is.EqualTo(gavin).Using((Student x, Student y) => x.StudentNumber == y.StudentNumber && x.Name == y.Name ? 0 : 1));
	}
}

[Test]
public void UseEntityProjection()
{
	//NH-3435
	using (ISession session = Sfi.OpenSession())
	using (ITransaction tx = session.BeginTransaction())
	{
		Course course = new Course();
		course.CourseCode = "HIB";
		course.Description = "Hibernate Training";
		session.Save(course);

		Student gavin = new Student();
		gavin.Name = "Gavin King";
		gavin.StudentNumber = 667;
		session.Save(gavin);

		Enrolment enrolment = new Enrolment();
		enrolment.Course = course;
		enrolment.CourseCode = course.CourseCode;
		enrolment.Semester = 1;
		enrolment.Year = 1999;
		enrolment.Student = gavin;
		enrolment.StudentNumber = gavin.StudentNumber;
		gavin.Enrolments.Add(enrolment);
		session.Save(enrolment);
		session.Flush();

		//Clear session to make sure entity is eager fetched
		session.Clear();

		Student g = session.CreateCriteria(typeof(Enrolment))
			.Add(Expression.And(Expression.Eq("StudentNumber", gavin.StudentNumber), Expression.Eq("CourseCode", course.CourseCode)))
			.CreateAlias("Student", "s", JoinType.InnerJoin)
			.SetProjection(Projections.Entity<Student>("s"))
			.UniqueResult<Student>();

		Assert.That(NHibernateUtil.IsInitialized(g), Is.True, "object must be initialized");
		Assert.That(g, Is.EqualTo(gavin).Using((Student x, Student y) => x.StudentNumber == y.StudentNumber && x.Name == y.Name ? 0 : 1));
	}
}

@bahusoid
Copy link
Member

bahusoid commented Nov 23, 2017

Updated tests can also be found here (rebased with master): https://github.com/bahusoid/nhibernate-core/commits/GH948.

@bahusoid
Copy link
Member

Yayk! It's actually possible to eager fetch. It's just not exposed for some reasons via Projections helper methods. But it's possible via:
.SetProjection(new RootEntityProjection().SetLazy(false))
.SetProjection(new EntityProjection<Student>("s").SetLazy(false))

I think SetLazy should be definitely exposed via additional parameter (or maybe Projections helper methods should simply return exact class instead of IProjection interface)

@fredericDelaporte
Copy link
Member

fredericDelaporte commented Nov 23, 2017

If you wish to tackle this subject, PR your rebase and go ahead with required changes, it will then replace this PR.

Update: I am not sure this PR was actually about eager fetching. If you change things about it, it may be better to do it as a separated PR.

@bahusoid
Copy link
Member

@fredericDelaporte No changes needed (except small changes to better expose already present functionality). This PR allows to retrieve entity as projection both ways (as lazy and eager fetched). Ok I will add separate tests for lazy and eager part and submit a new PR for this.

@hazzik
Copy link
Member

hazzik commented Nov 28, 2017

Replaced by #1458

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

Successfully merging this pull request may close these issues.

6 participants