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

SQLServer: ExecuteDelete without Where should translate to TRUNCATE TABLE #34957

Closed
bricelam opened this issue Oct 22, 2024 · 6 comments
Closed
Assignees
Labels
area-bulkcud closed-out-of-scope This is not something that will be fixed/implemented and the issue is closed.

Comments

@bricelam
Copy link
Contributor

Today, the following statement...

db.Entities.ExecuteDelete()

...translates to the following on SQL Server.

DELETE FROM [e]
FROM [Entities] AS [e]

But it would be more performant to use a truncate statement.

TRUNCATE TABLE [Entities]
@Bretttt
Copy link

Bretttt commented Oct 22, 2024

Plenty of reasons why this is fundamentally different.
https://stackoverflow.com/questions/139630/whats-the-difference-between-truncate-and-delete-in-sql.
Additionally, the contract would change because TRUNCATE does not return the number of deleted rows like the current API.
A bit of code if you want to handle this:

public static void Truncate<TEntity>(this DbSet<TEntity> dbSet)
    where TEntity : class
{
        DbContext dbContext = ((IInfrastructure<IServiceProvider>)dbSet).Instance.GetRequiredService<ICurrentDbContext>().Context;
        dbContext.Database.ExecuteSql($"TRUNCATE {dbSet.EntityType.GetSchemaQualifiedTableName()}");
}

public static Task TruncateAsync<TEntity>(this DbSet<TEntity> dbSet)
    where TEntity : class
{
    DbContext dbContext = ((IInfrastructure<IServiceProvider>)dbSet).Instance.GetRequiredService<ICurrentDbContext>().Context;
    return dbContext.Database.ExecuteSqlAsync($"TRUNCATE {dbSet.EntityType.GetSchemaQualifiedTableName()}");
}

@cincuranet
Copy link
Contributor

There's simply too many differences between DELETE and TRUNCATE, so I don't think doing that silently for this specific case is a good idea. But there might be some value in having a method like ExecuteTruncate.

cc @maumar @roji

@roji
Copy link
Member

roji commented Oct 23, 2024

Yeah, I tend to agree with @Bretttt and @cincuranet on this - IIRC there are some observable difference between the two. For example, unless I'm mistaken, identity columns are reset by TRUNCATE but not by DELETE... For cases where there are observable differences, it's probably better to also keep the .NET user-facing API distinct so that everything is clear and predictable, rather than having a weird thing where just adding/removing a Where clause suddenly leads to behavioral differences... @Bretttt's point about TRUNCATE not returning rows is also good and points in the direction of a separate API.

We can indeed add an API here, though this really is an extremely simply command to do via SQL TRUNCATE <table_name>. Sure, there's still some value in doing that for the user (and not forcing them to hardcode the table name), but there's also some API pollution cost of adding another method on DbSet, which very few people actually need... So I'm a bit on the fence here.

Note previous discussion on introducing TRUNCATE to EF in #5972.

@ErikEJ
Copy link
Contributor

ErikEJ commented Oct 23, 2024

Also, TRUNCATE requires higher rights than READ or WRITE

@bricelam
Copy link
Contributor Author

bricelam commented Oct 27, 2024

lol, clearly I’m not a SQL Server expert. I don’t think there’s much value in having a method for this. ExecuteSqlCommand is sufficient for cases where you can use it and need the perf.

Not returning the affected rows, requiring ALTER TABLE permissions, and not running DELETE triggers are definitely deal-breakers for the general case.

@roji
Copy link
Member

roji commented Oct 27, 2024

@bricelam yeah, I agree... There's the slight advantage of not having to hardcode the table name (or get it from the model), but that doesn't seem worth it... As there seems to be consensus that there isn't enough value here, I'll go ahead and close for now.

@roji roji closed this as not planned Won't fix, can't repro, duplicate, stale Oct 27, 2024
@roji roji added the closed-out-of-scope This is not something that will be fixed/implemented and the issue is closed. label Oct 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-bulkcud closed-out-of-scope This is not something that will be fixed/implemented and the issue is closed.
Projects
None yet
Development

No branches or pull requests

6 participants