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

Use column/property facets for parameter types in Query Pipeline #4608

Closed
carlcamilleri opened this issue Feb 19, 2016 · 5 comments
Closed
Labels
closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. type-enhancement
Milestone

Comments

@carlcamilleri
Copy link

Unless I'm missing something, parameterised queries (against SQL Server in my case) are treating all strings as NVARCHAR

I have the below table definition:

modelBuilder.Entity<Products>(entity =>
            {
                entity.ToTable("Products", "ofr");

                entity.HasIndex(e => e.ProductId).HasName("IX_Products_ProductId").IsUnique();

                entity.HasIndex(e => new { e.Id, e.ProductId, e.PublicationId }).HasName("IX_ofr_Products_PublicationId");

                entity.Property(e => e.ProductId)
                    .IsRequired()
                    .HasMaxLength(50)                    
                    .HasColumnType("VARCHAR");
            });

Then a query:

 var q = from p in products

                        join sp in dbContext.SiteProducts
                        on new { Id = p.Id, ProductId = p.ProductId }
                        equals new { Id = sp.ProdId, ProductId = req.ProductId}

                        where   sp.SiteId == req.Engine.Context.SiteId                                            

                        select p;
var res = q.ToList();

The query generated is:

exec sp_executesql N'SELECT TOP(1) [p].[Id], [p].[ProductId], [p].[PublicationId]
FROM [ofr].[Products] AS [p]
INNER JOIN [ofr].[SiteProducts] AS [sp] ON ([p].[Id] = [sp].[ProdId]) AND ([p].[ProductId] = @__req_ProductId_0)
WHERE [sp].[SiteId] = @__req_Engine_Context_SiteId_1',N'@__req_ProductId_0 nvarchar(4000),@__req_Engine_Context_SiteId_1 int',@__req_ProductId_0=N'350003995',@__req_Engine_Context_SiteId_1=1

The "ProductId" parameter is therefore treated as an NVARCHAR, obviously missing the index.

I have tested with EF6 against the same database, and the query generated correctly uses a VARCHAR parameter:

exec sp_executesql N'SELECT 
    [Extent1].[Id] AS [Id], 
    [Extent1].[ProductId] AS [ProductId], 
    [Extent1].[PublicationId] AS [PublicationId]
    FROM  [ofr].[Products] AS [Extent1]
    INNER JOIN [ofr].[SiteProducts] AS [Extent2] ON ([Extent1].[Id] = [Extent2].[ProdId]) AND ([Extent1].[ProductId] = @p__linq__0)
    WHERE 1 = [Extent2].[SiteId]',N'@p__linq__0 varchar(8000)',@p__linq__0='324324324'

Is this something which is in-line to be fixed, or is there perhaps a workaround?

Thanks

@ErikEJ
Copy link
Contributor

ErikEJ commented Feb 19, 2016

Try:

.HasColumnType("VARCHAR(50)");

@carlcamilleri
Copy link
Author

Hi,

Thanks, tested but unfortunately same query is generated:

        modelBuilder.Entity<Products>(entity =>
            {
                entity.ToTable("Products", "ofr");

                entity.HasIndex(e => e.ProductId).HasName("IX_Products_ProductId").IsUnique();

                entity.HasIndex(e => new { e.Id, e.ProductId, e.PublicationId }).HasName("IX_ofr_Products_PublicationId");

                entity.Property(e => e.ProductId)
                    .IsRequired()
                    .HasMaxLength(50)                    
                    .HasColumnType("VARCHAR(50)");
            });
exec sp_executesql N'SELECT [p].[Id], [p].[ProductId], [p].[PublicationId]
FROM [ofr].[Products] AS [p]
INNER JOIN [ofr].[SiteProducts] AS [sp] ON ([p].[Id] = [sp].[ProdId]) AND ([p].[ProductId] = @__req_ProductId_0)
WHERE [sp].[SiteId] = @__req_Engine_Context_SiteId_1',N'@__req_ProductId_0 nvarchar(4000),@__req_Engine_Context_SiteId_1 int',@__req_ProductId_0=N'350003995',@__req_Engine_Context_SiteId_1=1

For reference I'm running this from a DNX project, my project.json file:

