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.

Sign up for GitHub

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

Case sensitive parameters 18861 #26287

Closed
wants to merge 19 commits into from
Closed

Case sensitive parameters 18861 #26287

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
e19169f
Update dependencies from https://github.com/dotnet/runtime build 2021…
dotnet-maestro[bot] Oct 7, 2021
41db08c
Update dependencies from https://github.com/dotnet/runtime build 2021…
dotnet-maestro[bot] Oct 7, 2021
26f82f0
Update dependencies from https://github.com/dotnet/runtime build 2021…
dotnet-maestro[bot] Oct 7, 2021
d04a65c
Update dependencies from https://github.com/dotnet/runtime build 2021…
dotnet-maestro[bot] Oct 7, 2021
2b60270
Update dependencies from https://github.com/dotnet/runtime build 2021…
dotnet-maestro[bot] Oct 7, 2021
458569e
Update dependencies from https://github.com/dotnet/runtime build 2021…
dotnet-maestro[bot] Oct 8, 2021
45e7da2
Allow case sensitive parameter names
KevRitchie Oct 8, 2021
7d1c9e1
Merge branch 'dotnet:release/6.0' into case-sensitive-parameters-18861
KevRitchie Oct 8, 2021
2ccd12e
Add test for case-insensitive parameter bind
KevRitchie Oct 8, 2021
b8aaf09
Merge branch 'case-sensitive-parameters-18861' of https://github.com/…
KevRitchie Oct 8, 2021
ee5b4d1
Allow case sensitive parameter names
KevRitchie Oct 8, 2021
9dd5e20
Update dependencies from https://github.com/dotnet/runtime build 2021…
dotnet-maestro[bot] Oct 7, 2021
ddb5da0
Update dependencies from https://github.com/dotnet/runtime build 2021…
dotnet-maestro[bot] Oct 7, 2021
69b38cf
Update dependencies from https://github.com/dotnet/runtime build 2021…
dotnet-maestro[bot] Oct 7, 2021
c0be32f
Update dependencies from https://github.com/dotnet/runtime build 2021…
dotnet-maestro[bot] Oct 7, 2021
a5b54a3
Update dependencies from https://github.com/dotnet/runtime build 2021…
dotnet-maestro[bot] Oct 7, 2021
5c60edc
Update dependencies from https://github.com/dotnet/runtime build 2021…
dotnet-maestro[bot] Oct 8, 2021
f46fbb7
Add test for case-insensitive parameter bind
KevRitchie Oct 8, 2021
9479558
Merge branch 'case-sensitive-parameters-18861' of https://github.com/…
KevRitchie Oct 8, 2021
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
44 changes: 22 additions & 22 deletions eng/Version.Details.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,49 +1,49 @@
<?xml version="1.0" encoding="utf-8"?>
<Dependencies>
<ProductDependencies>
<Dependency Name="Microsoft.Extensions.Caching.Memory" Version="7.0.0-alpha.1.21501.7">
<Dependency Name="Microsoft.Extensions.Caching.Memory" Version="6.0.0-rtm.21507.16">
<Uri>https://github.com/dotnet/runtime</Uri>
<Sha>03e90a540cb2dfe5cab4086ea54ad5dd1f655749</Sha>
<Sha>733a3089ec6945422caf06035c18ff700c9d51be</Sha>
</Dependency>
<Dependency Name="Microsoft.Extensions.Configuration.EnvironmentVariables" Version="7.0.0-alpha.1.21501.7">
<Dependency Name="Microsoft.Extensions.Configuration.EnvironmentVariables" Version="6.0.0-rtm.21507.16">
<Uri>https://github.com/dotnet/runtime</Uri>
<Sha>03e90a540cb2dfe5cab4086ea54ad5dd1f655749</Sha>
<Sha>733a3089ec6945422caf06035c18ff700c9d51be</Sha>
</Dependency>
<Dependency Name="Microsoft.Extensions.Configuration.Abstractions" Version="7.0.0-alpha.1.21501.7">
<Dependency Name="Microsoft.Extensions.Configuration.Abstractions" Version="6.0.0-rtm.21507.16">
<Uri>https://github.com/dotnet/runtime</Uri>
<Sha>03e90a540cb2dfe5cab4086ea54ad5dd1f655749</Sha>
<Sha>733a3089ec6945422caf06035c18ff700c9d51be</Sha>
</Dependency>
<Dependency Name="Microsoft.Extensions.Configuration.Json" Version="7.0.0-alpha.1.21501.7">
<Dependency Name="Microsoft.Extensions.Configuration.Json" Version="6.0.0-rtm.21507.16">
<Uri>https://github.com/dotnet/runtime</Uri>
<Sha>03e90a540cb2dfe5cab4086ea54ad5dd1f655749</Sha>
<Sha>733a3089ec6945422caf06035c18ff700c9d51be</Sha>
</Dependency>
<Dependency Name="Microsoft.Extensions.Configuration" Version="7.0.0-alpha.1.21501.7">
<Dependency Name="Microsoft.Extensions.Configuration" Version="6.0.0-rtm.21507.16">
<Uri>https://github.com/dotnet/runtime</Uri>
<Sha>03e90a540cb2dfe5cab4086ea54ad5dd1f655749</Sha>
<Sha>733a3089ec6945422caf06035c18ff700c9d51be</Sha>
</Dependency>
<Dependency Name="Microsoft.Extensions.DependencyInjection" Version="7.0.0-alpha.1.21501.7">
<Dependency Name="Microsoft.Extensions.DependencyInjection" Version="6.0.0-rtm.21507.16">
<Uri>https://github.com/dotnet/runtime</Uri>
<Sha>03e90a540cb2dfe5cab4086ea54ad5dd1f655749</Sha>
<Sha>733a3089ec6945422caf06035c18ff700c9d51be</Sha>
</Dependency>
<Dependency Name="Microsoft.Extensions.DependencyModel" Version="7.0.0-alpha.1.21501.7">
<Dependency Name="Microsoft.Extensions.DependencyModel" Version="6.0.0-rtm.21507.16">
<Uri>https://github.com/dotnet/runtime</Uri>
<Sha>03e90a540cb2dfe5cab4086ea54ad5dd1f655749</Sha>
<Sha>733a3089ec6945422caf06035c18ff700c9d51be</Sha>
</Dependency>
<Dependency Name="Microsoft.Extensions.HostFactoryResolver.Sources" Version="7.0.0-alpha.1.21501.7">
<Dependency Name="Microsoft.Extensions.HostFactoryResolver.Sources" Version="6.0.0-rtm.21507.16">
<Uri>https://github.com/dotnet/runtime</Uri>
<Sha>03e90a540cb2dfe5cab4086ea54ad5dd1f655749</Sha>
<Sha>733a3089ec6945422caf06035c18ff700c9d51be</Sha>
</Dependency>
<Dependency Name="Microsoft.Extensions.Logging" Version="7.0.0-alpha.1.21501.7">
<Dependency Name="Microsoft.Extensions.Logging" Version="6.0.0-rtm.21507.16">
<Uri>https://github.com/dotnet/runtime</Uri>
<Sha>03e90a540cb2dfe5cab4086ea54ad5dd1f655749</Sha>
<Sha>733a3089ec6945422caf06035c18ff700c9d51be</Sha>
</Dependency>
<Dependency Name="System.Collections.Immutable" Version="7.0.0-alpha.1.21501.7">
<Dependency Name="System.Collections.Immutable" Version="6.0.0-rtm.21507.16">
<Uri>https://github.com/dotnet/runtime</Uri>
<Sha>03e90a540cb2dfe5cab4086ea54ad5dd1f655749</Sha>
<Sha>733a3089ec6945422caf06035c18ff700c9d51be</Sha>
</Dependency>
<Dependency Name="System.Diagnostics.DiagnosticSource" Version="7.0.0-alpha.1.21501.7">
<Dependency Name="System.Diagnostics.DiagnosticSource" Version="6.0.0-rtm.21507.16">
<Uri>https://github.com/dotnet/runtime</Uri>
<Sha>03e90a540cb2dfe5cab4086ea54ad5dd1f655749</Sha>
<Sha>733a3089ec6945422caf06035c18ff700c9d51be</Sha>
</Dependency>
</ProductDependencies>
<ToolsetDependencies>
Expand Down
22 changes: 11 additions & 11 deletions eng/Versions.props
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,17 +15,17 @@
<UsingToolXliff>False</UsingToolXliff>
</PropertyGroup>
<PropertyGroup Label="Dependencies from dotnet/runtime">
<SystemCollectionsImmutableVersion>7.0.0-alpha.1.21501.7</SystemCollectionsImmutableVersion>
<SystemDiagnosticsDiagnosticSourceVersion>7.0.0-alpha.1.21501.7</SystemDiagnosticsDiagnosticSourceVersion>
<MicrosoftExtensionsCachingMemoryVersion>7.0.0-alpha.1.21501.7</MicrosoftExtensionsCachingMemoryVersion>
<MicrosoftExtensionsConfigurationVersion>7.0.0-alpha.1.21501.7</MicrosoftExtensionsConfigurationVersion>
<MicrosoftExtensionsConfigurationAbstractionsVersion>7.0.0-alpha.1.21501.7</MicrosoftExtensionsConfigurationAbstractionsVersion>
<MicrosoftExtensionsConfigurationEnvironmentVariablesVersion>7.0.0-alpha.1.21501.7</MicrosoftExtensionsConfigurationEnvironmentVariablesVersion>
<MicrosoftExtensionsConfigurationJsonVersion>7.0.0-alpha.1.21501.7</MicrosoftExtensionsConfigurationJsonVersion>
<MicrosoftExtensionsDependencyInjectionVersion>7.0.0-alpha.1.21501.7</MicrosoftExtensionsDependencyInjectionVersion>
<MicrosoftExtensionsDependencyModelVersion>7.0.0-alpha.1.21501.7</MicrosoftExtensionsDependencyModelVersion>
<MicrosoftExtensionsHostFactoryResolverSourcesVersion>7.0.0-alpha.1.21501.7</MicrosoftExtensionsHostFactoryResolverSourcesVersion>
<MicrosoftExtensionsLoggingVersion>7.0.0-alpha.1.21501.7</MicrosoftExtensionsLoggingVersion>
<SystemCollectionsImmutableVersion>6.0.0-rtm.21507.16</SystemCollectionsImmutableVersion>
<SystemDiagnosticsDiagnosticSourceVersion>6.0.0-rtm.21507.16</SystemDiagnosticsDiagnosticSourceVersion>
<MicrosoftExtensionsCachingMemoryVersion>6.0.0-rtm.21507.16</MicrosoftExtensionsCachingMemoryVersion>
<MicrosoftExtensionsConfigurationVersion>6.0.0-rtm.21507.16</MicrosoftExtensionsConfigurationVersion>
<MicrosoftExtensionsConfigurationAbstractionsVersion>6.0.0-rtm.21507.16</MicrosoftExtensionsConfigurationAbstractionsVersion>
<MicrosoftExtensionsConfigurationEnvironmentVariablesVersion>6.0.0-rtm.21507.16</MicrosoftExtensionsConfigurationEnvironmentVariablesVersion>
<MicrosoftExtensionsConfigurationJsonVersion>6.0.0-rtm.21507.16</MicrosoftExtensionsConfigurationJsonVersion>
<MicrosoftExtensionsDependencyInjectionVersion>6.0.0-rtm.21507.16</MicrosoftExtensionsDependencyInjectionVersion>
<MicrosoftExtensionsDependencyModelVersion>6.0.0-rtm.21507.16</MicrosoftExtensionsDependencyModelVersion>
<MicrosoftExtensionsHostFactoryResolverSourcesVersion>6.0.0-rtm.21507.16</MicrosoftExtensionsHostFactoryResolverSourcesVersion>
<MicrosoftExtensionsLoggingVersion>6.0.0-rtm.21507.16</MicrosoftExtensionsLoggingVersion>
</PropertyGroup>
<PropertyGroup Label="Other dependencies">
<MicrosoftCodeAnalysisVersion>3.7.0</MicrosoftCodeAnalysisVersion>
Expand Down
3 changes: 3 additions & 0 deletions src/Microsoft.Data.Sqlite.Core/SqliteCommand.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -322,12 +322,15 @@ private IEnumerable<sqlite3_stmt> GetStatements(Stopwatch timer)
? PrepareAndEnumerateStatements(timer)
: _preparedStatements)
{

var boundParams = _parameters?.Bind(stmt) ?? 0;

var expectedParams = sqlite3_bind_parameter_count(stmt);

if (expectedParams != boundParams)
{
var unboundParams = new List<string>();

for (var i = 1; i <= expectedParams; i++)
{
var name = sqlite3_bind_parameter_name(stmt, i).utf8_to_string();
Expand Down
17 changes: 17 additions & 0 deletions src/Microsoft.Data.Sqlite.Core/SqliteParameter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,23 @@ internal bool Bind(sqlite3_stmt stmt)
}

var index = sqlite3_bind_parameter_index(stmt, ParameterName);

if(index == 0)
{
var expectedParams = sqlite3_bind_parameter_count(stmt);

for (var i = 1; i <= expectedParams; i++)
{
var name = sqlite3_bind_parameter_name(stmt, i).utf8_to_string();

if (String.Compare(ParameterName, name, StringComparison.OrdinalIgnoreCase) == 0)
{
index = i;
break;
}
}
}

if (index == 0
&& (index = FindPrefixedParameter(stmt)) == 0)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should also add a similar case-insensitive fallback for FindPrefixedParameter.

{
Expand Down
15 changes: 13 additions & 2 deletions src/Microsoft.Data.Sqlite.Core/SqliteParameterCollection.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -328,11 +328,22 @@ protected override void SetParameter(string parameterName, DbParameter value)
internal int Bind(sqlite3_stmt stmt)
{
var bound = 0;
var checkedParams = new List<SqliteParameter>();

foreach (var parameter in _parameters)
{
if (parameter.Bind(stmt))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code shouldn't require any changes since parameter.Bind will return true if any match was found.

We do however have to update GetStatements.

Copy link
Contributor Author

@KevRitchie KevRitchie Jun 9, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've reverted the code back.

Is this the GetStatements method in SqliteCommand?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, for the error message that says which parameters weren't bound. Today, it just does a case-sensitive check:

efcore/src/Microsoft.Data.Sqlite.Core/SqliteCommand.cs

Lines 332 to 335 in b19c1e7

var name = sqlite3_bind_parameter_name(stmt, i).utf8_to_string();
if (_parameters != null
&& !_parameters.Cast<SqliteParameter>().Any(p => p.ParameterName == name))

if(!checkedParams.Cast<SqliteParameter>().Any(p => p.ParameterName.Equals(parameter.ParameterName, StringComparison.OrdinalIgnoreCase)))
{
if (parameter.Bind(stmt))
{
bound++;

checkedParams.Add(parameter);
}
}
else
{
bound++;
throw new InvalidOperationException(Resources.AmbiguousParameterName(parameter.ParameterName));
}
}

Expand Down
16 changes: 16 additions & 0 deletions test/Microsoft.Data.Sqlite.Tests/SqliteParameterTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -433,6 +433,22 @@ public void Bind_does_not_require_prefix(string parameterName)
}
}

[Fact]
public void Bind_is_not_case_sensitive()
{
using (var connection = new SqliteConnection("Data Source=:memory:"))
{
var command = connection.CreateCommand();
command.CommandText = "SELECT @param";
command.Parameters.AddWithValue("@Param", "harvest");
Comment on lines +442 to +443
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs more coverage:

SQL Parameter name(s) Expected Result
SELECT $param Param Adds prefix and binds using a case-insensitive match
SELECT $param, $Param $param and $Param Both parameters are bound using case-sensitive matches
SELECT $param, $Param $Param and then $param Both parameters are bound using case-sensitive matches. Order doesn't matter
SELECT $param, $PARAM $Param Throws ambiguous parameter exception
SELECT $param, $PARAM Param Throws ambiguous parameter exception

Copy link
Contributor

@bricelam bricelam Jan 25, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also make sure the new GetStatements code gets covered.

connection.Open();

var result = command.ExecuteScalar();

Assert.Equal("harvest", result);
}
}

[Fact]
public void Bind_throws_for_ambiguous_parameters()
{
Expand Down