Global query filter generates wrong sql command

[alirezanet]

global query filters generate the wrong SQL query when Entity has multiple references to another table.

Steps to reproduce with example

// _Skip_Condition = true;
// _ValidIDs = int[] { 1 , 2 , 3 , 4}
// Receiver and Sender are two object of One Entity (eg MyEntity2)
builder.Entity<MyEntity>().HasQueryFilter(q =>
              (_Skip_Condition ||
              (_ValidIDs.Contains(q.Reciever.Address.Id)) ||
              (_ValidIDs.Contains(q.Sender.Address.Id)))
            );

this global filter generate two seprate INNER JOIN and causing null result even when _Skip_Condition is Ture (having multiple INNER JOIN here ignores OR conditions)

the generated SQL is something like this

SELECT * FROM `MyEntity` AS `w`
INNER JOIN (
    SELECT *
    FROM `MyEntity2` AS `i`
    LEFT JOIN `Addresses` AS `i0` ON `i`.`Id` = `i0`.`MyEntity2Id`
    WHERE `i`.`Id` IN (1 , 2 , 3 , 4)
) AS `t` ON `w`.`RecieverId` = `t`.`Id`
INNER JOIN (
    SELECT *
    FROM `MyEntity2` AS `i1`
    LEFT JOIN `Addresses` AS `i2` ON `i1`.`Id` = `i2`.`MyEntity2Id`
    WHERE `i2`.`Id` IN (1 , 2 , 3 , 4)
) AS `t0` ON `w`.`SenderId` = `t0`.`Id`
WHERE ((TRUE OR (TRUE = FALSE)) OR (TRUE = FALSE)) 
LIMIT 2

Exprected result

 SELECT * FROM `MyEntity` AS `w`
 LEFT JOIN `MyEntity2` AS `i` ON `w`.`RecieverId` = `i`.`Id`
 LEFT JOIN `MyEntity2` AS `i0` ON `w`.`SenderId` = `i0`.`Id`
 LEFT JOIN `Addresses` AS `i1` ON `i`.`Id` = `i1`.`MyEntity2Id`
 LEFT JOIN `Addresses` AS `i2` ON `i0`.`Id` = `i2`.`MyEntity2Id`
 WHERE (`i1`.`Id` IN (1, 2, 3, 4) OR `i2`.`Id` IN (1,2, 3, 4))
 LIMIT 2

Further technical details

EF Core version: 3.1.1
Database provider: (e.g. Pomelo.EntityFrameworkCore.MySql)
Target framework: (e.g. .NET Core 3.1)

tip

this code was working correctly in previous versions (EF core 2.1).

UPDATE

I think this is happening because MyEntity2 has some other query filters too. but anyway LEFT JOIN in this case can solve the problem

[alirezanet]

Is there any way to temporarily override this behavior until we have an official fix for this issue?
the problem is we built a complete user access control system on top of the GlobalQueryFilter feature in our production App, therefore we can not change the entire system and stop using global query filters also it's not possible to update anything because of this bug. I appreciate it if you could provide a solution.
@ajcvickers
@maumar

[maumar]

@alirezanet there is no way currently to alter query filter behavior, without changing the model. You can get LEFT JOIN by making Reciever and Sender relationships optional.

Also, can you provide full repro code? I tried to reverse-engineer the scenario based on the code you provided, ended up with this:

        [ConditionalFact]
        public virtual void Test19764()
        {
            using var ctx = new Context19764();
            ctx.Database.EnsureDeleted();
            ctx.Database.EnsureCreated();

            var a1 = new Address19764 { Name = "address1" };
            var a2 = new Address19764 { Name = "address2" };

            var e2_1 = new MyEntity2_19764 { Address = a1 };
            var e2_2 = new MyEntity2_19764 { Address = a2 };

            var e1_1 = new MyEntity19764 { Reciever = e2_1, Sender = e2_2 };

            ctx.Entities.Add(e1_1);
            ctx.Entities2.AddRange(e2_1, e2_2);
            ctx.Addresses.AddRange(a1, a2);
            ctx.SaveChanges();

           
            var query = ctx.Entities.ToList();
        }

        public class Context19764 : DbContext
        {
            public DbSet<MyEntity19764> Entities { get; set; }
            public DbSet<MyEntity2_19764> Entities2 { get; set; }
            public DbSet<Address19764> Addresses { get; set; }

            protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
            {
                optionsBuilder.UseSqlServer(@"Server=.;Database=Repro19764;Trusted_Connection=True;MultipleActiveResultSets=True");
            }

            public bool _Skip_Condition = false;
            public List<int> _ValidIDs = new List<int> { 1, 2, 3, 4 };

            protected override void OnModelCreating(ModelBuilder modelBuilder)
            {
                modelBuilder.Entity<MyEntity19764>().HasOne(e => e.Sender).WithOne().HasForeignKey<MyEntity19764>(e => e.SenderId).OnDelete(DeleteBehavior.NoAction);
                modelBuilder.Entity<MyEntity19764>().HasOne(e => e.Reciever).WithOne().HasForeignKey<MyEntity19764>(e => e.RecieverId).OnDelete(DeleteBehavior.NoAction);
                modelBuilder.Entity<MyEntity2_19764>().HasOne(e => e.Address).WithOne().HasForeignKey<Address19764>(a => a.MyEntity2_19764Id);

                modelBuilder.Entity<MyEntity19764>().HasQueryFilter(q =>
                  (_Skip_Condition ||
                  (_ValidIDs.Contains(q.Reciever.Address.Id)) ||
                  (_ValidIDs.Contains(q.Sender.Address.Id))));
            }
        }

        public class MyEntity19764
        {
            public int Id { get; set; }

            public int RecieverId { get; set; }
            public MyEntity2_19764 Reciever { get; set; }


            public int SenderId { get; set; }
            public MyEntity2_19764 Sender { get; set; }
        }

        public class MyEntity2_19764
        {
            public int Id { get; set; }
            public Address19764 Address { get; set; }
        }

        public class Address19764
        {
            public int Id { get; set; }
            public string Name { get; set; }

            public int? MyEntity2_19764Id { get; set; }
        }