{
  "version": "1.0.0-*",
  "description": "...",
  "authors": [ "..." ],
  "tags": [ "" ],
  "projectUrl": "",
  "licenseUrl": "",
  "frameworks": {    
    "dnx451": {
      "dependencies": {
        "Microsoft.CSharp": "4.0.1-beta-23516",
        "System.Collections": "4.0.11-beta-23516",
        "System.Linq": "4.0.1-beta-23516",
        "System.Runtime": "4.0.21-beta-23516",
        "System.Threading": "4.0.11-beta-23516"
      }
    }
  },
  "dependencies": {
    "EntityFramework.Commands": "7.0.0-rc1-final",
    "EntityFramework.MicrosoftSqlServer": "7.0.0-rc1-final",
    "EntityFramework.MicrosoftSqlServer.Design": "7.0.0-rc1-final",    
    "EntityFramework.Core": "7.0.0-rc1-final",    
    "Microsoft.Extensions.Logging": "1.0.0-rc1-final",
    "Microsoft.Extensions.Logging.Abstractions": "1.0.0-rc1-final",
    "Microsoft.Extensions.DependencyInjection.Abstractions": "1.0.0-rc1-final",
    "Microsoft.Extensions.DependencyInjection": "1.0.0-rc1-final"    
  },
  "commands": {
    "ef": "EntityFramework.Commands"
  }
}

@rowanmiller
Copy link
Contributor

Probably considered a duplicate of #4134 and missing the index would be a good reason to do that work sooner rather than later.

@rowanmiller rowanmiller changed the title SQL Server NVARCHAR parameterised query - performance Use column/property facets for parameter types in Query Pipeline Feb 19, 2016
@rowanmiller rowanmiller added this to the 1.0.0 milestone Feb 19, 2016
@carlcamilleri
Copy link
Author

Thanks for the feedback.

Just to add one further detail in case it helps: this is only impacting queries which compare VARCHAR columns to string variables.

The following query compares a VARCHAR column to an explicit string value "12345"

  var q = from p in products

                        join sp in dbContext.SiteProducts
                        on new { Id = p.Id, ProductId = p.ProductId }
                        equals new { Id = sp.ProdId, ProductId = "12345"}

                        where   sp.SiteId == req.Engine.Context.SiteId                                            

                        select p;

The comparison at SQL level is fine, "12345" is not presented as N'12345" and therefore utilises indexes on the VARCHAR column correctly:

exec sp_executesql N'SELECT [p].[Id], [p].[ProductId], [p].[PublicationId]
FROM [ofr].[Products] AS [p]
INNER JOIN [ofr].[SiteProducts] AS [sp] ON ([p].[Id] = [sp].[ProdId]) AND ([p].[ProductId] = ''12345'')
WHERE [sp].[SiteId] = @__req_Engine_Context_SiteId_0',N'@__req_Engine_Context_SiteId_0 int',@__req_Engine_Context_SiteId_0=1

@mikary
Copy link
Contributor

mikary commented Mar 4, 2016

Once facets for parameter types have been added to the query pipeline, we should ensure the DefaultQuerySqlGenerator uses the IRelationalCommandBuilder / IRelationalTypeMapper overload that maps from the IProperty type instead of the clr type.

@rowanmiller rowanmiller assigned ajcvickers and unassigned smitpatel May 9, 2016
ajcvickers added a commit that referenced this issue May 25, 2016
Issues:
#4608 Use column/property facets for parameter types in Query Pipeline
#4134 Use column/property facets for parameter types in Update Pipeline

If a property length is specified, then this is used to infer the length to use for parameters relating to that property, unless the length of the data is too long, in which case unbounded length is used. Because query has code that can infer the length to use for a parameter this still means that fragmentation will not happen without always using the 4000/8000 value.
ajcvickers added a commit that referenced this issue May 26, 2016
Issues:
#4608 Use column/property facets for parameter types in Query Pipeline
#4134 Use column/property facets for parameter types in Update Pipeline

If a property length is specified, then this is used to infer the length to use for parameters relating to that property, unless the length of the data is too long, in which case unbounded length is used. Because query has code that can infer the length to use for a parameter this still means that fragmentation will not happen without always using the 4000/8000 value.
ajcvickers added a commit that referenced this issue May 26, 2016
Issues:
#4608 Use column/property facets for parameter types in Query Pipeline
#4134 Use column/property facets for parameter types in Update Pipeline

If a property length is specified, then this is used to infer the length to use for parameters relating to that property, unless the length of the data is too long, in which case unbounded length is used. Because query has code that can infer the length to use for a parameter this still means that fragmentation will not happen without always using the 4000/8000 value.
@ajcvickers ajcvickers added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label Oct 15, 2022
@ajcvickers ajcvickers removed their assignment Sep 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. type-enhancement
Projects
None yet
Development

No branches or pull requests

6 participants