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

Translation of ISNUMERIC under SF.Functions #23114

Closed
itx-digital opened this issue Oct 28, 2020 · 9 comments · Fixed by #23232
Closed

Translation of ISNUMERIC under SF.Functions #23114

itx-digital opened this issue Oct 28, 2020 · 9 comments · Fixed by #23232
Assignees
Labels
area-query area-sqlserver closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. community-contribution customer-reported type-enhancement
Milestone

Comments

@itx-digital
Copy link

Hello,
Could you please add a translation to SQL Server's ISNUMERIC under EF.Functions ?
Thank you.
Best regards.

Originally posted by @evannah in #23078 (comment)

@roji
Copy link
Member

roji commented Oct 28, 2020

Until this is done in the provider, you can map this as a DbFunction yourself - see the code sample below.

Using DbFunction
await using var ctx = new BlogContext();
await ctx.Database.EnsureDeletedAsync();
await ctx.Database.EnsureCreatedAsync();

_ = await ctx.Blogs.Where(b => ctx.IsNumeric(b.Name)).ToListAsync();

public class BlogContext : DbContext
{
    public DbSet<Blog> Blogs { get; set; }

    static ILoggerFactory ContextLoggerFactory
        => LoggerFactory.Create(b => b.AddConsole().AddFilter("", LogLevel.Information));

    protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
        => optionsBuilder
            .UseSqlServer(@"Server=localhost;Database=test;User=SA;Password=Abcd5678;Connect Timeout=60;ConnectRetryCount=0")
            .EnableSensitiveDataLogging()
            .UseLoggerFactory(ContextLoggerFactory);

    protected override void OnModelCreating(ModelBuilder modelBuilder)
    {
        modelBuilder.HasDbFunction(IsNumericMethodInfo)
            .HasName("ISNUMERIC")
            .IsBuiltIn();
    }

    private static readonly MethodInfo IsNumericMethodInfo = typeof(BlogContext)
        .GetRuntimeMethod(nameof(IsNumeric), new[] { typeof(string) });

    public bool IsNumeric(string s) => throw new NotSupportedException();
}

public class Blog
{
    public int Id { get; set; }
    public string Name { get; set; }
}

@roji
Copy link
Member

roji commented Oct 28, 2020

@smitpatel @maumar note that with the above I get:

SELECT [b].[Id], [b].[Name]
FROM [Blogs] AS [b]
WHERE ISNUMERIC([b].[Name]) = CAST(1 AS bit)

The CAST(1 AS bit) looks like it can go away, some search condition improvement?

@smitpatel
Copy link
Contributor

smitpatel commented Oct 28, 2020

IsNumeric returns int in T-Sql.
No changes to search conditions required.

@roji
Copy link
Member

roji commented Oct 28, 2020

Ah weird...

@ajcvickers ajcvickers added this to the Backlog milestone Oct 30, 2020
@ajcvickers ajcvickers added the good first issue This issue should be relatively straightforward to fix. label Oct 30, 2020
@ajcvickers
Copy link
Contributor

Note from triage: If there is an appropriate BCL method to translate, then we should do that instead. However, I don't think we have identified such a method at this time.

@smitpatel smitpatel self-assigned this Dec 22, 2020
smitpatel pushed a commit that referenced this issue Dec 22, 2020
@smitpatel smitpatel modified the milestones: Backlog, 6.0.0 Dec 22, 2020
@smitpatel smitpatel reopened this Dec 22, 2020
@smitpatel smitpatel added needs-design and removed good first issue This issue should be relatively straightforward to fix. labels Dec 22, 2020
@smitpatel
Copy link
Contributor

Design question for @dotnet/efteam
What should be the return type of IsNumeric?
It returns int in T-Sql. In PR we added bool. Looking at ef6 code, it returns int it seems. (Note it should not return int? as it can never be null value.
https://github.com/dotnet/ef6/blob/04a6ae5556f38fb3ba622dcb142a6c6b37101dd9/src/EntityFramework.SqlServer/SqlFunctions.cs#L1607-L1615

@ajcvickers
Copy link
Contributor

@smitpatel Did you get an answer on this?

@smitpatel
Copy link
Contributor

No. I will bring it to next design meeting.

@smitpatel
Copy link
Contributor

Design meeting decision: We decided to keep this as bool return value. It is minor breaking change from ef6 but bool here gives better query writing to customer given the function definition.

@smitpatel smitpatel added closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. and removed needs-design labels Mar 3, 2021
@ajcvickers ajcvickers modified the milestones: 6.0.0, 6.0.0-preview1 Mar 5, 2021
@ajcvickers ajcvickers modified the milestones: 6.0.0-preview1, 6.0.0 Nov 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-query area-sqlserver closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. community-contribution customer-reported type-enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants