Skip to content
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

BulkInsert ignoring session context value set before insert #581

Closed
bptillman opened this issue May 13, 2024 · 6 comments
Closed

BulkInsert ignoring session context value set before insert #581

bptillman opened this issue May 13, 2024 · 6 comments
Assignees

Comments

@bptillman
Copy link

bptillman commented May 13, 2024

Description

In our code base we have a table that has a trigger on it that checks for a value set in the session context and skips the trigger logic, otherwise it proceeds with executing the trigger logic like so:

ALTER   TRIGGER [dbo].[table1_versioning] ON [dbo].[table1]
AFTER UPDATE, DELETE, INSERT
AS
BEGIN
IF CAST(SESSION_CONTEXT(N'DisableHistoryTriggers') AS BIT) = 1 RETURN

DECLARE @Now DATETIME2 = GETUTCDATE();

UPDATE x
SET
	x.[EffectiveFromDate] = @Now,

When we perform a Bulk Insert to this table in code, we first make a call to set that session context value so that the trigger should return and not proceed with its remaining logic, but when executing the code below, we are seeing that the triggers logic after the return statement ends up being executed since afterwards when looking in the database, we see the EffectiveFromDate being set to the UTC Now Date, instead of the date we set it to before the BulkInsert.

foreach (var entityToInsert in entitiesToInsert)
{
	entityToInsert.EffectiveFromDate = new DateTime(2020, 1, 1);
}

EntityFrameworkManager.ContextFactory = _ => context;

context.Database.ExecuteSqlRaw("EXEC sp_set_session_context 'DisableHistoryTriggers', @value;", new SqlParameter("@value", 1));

var test = context.Database.SqlQueryRaw<int?>($"SELECT SESSION_CONTEXT(N'DisableHistoryTriggers');").ToList().First();
test.ShouldBe(1); //this passes

context.BulkInsert(entitiesToInsert, option =>
{
	option.IncludeGraph = includeGraph;
});

context.Database.ExecuteSqlRaw("EXEC sp_set_session_context 'DisableHistoryTriggers', NULL;");
context.CloseTransaction();

test = context.Database.SqlQueryRaw<int?>($"SELECT SESSION_CONTEXT(N'DisableHistoryTriggers');");
test.ShouldBeNull(); //passes

foreach (var entityToInsert in entitiesToInsert)
{
	entityToInsert.EffectiveFromDate.ShouldBe(new DateTime(2020, 1, 1)); //this fails
}

This all leads me to believe that a different session context is used for the BulkInsert, despite specifying to use the same existing context with the ContextFactory.

I have also tried using the PreBulkInsert event to try and set the session context value there, but it did not appear to make a difference:

                EntityFrameworkManager.PreBulkInsert = (ctx, obj) =>
                {
                    ctx.Database.ExecuteSqlRaw("EXEC sp_set_session_context 'DisableHistoryTriggers', @value;",
                        new SqlParameter("@value", 1));
                };

Further technical details

  • EF version: [EF Core v7.0.16]
  • EF Extensions version: [EFE Core v7.102.2.4]
  • Database Server version: [SQL Server 2019]
  • Database Provider version (NuGet): [Microsoft.Data.SqlClient v5.1.5]
@JonathanMagnan JonathanMagnan self-assigned this May 13, 2024
@JonathanMagnan
Copy link
Member

Hello @bptillman ,

Thank you for reporting. My developer tried to reproduce the issue, but everything was working with him.

One major difference I can see in his code is that we were opening the connection, but since you are in a transaction, I would assume you do the same.

I'm coming back from vacation next Monday, so I will look with him next week.

Best Regards,

Jon

@bptillman
Copy link
Author

bptillman commented May 14, 2024

@JonathanMagnan thank you for checking on this. I have done some more digging and I have found that this has actually has nothing to do with the session context value in my triggers and instead has to do with two other items causing the issue I'm seeing, depending on how they are set, and what version of this extension package that I am using, so I apologize for sending you down the rabbit hole of looking at that initially.

In my code, the EffectiveFromDate has a default SQL value set in our model builder like so:

public class MyDbContext : DbContext
{
	protected override void OnModelCreating(ModelBuilder modelBuilder)
	{
		modelBuilder.ApplyConfigurationsFromAssembly(GetType().Assembly)
			.ConfigureTemporalEntities();
	}
		
	ModelBuilder ConfigureTemporalEntities(this ModelBuilder modelBuilder)
	{
		var temporalEntityTypes = modelBuilder.Model.GetEntityTypes()
			.Where(x => x.ClrType.IsAssignableTo(typeof(ITemporal)));

		foreach (var entityType in temporalEntityTypes)
		{
			var effectiveFromDate = entityType.GetProperty(nameof(ITemporal.EffectiveFromDate))
				?? entityType.AddProperty(entityType.ClrType.GetProperty(nameof(ITemporal.EffectiveFromDate)));

			effectiveFromDate.SetDefaultValueSql("(sysutcdatetime())");
		}

		return modelBuilder;
	}
}

public interface ITemporal
{
	public DateTime EffectiveFromDate { get; set; }
}

public class Table1 : Entity<int>, ITemporal
{
	public Table1()
	{
	}

	public int Id { get; set; }
	public int ClientId { get; set; }
	public int TypeId { get; set; }
	public bool Deleted { get; set;}
	public int CreatedUserId { get; set; }
	public int UpdatedUserId { get; set; }
	public DateTime CreatedDate { get; set; }
	public DateTime EffectiveFromDate { get; set; }
	public string TagId { get; set; }
	public string SerialNumber { get; set; }
	public string Barcode { get; set; }
}

Having this in the code, plus having the Bulk Insert option for IncludeGraph being false appears to cause the issue of the date set in code not being persisted to the database on version 7.22.2.

