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

Query: Add regression test for #10642 #15968

Merged
merged 2 commits into from
Jun 6, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 1 addition & 11 deletions src/EFCore.Relational/Diagnostics/RelationalEventId.cs
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ private enum Id
QueryClientEvaluationWarning = CoreEventId.RelationalBaseId + 500,
QueryPossibleUnintendedUseOfEqualsWarning,
QueryPossibleExceptionWithAggregateOperatorWarning,
ValueConversionSqlLiteralWarning,
ValueConversionSqlLiteralWarning, // This warning has been removed.

// Model validation events
ModelValidationKeyDefaultValueWarning = CoreEventId.RelationalBaseId + 600,
Expand Down Expand Up @@ -463,16 +463,6 @@ private enum Id
/// </summary>
public static readonly EventId QueryPossibleExceptionWithAggregateOperatorWarning = MakeQueryId(Id.QueryPossibleExceptionWithAggregateOperatorWarning);

/// <summary>
/// <para>
/// A SQL literal is being generated for a value that is using a value conversion.
/// </para>
/// <para>
/// This event is in the <see cref="DbLoggerCategory.Query" /> category.
/// </para>
/// </summary>
public static readonly EventId ValueConversionSqlLiteralWarning = MakeQueryId(Id.ValueConversionSqlLiteralWarning);

private static readonly string _validationPrefix = DbLoggerCategory.Model.Validation.Name + ".";
private static EventId MakeValidationId(Id id) => new EventId((int)id, _validationPrefix + id);

Expand Down
42 changes: 0 additions & 42 deletions src/EFCore.Relational/Diagnostics/RelationalLoggerExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1363,48 +1363,6 @@ public static void QueryPossibleExceptionWithAggregateOperatorWarning(
}
}

/// <summary>
/// Logs for the <see cref="RelationalEventId.ValueConversionSqlLiteralWarning" /> event.
/// </summary>
/// <param name="diagnostics"> The diagnostics logger to use. </param>
/// <param name="mappingClrType"> The CLR type. </param>
/// <param name="valueConverter"> The value converter. </param>
public static void ValueConversionSqlLiteralWarning(
[NotNull] this IDiagnosticsLogger<DbLoggerCategory.Query> diagnostics,
[NotNull] Type mappingClrType,
[NotNull] ValueConverter valueConverter)
{
var definition = RelationalResources.LogValueConversionSqlLiteralWarning(diagnostics);

var warningBehavior = definition.GetLogBehavior(diagnostics);
if (warningBehavior != WarningBehavior.Ignore)
{
definition.Log(
diagnostics,
warningBehavior,
mappingClrType.ShortDisplayName(),
valueConverter.GetType().ShortDisplayName());
}

if (diagnostics.DiagnosticSource.IsEnabled(definition.EventId.Name))
{
diagnostics.DiagnosticSource.Write(
definition.EventId.Name,
new ValueConverterEventData(
definition,
ValueConversionSqlLiteral,
mappingClrType,
valueConverter));
}
}

private static string ValueConversionSqlLiteral(EventDefinitionBase definition, EventData payload)
{
var d = (EventDefinition<object, object>)definition;
var p = (ValueConverterEventData)payload;
return d.GenerateMessage(p.MappingClrType.ShortDisplayName(), p.ValueConverter.GetType().ShortDisplayName());
}

/// <summary>
/// Logs for the <see cref="RelationalEventId.ModelValidationKeyDefaultValueWarning" /> event.
/// </summary>
Expand Down
24 changes: 0 additions & 24 deletions src/EFCore.Relational/Properties/RelationalStrings.Designer.cs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 0 additions & 4 deletions src/EFCore.Relational/Properties/RelationalStrings.resx
Original file line number Diff line number Diff line change
Expand Up @@ -445,10 +445,6 @@
<value>A MigrationAttribute isn't specified on the '{class}' class.</value>
<comment>Warning RelationalEventId.MigrationAttributeMissingWarning string</comment>
</data>
<data name="LogValueConversionSqlLiteralWarning" xml:space="preserve">
<value>A SQL parameter or literal was generated for the type '{type}' using the ValueConverter '{valueConverter}'. Review the generated SQL for correctness and consider evaluating the target expression in-memory instead.</value>
<comment>Warning RelationalEventId.ValueConversionSqlLiteralWarning object object</comment>
</data>
<data name="DerivedTypeTable" xml:space="preserve">
<value>The entity type '{entityType}' cannot be mapped to a table because it is derived from '{baseType}'. Only base entity types can be mapped to a table.</value>
</data>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,6 @@ public abstract class GearsOfWarQueryRelationalFixture : GearsOfWarQueryFixtureB

