Efcore: DbParameter naming convention for FromSql(FormattableString) broken in 2.2

Created on 15 Dec 2018  路  10Comments  路  Source: dotnet/efcore

In EF Core 2.1.4, you could create a DbParameter called "p0" and it would automatically get the "@" prefix added so in the SQL, so the parameter would appear as "@p0".

Starting with EF Core 2.2, if you pass a DbParameter called "p0", the SQL will contain "p0" without the "@" prefix an error on query execution.

For example, from the sources in https://github.com/divega/ContainsOptimization/

var query = context.People.FromSql(
   $"select * from People p where p.Id in(select * from {context.CreateTableValuedParameter(ids, "p0")});

Generated SQL in 2.1:

exec sp_executesql N'select * from People p where p.Id in(select * from @p0)

Generated SQL in 2.2:

exec sp_executesql N'select * from People p where p.Id in(select * from p0)

This causes this error (but I am sure you can get different errors depending on the query):

System.Data.SqlClient.SqlException
  HResult=0x80131904
  Message=Invalid object name 'p0'.
  Source=Core .Net SqlClient Data Provider

The workaround is to add the "@" to the name explicitly.

var query = context.People.FromSql(
   $"select * from People p where p.Id in(select * from {context.CreateTableValuedParameter(ids, "@p0")});

The breaking change may also affect other variations of the API, e.g. the one that takes a raw string but uses {n} placeholders, and almost for sure, ExecuteSqlCommand.

closed-fixed type-bug

Most helpful comment

I would like to call out that if you are writing your own SQL, you never need to prefix the parameter name with "@". For example, this works:

  var param = new SqlParameter("id", id);
  var results = Set<Name>().FromRawSql("dbo.SomeProc @id", param);

In my own experience as an ADO.NET user (mostly with SqlClient) for many years, I have a strong expectation that I don't need to do it.

All 10 comments

Note for triage: I am filing this issue because it is a breaking change and I cannot remember if we made an explicit decision on this. Also, I discovered while working on some experiments with Contains, and I haven't tried to create a simpler repro.

@smitpatel to investigate what we did to break this.

PR: https://github.com/aspnet/EntityFrameworkCore/pull/13127

Which fixed bug https://github.com/aspnet/EntityFrameworkCore/issues/12067
If you change parameter name from p0 to something else, it shows same bug as #12067

We started allowing DbParameters in FromSql since https://github.com/aspnet/EntityFrameworkCore/issues/8721
But current code requires user to pass in parameter name with @

Triage: We're leaning towards requiring that the "@" be included, possibly with better validation on the EF side. We could instead allow the provider to "correct" the parameter name, but there seems little value in doing so.

Some notes: ISqlGenerationHelper.GenerateParameterName() generates SQL parameter references. Microsoft.Data.Sqlite has some logic for allowing parameter names with or without a prefix, but it may not be reusable in this context.

I would like to call out that if you are writing your own SQL, you never need to prefix the parameter name with "@". For example, this works:

  var param = new SqlParameter("id", id);
  var results = Set<Name>().FromRawSql("dbo.SomeProc @id", param);

In my own experience as an ADO.NET user (mostly with SqlClient) for many years, I have a strong expectation that I don't need to do it.

@divega I agree, although I think most providers actually support both styles (with or without a prefix in the DbParameter). There's even a sentence in the ADO.NET DataSourceInformation schema collection docs that refers to this (see ParameterMarkerFormat).

(not that this matters much, just posting it because I know how much you like that API :))

We need to add the @ if it is not there.

15890 should fix this.

Was this page helpful?
0 / 5 - 0 ratings