I tested this out with the combinations of

  1. IncludeGraph = false, and SetDefaultValueSql being declared
  2. IncludeGraph = false, and SetDefaultValueSql not being declared
  3. IncludeGraph = true, and SetDefaultValueSql being declared
  4. IncludeGraph = true, and SetDefaultValueSql not being declared

Only under scenario 1 did I find that the value I set in my code for EffectiveFromDate not making it into the database.

Running Sql Profiler I was able to see why in that scenario, since the merge statement being run did not end up including EffectiveFromDate when IncludeGraph was false and the SetDefaultValueSql was declared, as you can see from this query I grabbed:

exec sp_executesql N'CREATE TABLE #ZZZProjects_8c31ee2f_6782_43c9_b431_fc149431c312z ( [$action] VARCHAR(100) NULL, ZZZ_Index INT NULL, [Id] [sys].[int] NULL );
MERGE INTO [dbo].[table1]  AS DestinationTable
USING
(
SELECT TOP 100 PERCENT * FROM (SELECT @0_0 AS [ClientId], @0_1 AS [TypeId], @0_2 AS [CreatedDate], @0_3 AS [CreatedUserId], @0_4 AS [SerialNumber], @0_5 AS [TagId], @0_6 AS [Deleted], @0_7 AS [ProductId], @0_8 AS [Barcode], @0_10 AS [UpdatedUserId], @0_11 AS [Id], @0_12 AS ZZZ_Index) AS StagingTable ORDER BY ZZZ_Index
) AS StagingTable
ON 1 = 2
WHEN NOT MATCHED THEN
    INSERT ( [ClientId], [TypeId], [CreatedDate], [CreatedUserId], [SerialNumber], [TagId], [Deleted], [ProductId], [Barcode], [UpdatedUserId] )
    VALUES ( [ClientId], [TypeId], [CreatedDate], [CreatedUserId], [SerialNumber], [TagId], [Deleted], [ProductId], [Barcode], [UpdatedUserId] )
OUTPUT
    $action,
    StagingTable.ZZZ_Index,
    INSERTED.[Id]
INTO #ZZZProjects_8c31ee2f_6782_43c9_b431_fc149431c312z

;
SELECT   A.* ,B.[Id] AS [Id_zzzinserted], B.[EffectiveFromDate] AS [EffectiveFromDate_zzzinserted], B.[EffectiveThruDate] AS [EffectiveThruDate_zzzinserted] FROM #ZZZProjects_8c31ee2f_6782_43c9_b431_fc149431c312z AS A
INNER JOIN [dbo].[table1] AS B  ON  A.[Id] = B.[Id]
;
DROP TABLE #ZZZProjects_8c31ee2f_6782_43c9_b431_fc149431c312z;',N'@0_0 int,@0_1 int,@0_2 datetime2(7),@0_3 int,@0_4 nvarchar(256),@0_5 nvarchar(256),@0_6 bit,@0_7 int,@0_8 varchar(512),@0_10 int,@0_11 int,@0_12 int',@0_0=135,@0_1=5,@0_2='2020-01-01 00:00:00',@0_3=0,@0_4=NULL,@0_5=NULL,@0_6=0,@0_7=1505,@0_8=NULL,@0_9=N'RFID-3b646811-563c-45c9-a5c4-d1a549685d0b',@0_10=0,@0_11=0,@0_12=0

Upon updating to 7.102.2.4 the behavior is different under the same 4 scenarios I mentioned above with IncludeGraph and the SetDefaultValueSql. On this newer version I see the following:

  1. IncludeGraph = false, and SetDefaultValueSql being declared - The incorrect date is persisted.
  2. IncludeGraph = false, and SetDefaultValueSql not being declared - The correct date is persisted.
  3. IncludeGraph = true, and SetDefaultValueSql being declared - The incorrect date is persisted.
  4. IncludeGraph = true, and SetDefaultValueSql not being declared - The correct date is persisted.

So you can see how now that as long as SetDefaultValueSql is declared, no matter if we use IncludeGraph = true or false, it does not persist the date we set it to.

We are re-evaluating the need to use SetDefaultValueSql in our code, but for now it is there and we end up needing to bulk insert different objects that have the EffectivefromDate columns with IncludeGraph = false and for others true.

@JonathanMagnan
Copy link
Member

Hello @bptillman ,

Indeed, there is a major breaking change in the IncludeGraph starting from v7.100.,0.0. You can see also it impact the default value like in your case: https://entityframework-extensions.net/v7-100-0-0-include-graph#default-value-handling

As documented, you can still have the same behavior under the latest version by using the LegacyIncludeGraph = true instead of the IncludeGraph. With the LegacyIncludeGraph, you will have the same behavior as the v7.22.2.

You can also change how the default value is handled with the option ForceValueGeneratedStrategy

We are currently improving this part when a value is provided to align more like the SaveChanges for our Bulk Insert method. So if I understand your scenario correctly, it might be supported by default during the summer.

Let me know if that explains the current behavior correctly and if my answer provides the solution you currently need.

Best Regards,

Jon

@JonathanMagnan
Copy link
Member

Hello @bptillman ,

Since our last conversation, we haven't heard from you.

Let me know if you need further assistance.

Best regards,

Jon

@bptillman
Copy link
Author

@JonathanMagnan thank you for that advice. I finally had some time to test it out, and the right combination of setting IncludeGraph and ForceValueGeneratedStrategy set to OnAddOrUpdate allowed my code to work as it did in previous versions. Thanks!

@JonathanMagnan
Copy link
Member

Awesome,

We made a lot of progress in handling default values better. I will let you know when the new version will be ready.

Best Regards,

Jon

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

No branches or pull requests

2 participants