Skip to content

One to One constrained relation with PropertyRef causes N+1 #3292

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
Madajevas opened this issue Apr 25, 2023 · 6 comments
Closed

One to One constrained relation with PropertyRef causes N+1 #3292

Madajevas opened this issue Apr 25, 2023 · 6 comments

Comments

@Madajevas
Copy link

Consider code bellow:

private ISessionFactory sessionFactory;

[SetUp]
public void SetUp()
{
    var config = Fluently.Configure()
        .Database(MsSqlConfiguration.MsSql2008.ConnectionString("Server=localhost;Database=test;User Id=sa;Password=Test4000;"))
        .Mappings(m => m.FluentMappings.AddFromAssemblyOf<NPlusOneWithQueryButNotQueryOver>())
        .BuildConfiguration();

    sessionFactory = config.BuildSessionFactory();
}

[Test]
public void Query()
{
    using var session = sessionFactory.OpenSession(new LoggingInterceptor());

    var entities = session.Query<EntityOne>().ToList();
}

public class EntityOne
{
    public virtual int Id { get; set; }
    public virtual string Name { get; set; }
    public virtual EntityTwo EntityTwo { get; set; }
}

public class EntityTwo
{
    public virtual int Id { get; set; }
    public virtual string Name { get; set; }
    public virtual EntityOne EntityOne { get; set; }
}

public class EntityOneMap : ClassMap<EntityOne>
{
    public EntityOneMap()
    {
        Id(e => e.Id).GeneratedBy.Identity();
        Map(e => e.Name);
        HasOne(e => e.EntityTwo)
            .PropertyRef(x => x.EntityOne)
            .Constrained()
            .LazyLoad()
            .Cascade.Delete()
            .Not.ForeignKey();
    }
}

public class EntityTwoMap : ClassMap<EntityTwo>
{
    public EntityTwoMap()
    {
        Id(e => e.Id).GeneratedBy.Identity();
        Map(e => e.Name);
        References(e => e.EntityOne)
            .Unique()
            .Not.Nullable();
    }
}

public class LoggingInterceptor : EmptyInterceptor
{
    public override SqlString OnPrepareStatement(SqlString sql)
    {
        Console.WriteLine(sql);

        return sql;
    }
}

With schema and data:

drop table if exists EntityOne;
create table EntityOne (
	id int primary key,
	name varchar(10)
);

drop table if exists EntityTwo;
create table EntityTwo (
	id int primary key,
	EntityOne_id int not null,
	name varchar(10)
);

insert into EntityOne values (1, 'e1_1'), (2, 'e1_2'), (3, 'e1_3');
insert into EntityTwo values (4, 1, 'e2_1'), (5, 2, 'e2_2'), (6, 3, 'e2_3');

When test is executed, four queries can be found in test output: one to retrieve records from "EntityOne" table, and three queries for each "EntityTwo".
If .PropertyRef(x => x.EntityOne) is removed, only one query is made. And now, if .Fetch(e => e.EntityTwo) is added, query with join clause is made, however joins are happening on id fields, not EntityOne.Id = EntityTwo.EntityOne_id.

Is this how .Constrined() with .PropertyRef() supposed to work? If so, can limitation not be able join on specified field be explained?

@bahusoid
Copy link
Member

bahusoid commented Apr 25, 2023

I think this issue is similar to #2716

To auto join one-to-one associations, we need some logic that checks all mapping associations for loading entities and adds missing required joins.

Such logic is implemented for Criteria/QueryOver and EntityLoader(session.Get/lazy loading)). But not implemented for LINQ.

So there is a big chance that's QueryOver version session.QueryOver<EntityOne>().ToList(); works as you want.

@Madajevas
Copy link
Author

I have checked QueryOver too. It behaves exactly the same:

[Test]
public void QueryOver()
{
    using var session = sessionFactory.OpenSession(new LoggingInterceptor());

    var entities = session.QueryOver<EntityOne>().List();
}

Query does:

select entityone0_.Id as id1_1_, entityone0_.Name as name2_1_ from [EntityOne] entityone0_
SELECT entitytwo0_.Id as id1_2_0_, entitytwo0_.Name as name2_2_0_, entitytwo0_.EntityOne_id as entityone3_2_0_ FROM [EntityTwo] entitytwo0_ WHERE entitytwo0_.EntityOne_id=?
SELECT entitytwo0_.Id as id1_2_0_, entitytwo0_.Name as name2_2_0_, entitytwo0_.EntityOne_id as entityone3_2_0_ FROM [EntityTwo] entitytwo0_ WHERE entitytwo0_.EntityOne_id=?
SELECT entitytwo0_.Id as id1_2_0_, entitytwo0_.Name as name2_2_0_, entitytwo0_.EntityOne_id as entityone3_2_0_ FROM [EntityTwo] entitytwo0_ WHERE entitytwo0_.EntityOne_id=?

And query over:

SELECT this_.Id as id1_1_0_, this_.Name as name2_1_0_ FROM [EntityOne] this_
SELECT entitytwo0_.Id as id1_2_0_, entitytwo0_.Name as name2_2_0_, entitytwo0_.EntityOne_id as entityone3_2_0_ FROM [EntityTwo] entitytwo0_ WHERE entitytwo0_.EntityOne_id=?
SELECT entitytwo0_.Id as id1_2_0_, entitytwo0_.Name as name2_2_0_, entitytwo0_.EntityOne_id as entityone3_2_0_ FROM [EntityTwo] entitytwo0_ WHERE entitytwo0_.EntityOne_id=?
SELECT entitytwo0_.Id as id1_2_0_, entitytwo0_.Name as name2_2_0_, entitytwo0_.EntityOne_id as entityone3_2_0_ FROM [EntityTwo] entitytwo0_ WHERE entitytwo0_.EntityOne_id=?

Double checked with SQL Server Profiler.

@Madajevas Madajevas changed the title One to One constained relation with PropertyRef causes N+1 One to One constrained relation with PropertyRef causes N+1 Apr 25, 2023
@bahusoid
Copy link
Member

bahusoid commented Apr 25, 2023

I have checked QueryOver too. It behaves exactly the same:

Ok. Then nhibernate is not smart enough and only explicit fetch="join" mapping are processed. So Mapping for one-to-one should also be updated with fetch="join". Should be easy to fix for QueryOver.

@Madajevas
Copy link
Author

@bahusoid you are right. Adding .Fetch.Join() solved it for QueryOver (generates join even without .Constrained()). Are there any plans implementing this behavior with IQueryable?

@bahusoid
Copy link
Member

bahusoid commented Apr 25, 2023

Are there any plans

There is no roadmap. You need it you do it and contribute it. That's how it works now. I can work on it but only if someone is sponsoring this job. But it's not a trivial task.

Another way to solve n+1 issue is to enable batch loading for your one-to-one entity (like adding BatchSize(100); to EntityTwoMap). See docs. In theory it should reduce additional queries for EntityTwo to single (depending on your batch size) IN query

@Madajevas
Copy link
Author

Thanks for your insights.

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