Query: FromSql and SqlParameter issue when using with Include methods

[punssoma]

When using FromSql with SqlParameters and Include methods, it is failing with an exception
It seems the query is executing twice and gives error

Exception message: "The SqlParameter is already contained by another SqlParameterCollection."
Stack trace:    at System.Data.SqlClient.SqlParameterCollection.Validate(Int32 index, Object value)
   at System.Data.SqlClient.SqlParameterCollection.Add(Object value)
   at Microsoft.EntityFrameworkCore.Storage.Internal.DynamicRelationalParameter.AddDbParameter(DbCommand command, Object value)
   at Microsoft.EntityFrameworkCore.Storage.Internal.CompositeRelationalParameter.AddDbParameter(DbCommand command, Object value)
   at Microsoft.EntityFrameworkCore.Storage.Internal.RelationalParameterBase.AddDbParameter(DbCommand command, IReadOnlyDictionary`2 parameterValues)
   at Microsoft.EntityFrameworkCore.Storage.Internal.RelationalCommand.CreateCommand(IRelationalConnection connection, IReadOnlyDictionary`2 parameterValues)
   at Microsoft.EntityFrameworkCore.Storage.Internal.RelationalCommand.Execute(IRelationalConnection connection, DbCommandMethod executeMethod, IReadOnlyDictionary`2 parameterValues)
   at Microsoft.EntityFrameworkCore.Storage.Internal.RelationalCommand.ExecuteReader(IRelationalConnection connection, IReadOnlyDictionary`2 parameterValues)
   at Microsoft.EntityFrameworkCore.Query.Internal.QueryingEnumerable`1.Enumerator.BufferlessMoveNext(Boolean buffer)
   at Microsoft.EntityFrameworkCore.Storage.Internal.SqlServerExecutionStrategy.Execute[TState,TResult](TState state, Func`3 operation, Func`3 verifySucceeded)
   at Microsoft.EntityFrameworkCore.Query.Internal.QueryingEnumerable`1.Enumerator.MoveNext()
   at Microsoft.EntityFrameworkCore.Query.Internal.QueryBuffer.IncludeCollection(Int32 includeId, INavigation navigation, INavigation inverseNavigation, IEntityType targetEntityType, IClrCollectionAccessor clrCollectionAccessor, IClrPropertySetter inverseClrPropertySetter, Boolean tracking, Object entity, Func`1 relatedEntitiesFactory)
   at lambda_method(Closure , QueryContext , UserFilterGroup , Object[] )
   at Microsoft.EntityFrameworkCore.Query.Internal.IncludeCompiler._Include[TEntity](QueryContext queryContext, TEntity entity, Object[] included, Action`3 fixup)
   at lambda_method(Closure , UserFilterGroup )
   at System.Linq.Enumerable.WhereSelectEnumerableIterator`2.MoveNext()
   at Microsoft.EntityFrameworkCore.Query.Internal.LinqOperatorProvider.<_TrackEntities>d__17`2.MoveNext()
   at Microsoft.EntityFrameworkCore.Query.Internal.LinqOperatorProvider.ExceptionInterceptor`1.EnumeratorExceptionInterceptor.MoveNext()
   at System.Collections.Generic.List`1..ctor(IEnumerable`1 collection)
   at System.Linq.Enumerable.ToList[TSource](IEnumerable`1 source)
   at RaptorWeb.DataRepository.UserFilterDataRepository.GetUserFilterGroups(UserFilterGroupSearchCriteria criteria)
   at RaptorWeb.Controllers.Api.UserFilterGroupApi.GetUserFilterGroups(UserFilterGroupSearchCriteria criteria)

Code

public IEnumerable<UserFilterGroup> GetUserFilterGroups(UserFilterGroupSearchCriteria criteria)
{
    List<SqlParameter> parameters = GenerateSqlParameters(criteria);
    parameters.Add(new SqlParameter("@pageNumber", (criteria.PageNumber * criteria.Rows)));
    parameters.Add(new SqlParameter("@rows", criteria.Rows));

    var query = CurrentDbContext.UserFilterGroups.FromSql(@"SELECT * FROM (" + GenerateCriteriaQuery(criteria) + @")C 
            ORDER BY NAME OFFSET @pageNumber ROWS FETCH NEXT @rows ROWS ONLY"
        , parameters.ToArray());

    return query.Include(x => x.UserFilterValues).Include(x => x.UserFilterDefaultSettings).ToList();
}

