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

Global API to turn off code generation if Table has triggers and optimization in code generation #27879

Closed
sandeepmgl opened this issue Apr 25, 2022 · 3 comments
Labels
closed-no-further-action The issue is closed and no further action is planned. customer-reported

Comments

@sandeepmgl
Copy link

for the breaking change related to code generation (optimization) which doesn't work with tables with triggers, we should give an API to disable this option of optimization (Trigger one) at the DB level.

for the breaking change, we should give an API to disable this option of optimization (Trigger one) at the DB level so that people can move to EF7 easily, and then they can set this at Entity (Table) level at their own pace. I hope it makes sense :)

Also, code optimization using merge can also be probably replaced with code using "insert into table", this is a small code I wrote using ADO.Net and it works. I have tested it with both Identity and sequence ID. The current commented code only has a table structure for sequence one.

using Microsoft.Data.SqlClient;

//USE[RND]
//GO

///****** Object:  Table [dbo].[TableToTestDefaultSeq]    Script Date: 4/25/2022 10:11:41 AM ******/
//SET ANSI_NULLS ON
//GO

//SET QUOTED_IDENTIFIER ON
//GO
//CREATE SEQUENCE[dbo].[PersonIdSeq]
//AS[bigint]
// START WITH 1
// INCREMENT BY 1
// MINVALUE -9223372036854775808
// MAXVALUE 9223372036854775807
// CACHE 
//GO
    
//CREATE TABLE [dbo].[TableToTestDefaultSeq] (

//[Id][bigint] NULL,
//	[Name][nvarchar] (50) NULL
//) ON[PRIMARY]
//GO

//ALTER TABLE [dbo].[TableToTestDefaultSeq] ADD CONSTRAINT[DF_PersonId]  DEFAULT (NEXT VALUE FOR [PersonIdSeq]) FOR[Id]
//GO


string connectionString =
            "Data Source=(local);Initial Catalog=RND;"
            + "Integrated Security=true; Connect Timeout=30; Encrypt=False; TrustServerCertificate=False; ApplicationIntent=ReadWrite; MultiSubnetFailover=False";

// Provide the query string with a parameter placeholder.
string queryString =
    @"insert into TableToTestDefaultSeq(Name)
    output inserted.[Id]
    values(@1),(@2),(@3)";

// Create and open the connection in a using block. This
// ensures that all resources will be closed and disposed
// when the code exits.
using var connection =
    new SqlConnection(connectionString);

// Create the Command and Parameter objects.
SqlCommand command = new SqlCommand(queryString, connection);
command.Parameters.AddWithValue("@1", "Shay");
command.Parameters.AddWithValue("@2", "Authur");
command.Parameters.AddWithValue("@3", "Jeremy");

// Open the connection in a try/catch block.
// Create and execute the DataReader, writing the result
// set to the console window.
try
{
    connection.Open();
    SqlDataReader reader = command.ExecuteReader();
    while (reader.Read())
    {
        Console.WriteLine("\t{0}", reader[0]);
    }
    reader.Close();
}
catch (Exception ex)
{
    Console.WriteLine(ex.Message);
}
Console.ReadLine();
@roji
Copy link
Member

roji commented Apr 25, 2022

Duplicate of #27531

@roji roji marked this as a duplicate of #27531 Apr 25, 2022
@roji
Copy link
Member

roji commented Apr 25, 2022

We already have a per-table way of specifying that a trigger exists (HasTrigger), which switches back to the slower technique with the temporary table. A model-global way of doing so is tracked by #27531 (need to determine whether this fits in 7.0).

Also, code optimization using merge can also be probably replaced with code using "insert into table", this is a small code I wrote using ADO.Net and it works. I have tested it with both Identity and sequence ID. The current commented code only has a table structure for sequence one.

In some cases we can replace MERGE with multi-row INSERT. However, when fetching back database-generated IDs (e.g. identity/sequence), we need to know which generated key corresponds to which entity instance, so that we can inject it; INSERT ... OUTPUT ... doesn't guarantee the ordering of the returned rows, so it's impossible to know which row belongs to which client-side instance. We use MERGE here because it allows us to also return the synthetic _Position value, which we use to locate the correct entity instance client-side. I suggest reading #27372 in detail, and especially #27372 (comment). Also, MERGE and multi-row INSERT have very similar perf characteristics (see benchmarking in #27372 (comment)), so there's no particular advantage in using the latter over the former.

@sandeepmgl
Copy link
Author

Thank you for responding, really appreciate it. If the team has already measured the performance of insert vs merge then we are good.
I hope that we get the Global API to disable the new optimized code that does not work with Trigger tables, it will help in the adoption of EF 7.

@roji roji added closed-no-further-action The issue is closed and no further action is planned. and removed type-enhancement labels Apr 26, 2022
@roji roji closed this as completed Apr 26, 2022
@ajcvickers ajcvickers reopened this Oct 16, 2022
@ajcvickers ajcvickers closed this as not planned Won't fix, can't repro, duplicate, stale Oct 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
closed-no-further-action The issue is closed and no further action is planned. customer-reported
Projects
None yet
Development

No branches or pull requests

3 participants