public TestSqlLoggerFactory TestSqlLoggerFactory => (TestSqlLoggerFactory)ListLoggerFactory;

public override DbContextOptionsBuilder AddOptions(DbContextOptionsBuilder builder)
=> base.AddOptions(builder).ConfigureWarnings(
c => c
.Log(RelationalEventId.ValueConversionSqlLiteralWarning));

protected override bool ShouldLogCategory(string logCategory)
=> logCategory == DbLoggerCategory.Query.Name;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,7 @@ public override DbContextOptionsBuilder AddOptions(DbContextOptionsBuilder build
=> base.AddOptions(builder).ConfigureWarnings(
c => c
.Log(RelationalEventId.QueryPossibleUnintendedUseOfEqualsWarning)
.Log(RelationalEventId.QueryPossibleExceptionWithAggregateOperatorWarning)
.Log(RelationalEventId.ValueConversionSqlLiteralWarning))
.Log(RelationalEventId.QueryPossibleExceptionWithAggregateOperatorWarning))
.EnableDetailedErrors();

protected override bool ShouldLogCategory(string logCategory)
Expand Down
12 changes: 12 additions & 0 deletions test/EFCore.Specification.Tests/Query/SimpleQueryTestBase.Where.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2004,5 +2004,17 @@ public virtual Task TypeBinary_short_circuit(bool isAsync)
os => os.Where(o => (customer is Order)));
#pragma warning restore CS0184 // 'is' expression's given expression is never of the provided type
}

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Decimal_cast_to_double_works(bool isAsync)
{
var customer = new Customer();

return AssertQuery<Product>(
isAsync,
ps => ps.Where(p => (double?)p.UnitPrice > 100),
entryCount: 2);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -23,22 +23,6 @@ public ConvertToProviderTypesSqlServerTest(ConvertToProviderTypesSqlServerFixtur
{
}

[ConditionalFact(Skip = "Issue#15312")]
public virtual void Warning_when_suspicious_conversion_in_sql()
{
using (var context = CreateContext())
{
Assert.Contains(
RelationalResources.LogValueConversionSqlLiteralWarning(new TestLogger<SqlServerLoggingDefinitions>())
.GenerateMessage(
typeof(decimal).ShortDisplayName(),
new NumberToBytesConverter<decimal>().GetType().ShortDisplayName()),
Assert.Throws<InvalidOperationException>(
() =>
context.Set<BuiltInDataTypes>().Where(b => b.TestDecimal > 123.0m).ToList()).Message);
}
}

[ConditionalFact]
public virtual void Columns_have_expected_data_types()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1619,10 +1619,6 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con
b => b.Property(e => e.Decimal).HasColumnType("decimal(5, 2)"));
}

public override DbContextOptionsBuilder AddOptions(DbContextOptionsBuilder builder)
=> base.AddOptions(builder).ConfigureWarnings(
c => c.Log(RelationalEventId.ValueConversionSqlLiteralWarning));

public override bool SupportsBinaryKeys => true;

public override DateTime DefaultDateTime => new DateTime();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,6 @@ public class ConvertToProviderTypesSqliteFixture : ConvertToProviderTypesFixture
protected override ITestStoreFactory TestStoreFactory => SqliteTestStoreFactory.Instance;
public TestSqlLoggerFactory TestSqlLoggerFactory => (TestSqlLoggerFactory)ListLoggerFactory;

public override DbContextOptionsBuilder AddOptions(DbContextOptionsBuilder builder)
=> base.AddOptions(builder).ConfigureWarnings(
c => c.Log(RelationalEventId.ValueConversionSqlLiteralWarning));

public override bool SupportsBinaryKeys => true;

public override DateTime DefaultDateTime => new DateTime();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,12 +37,6 @@ public class CustomConvertersSqliteFixture : CustomConvertersFixtureBase
public override bool SupportsBinaryKeys => true;

public override DateTime DefaultDateTime => new DateTime();

public override DbContextOptionsBuilder AddOptions(DbContextOptionsBuilder builder)
=> base
.AddOptions(builder)
.ConfigureWarnings(
c => c.Log(RelationalEventId.ValueConversionSqlLiteralWarning));
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -964,6 +964,13 @@ SELECT rtrim(rtrim(strftime('%Y-%m-%d %H:%M:%f', ""o"".""OrderDate"", CAST(((CAS
WHERE ""o"".""OrderDate"" IS NOT NULL");
}

public override async Task Decimal_cast_to_double_works(bool isAsync)
{
await base.Decimal_cast_to_double_works(isAsync);

AssertSql(@" ");
}

private void AssertSql(params string[] expected)
=> Fixture.TestSqlLoggerFactory.AssertBaseline(expected);
}
Expand Down