and I'm getting the following query (on Sql Server using current master branch) :

SELECT [e].[Id], [e].[RecieverId], [e].[SenderId]
FROM [Entities] AS [e]
INNER JOIN [Entities2] AS [e0] ON [e].[RecieverId] = [e0].[Id]
LEFT JOIN [Addresses] AS [a] ON [e0].[Id] = [a].[MyEntity2_19764Id]
INNER JOIN [Entities2] AS [e1] ON [e].[SenderId] = [e1].[Id]
LEFT JOIN [Addresses] AS [a0] ON [e1].[Id] = [a0].[MyEntity2_19764Id]
WHERE ((@__ef_filter___Skip_Condition_0 = CAST(1 AS bit)) OR [a].[Id] IN (1, 2, 3, 4)) OR [a0].[Id] IN (1, 2, 3, 4)

Which returns correct results.
I wonder if there is issue with pomelo provider, my reverse engineering missed some crucial info or the bug was fixed somewhere between 3.1.1 and current master.

[alirezanet]

thank you @maumar. I tried to simplify the code in my example. my real code is :

             builder.Entity<Industry>().HasQueryFilter(q =>
             (_uai.UserDataClaims._Skip_industry ||
             (_uai.UserDataClaims.Industry_id.Contains(q.Id)) ||
             (_uai.UserDataClaims.Industry_province.Contains(q.WorkshopAddress.ProvinceId)) ||
             (_uai.UserDataClaims.Industry_state.Contains(q.WorkshopAddress.StateId)) ||
             (_uai.UserDataClaims.Industry_isic2.Contains(q.IsicCodeId)) ||
             (_uai.UserDataClaims.Industry_isic4.Contains(q.IsicCode.ParentId))) &&
             _uai.UserClaims.Intersect(new string[] { "IndustryFull", "IndustryView", "god" }).Any()
            );

            builder.Entity<WasteTransfer>().HasQueryFilter(q =>
             _uai.UserDataClaims._Skip_wastetransfer ||
(_uai.UserDataClaims.Wastetransfer_reciever_province.Contains(q.RecieverIndustry.WorkshopAddress.ProvinceId)) ||
             (_uai.UserDataClaims.Wastetransfer_sender_province.Contains(q.SenderIndustry.WorkshopAddress.ProvinceId))
            );

MyEntity is an example of WasteTransfer entity
MyEntity2 is an example of Industry entity
_uai is a service that I injected to the dbContext to get user permissions to fetch data from the database.

I don't know this is relevant or not but I think this is happening because I have some other query filters on my industry entity too (MyEntity2). in my real code return value is null only when the user doesn't have access to either SenderIndustry or ReceiverIndustry (because it creates two separate SELECT statements for inner joins).

is this test pushed to the master? maybe I can help to reproduce the problem and update the test.
or I can upload the entire data layer if you want to check?

[alirezanet]

can you update the test like this :

            public bool _Skip_Condition = true;
            public bool _Skip_Condition2 = false;
            public List<int> _ValidIDs = new List<int> { 1, 2, 3, 4 };
            public List<int> _ValidIDs2 = new List<int> { 5, 6, 7, 8 };   
            public List<int> _ValidIDs3 = new List<int> { 9, 10, 11, 12 };
            
            protected override void OnModelCreating(ModelBuilder modelBuilder)
            {
                modelBuilder.Entity<MyEntity19764>().HasOne(e => e.Sender).WithOne().HasForeignKey<MyEntity19764>(e => e.SenderId).OnDelete(DeleteBehavior.NoAction);
                modelBuilder.Entity<MyEntity19764>().HasOne(e => e.Reciever).WithOne().HasForeignKey<MyEntity19764>(e => e.RecieverId).OnDelete(DeleteBehavior.NoAction);
                modelBuilder.Entity<MyEntity2_19764>().HasOne(e => e.Address).WithOne().HasForeignKey<Address19764>(a => a.MyEntity2_19764Id);
                
                modelBuilder.Entity<MyEntity2_19764>().HasQueryFilter(q =>
                  (_Skip_Condition2 ||
                  (_ValidIDs2.Contains(q.Address.Id)) ||
                  (_ValidIDs.Contains(q.Address.Id))));
                
                modelBuilder.Entity<MyEntity19764>().HasQueryFilter(q =>
                  (_Skip_Condition ||
                  (_ValidIDs.Contains(q.Reciever.Address.Id)) ||
                  (_ValidIDs3.Contains(q.Sender.Address.Id))));
            }

[maumar]

I'm able to reproduce this now

[smitpatel]

@maumar - Post repro please.

[maumar]

        [ConditionalFact]
        public virtual void Test19764()
        {
            using var ctx = new Context19764();
            ctx.Database.EnsureDeleted();
            ctx.Database.EnsureCreated();

            var a1 = new Address19764 { Name = "address1" };
            var a2 = new Address19764 { Name = "address2" };

            var e2_1 = new MyEntity2_19764 { Address = a1 };
            var e2_2 = new MyEntity2_19764 { Address = a2 };


            var e1_1 = new MyEntity19764 { Reciever = e2_1, Sender = e2_2 };

            ctx.Entities.Add(e1_1);
            ctx.Entities2.AddRange(e2_1, e2_2);
            ctx.Addresses.AddRange(a1, a2);
            ctx.SaveChanges();

            var query = ctx.Entities.ToList();
        }

        public class Context19764 : DbContext
        {
            public DbSet<MyEntity19764> Entities { get; set; }
            public DbSet<MyEntity2_19764> Entities2 { get; set; }
            public DbSet<Address19764> Addresses { get; set; }

            protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
            {
                optionsBuilder.UseSqlServer(@"Server=.;Database=Repro19764;Trusted_Connection=True;MultipleActiveResultSets=True");
            }

            public bool _Skip_Condition = true;
            public bool _Skip_Condition2 = false;
            public List<int> _ValidIDs = new List<int> { 1, 2, 3, 4 };
            public List<int> _ValidIDs2 = new List<int> { 5, 6, 7, 8 };
            public List<int> _ValidIDs3 = new List<int> { 9, 10, 11, 12 };

            //public bool _Skip_Condition = false;
            //public List<int> _ValidIDs = new List<int> { 1, 2, 3, 4 };

            protected override void OnModelCreating(ModelBuilder modelBuilder)
            {
                modelBuilder.Entity<MyEntity19764>().HasOne(e => e.Sender).WithOne().HasForeignKey<MyEntity19764>(e => e.SenderId).OnDelete(DeleteBehavior.NoAction);
                modelBuilder.Entity<MyEntity19764>().HasOne(e => e.Reciever).WithOne().HasForeignKey<MyEntity19764>(e => e.RecieverId).OnDelete(DeleteBehavior.NoAction);
                modelBuilder.Entity<MyEntity2_19764>().HasOne(e => e.Address).WithOne().HasForeignKey<Address19764>(a => a.MyEntity2_19764Id);



                modelBuilder.Entity<MyEntity2_19764>().HasQueryFilter(q =>
                  //_Skip_Condition2 ||
                  q.Address.Id == 3);

                modelBuilder.Entity<MyEntity19764>().HasQueryFilter(q =>
                  _Skip_Condition ||
                  q.Reciever.Address.Id == 1);
            }
        }

        public class MyEntity19764
        {
            public int Id { get; set; }

            public int RecieverId { get; set; }
            public MyEntity2_19764 Reciever { get; set; }


            public int SenderId { get; set; }
            public MyEntity2_19764 Sender { get; set; }
        }

        public class MyEntity2_19764
        {
            public int Id { get; set; }
            public Address19764 Address { get; set; }
        }

        public class Address19764
        {
            public int Id { get; set; }
            public string Name { get; set; }

            public int? MyEntity2_19764Id { get; set; }
        }

[maumar]

per design discussion, added a model validation phase that warns against using required navigations where required side has the query filter and the optional side doesn't (tracked by #19801). Also improved docs to illustrate the issue, closing.