The GenerateSqlParameters function creates the parameter list

The SQl query created is :

SELECT * FROM ( SELECT * FROM RaptorSys.UserFilterGroups WHERE FilterType = @FilterType  AND SiteGuid  = @SiteGuid AND UserId = @UserId) C 
                    ORDER BY NAME OFFSET @pageNumber ROWS FETCH NEXT @rows ROWS ONLY

The method is failing at the last statement while executing the query

return query.Include(x => x.UserFilterValues).Include(x => x.UserFilterDefaultSettings).ToList();

If I remove the Include methods and just call ToList(), it is working fine

Further technical details

EF Core version: 2.0.1
Database Provider: Microsoft.EntityFrameworkCore.SqlServer
Operating system: Windows 7
IDE: (e.g. Visual Studio 2017 15.5.4)

[ajcvickers]

Putting this on the backlog to look at post 2.1.

@punssoma Can you try using automatically generated parameters instead of using explicit SqlParameter objects? For example:

public IEnumerable<UserFilterGroup> GetUserFilterGroups(UserFilterGroupSearchCriteria criteria)
{
    var query = CurrentDbContext.UserFilterGroups.FromSql(
        @"SELECT * FROM (" + GenerateCriteriaQuery(criteria) + @")C 
            ORDER BY NAME OFFSET {0} ROWS FETCH NEXT {1} ROWS ONLY",
            criteria.PageNumber * criteria.Rows, criteria.Rows);

    return query.Include(x => x.UserFilterValues).Include(x => x.UserFilterDefaultSettings).ToList();
}

[punssoma]

@ajcvickers The initial implementation was as you mentioned above.
But as part of secure coding practice, we are suppose to use parameterized queries while executing sql queries directly.

Meanwhile as a workaround i have change the code as below:

var userFilterGroups = query.ToList();
foreach (var item in userFilterGroups)
{
    item.UserFilterValues = CurrentDbContext.UserFilterValues.Where(x => x.UserFilterGroupId == item.Id).ToList();
    item.UserFilterDefaultSettings = CurrentDbContext.UserFilterDefaultSettings.Where(x => x.UserFilterGroupId == item.Id).ToList();
}
return userFilterGroups;

But then it would be better if it can be done using Include methods in one go.

[smitpatel]

@punssoma - The query posted by @ajcvickers is still parametrized. When you use FromSql method with placeholders and pass in values for those placeholders as args to FromSql then we generate parameters for it in SQL. The difference is instead of you passing SqlParameters which causes above error, we will generate parameters for you and it won't hit the issue. (certainly its just a work-around for now)

[punssoma]

@smitpatel ok. Thanks for the info.

[Tasteful]

In our application we start getting this bug in some cases where we using .Includes() and SqlParameters that streams a list of items into a TVP-field. I found a workaround to clone the SqlParameter if they already is attached to another DbCommand, see changes in https://github.com/aspnet/EntityFrameworkCore/compare/master...Tasteful:bugfix/11370?expand=1#diff-92cbd310a2d450c3125d02f8544480a6

To make a formal PR with the fix I need some direction to get it correct:

• Using the relationa classes that I have changed as linked branch or implement overrides in SqlServer-project and use a separate implementation only for SqlServers?
• If implementing overrides in the SqlServer-project I need to do some overrides on the RelationalParameterBuilder, but that implementation is using a private field List<IRelationalParameter> _parameters that all the Add*-methods are using, can I change this to a protected property instead?

@ajcvickers @smitpatel

[smitpatel]

if #12098 is implemented then this issue may not exist.

[ahmadakra]

We are also trying to pass a table-valued parameter to the SQL query, therefore we are unable to use the workaround suggested by @ajcvickers of relying on auto-generated SQL parameters:

DataTable idsTable = // Create a DataTable
var tvp = new SqlParameter("@Ids", idsTable)
{
TypeName = $"dbo.IdList",
SqlDbType = SqlDbType.Structured
};

var rolesQuery = _db.Roles.FromSql("SELECT * FROM dbo.[Roles] WHERE Id IN (SELECT Id FROM @Ids)", tvp);
rolesQuery = rolesQuery.Include("Permissions"); // Include child collection
var rolesList = await rolesQuery.ToListAsync(); // FAILS with the above error

[ajcvickers]

@ahmadakra What happens if you use the {0} syntax, as shown above? You can still pass in the actual SqlParameter as the parameter value.

[smitpatel]

Closing as fixed since #12098 fixed the underlying issue in this case.