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

Add SqlCommand.EnableOptimizedParameterBinding Feature #1041

Merged
merged 18 commits into from
Jul 7, 2021
Merged

Add SqlCommand.EnableOptimizedParameterBinding Feature #1041

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
0057c01
merge
Wraith2 Jun 22, 2021
372c15d
add netcore ref definition
Wraith2 Apr 18, 2021
3e43f8c
add netfx SqlCommand.UsePositionalParameters property
Wraith2 Apr 18, 2021
250bec5
add netfx ref definition
Wraith2 Apr 18, 2021
b84da27
add first set of tests
Wraith2 Apr 18, 2021
a553c1b
add return test
Wraith2 Apr 18, 2021
1f88591
renamed to DisableOutputParameters
Wraith2 Apr 19, 2021
afb5050
merge
Wraith2 Jun 22, 2021
34e552e
tests changes
Wraith2 May 4, 2021
206d30d
add documentation
Wraith2 May 4, 2021
4f465fa
fixup merge conflicts
Wraith2 Jun 22, 2021
aa1b1f8
consistant naming
Wraith2 Jun 22, 2021
d6c66a1
sync netcore and netfx implementations
Wraith2 Jun 22, 2021
92961b1
add SqlCommnand documentation
Wraith2 Jun 23, 2021
a1302d0
fix normalizer resource again
Wraith2 Jun 23, 2021
3a01525
address feedback
Wraith2 Jun 23, 2021
2b5e8c9
use conditinoal fact in tests
Wraith2 Jun 24, 2021
421ee21
address feedback
Wraith2 Jul 5, 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
95 changes: 81 additions & 14 deletions doc/snippets/Microsoft.Data.SqlClient/SqlCommand.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -286,8 +286,13 @@ The following console application creates updates data within the **AdventureWor
<exception cref="T:System.InvalidOperationException">
The
<see cref="T:Microsoft.Data.SqlClient.SqlConnection" />
closed or dropped during a streaming operation. For more information about streaming, see [SqlClient Streaming Support](/sql/connect/ado-net/sqlclient-streaming-support).
</exception>
closed or dropped during a streaming operation. For more information about streaming, see [SqlClient Streaming Support](/sql/connect/ado-net/sqlclient-streaming-support).

- or -

<see cref="P:Microssoft.Data.SqlClient.SqlCommand.EnableOptimizedParameterBinding" />
is set to true and a parameter with direction Output or InputOutput has been added to the <see cref="P:Microsoft.Data.SqlClient.SqlCommand.Parameters" /> collection.
</exception>
<exception cref="T:System.IO.IOException">
An error occurred in a
<see cref="T:System.IO.Stream" />
Expand Down Expand Up @@ -399,8 +404,13 @@ To set up this example, create a new Windows application. Put a <xref:System.Win
<exception cref="T:System.InvalidOperationException">
The
<see cref="T:Microsoft.Data.SqlClient.SqlConnection" />
closed or dropped during a streaming operation. For more information about streaming, see [SqlClient Streaming Support](/sql/connect/ado-net/sqlclient-streaming-support).
</exception>
closed or dropped during a streaming operation. For more information about streaming, see [SqlClient Streaming Support](/sql/connect/ado-net/sqlclient-streaming-support).

- or -

<see cref="P:Microssoft.Data.SqlClient.SqlCommand.EnableOptimizedParameterBinding" />
is set to true and a parameter with direction Output or InputOutput has been added to the <see cref="P:Microsoft.Data.SqlClient.SqlCommand.Parameters" /> collection.
</exception>
</BeginExecuteNonQuery>
<BeginExecuteReader name="default">
<summary>
Expand Down Expand Up @@ -477,8 +487,13 @@ The following console application starts the process of retrieving a data reader
<exception cref="T:System.InvalidOperationException">
The
<see cref="T:Microsoft.Data.SqlClient.SqlConnection" />
closed or dropped during a streaming operation. For more information about streaming, see [SqlClient Streaming Support](/sql/connect/ado-net/sqlclient-streaming-support).
</exception>
closed or dropped during a streaming operation. For more information about streaming, see [SqlClient Streaming Support](/sql/connect/ado-net/sqlclient-streaming-support).

- or -

<see cref="P:Microssoft.Data.SqlClient.SqlCommand.EnableOptimizedParameterBinding" />
is set to true and a parameter with direction Output or InputOutput has been added to the <see cref="P:Microsoft.Data.SqlClient.SqlCommand.Parameters" /> collection.
</exception>
<exception cref="T:System.IO.IOException">
An error occurred in a
<see cref="T:System.IO.Stream" />
Expand Down Expand Up @@ -584,8 +599,13 @@ This example also passes the `CommandBehavior.CloseConnection` and `CommandBehav
<exception cref="T:System.InvalidOperationException">
The
<see cref="T:Microsoft.Data.SqlClient.SqlConnection" />
closed or dropped during a streaming operation. For more information about streaming, see [SqlClient Streaming Support](/sql/connect/ado-net/sqlclient-streaming-support).
</exception>
closed or dropped during a streaming operation. For more information about streaming, see [SqlClient Streaming Support](/sql/connect/ado-net/sqlclient-streaming-support).

- or -

<see cref="P:Microssoft.Data.SqlClient.SqlCommand.EnableOptimizedParameterBinding" />
is set to true and a parameter with direction Output or InputOutput has been added to the <see cref="P:Microsoft.Data.SqlClient.SqlCommand.Parameters" /> collection.
</exception>
<exception cref="T:System.IO.IOException">
An error occurred in a
<see cref="T:System.IO.Stream" />
Expand Down Expand Up @@ -702,8 +722,13 @@ To set up this example, create a new Windows application. Put a <xref:System.Win
<exception cref="T:System.InvalidOperationException">
The
<see cref="T:Microsoft.Data.SqlClient.SqlConnection" />
closed or dropped during a streaming operation. For more information about streaming, see [SqlClient Streaming Support](/sql/connect/ado-net/sqlclient-streaming-support).
</exception>
closed or dropped during a streaming operation. For more information about streaming, see [SqlClient Streaming Support](/sql/connect/ado-net/sqlclient-streaming-support).

- or -

<see cref="P:Microssoft.Data.SqlClient.SqlCommand.EnableOptimizedParameterBinding" />
is set to true and a parameter with direction Output or InputOutput has been added to the <see cref="P:Microsoft.Data.SqlClient.SqlCommand.Parameters" /> collection.
</exception>
<exception cref="T:System.IO.IOException">
An error occurred in a
<see cref="T:System.IO.Stream" />
Expand Down Expand Up @@ -831,8 +856,13 @@ This example passes the `CommandBehavior.CloseConnection` value in the `behavior
<exception cref="T:System.InvalidOperationException">
The
<see cref="T:Microsoft.Data.SqlClient.SqlConnection" />
closed or dropped during a streaming operation. For more information about streaming, see [SqlClient Streaming Support](/sql/connect/ado-net/sqlclient-streaming-support).
</exception>
closed or dropped during a streaming operation. For more information about streaming, see [SqlClient Streaming Support](/sql/connect/ado-net/sqlclient-streaming-support).

- or -

<see cref="P:Microssoft.Data.SqlClient.SqlCommand.EnableOptimizedParameterBinding" />
is set to true and a parameter with direction Output or InputOutput has been added to the <see cref="P:Microsoft.Data.SqlClient.SqlCommand.Parameters" /> collection.
</exception>
<exception cref="T:System.IO.IOException">
An error occurred in a
<see cref="T:System.IO.Stream" />
Expand Down Expand Up @@ -940,6 +970,11 @@ The following console application starts the process of retrieving XML data asyn
The
<see cref="T:Microsoft.Data.SqlClient.SqlConnection" />
closed or dropped during a streaming operation. For more information about streaming, see [SqlClient Streaming Support](/sql/connect/ado-net/sqlclient-streaming-support).

- or -

<see cref="P:Microssoft.Data.SqlClient.SqlCommand.EnableOptimizedParameterBinding" />
is set to true and a parameter with direction Output or InputOutput has been added to the <see cref="P:Microsoft.Data.SqlClient.SqlCommand.Parameters" /> collection.
</exception>
<exception cref="T:System.IO.IOException">
An error occurred in a
Expand Down Expand Up @@ -1067,8 +1102,13 @@ To set up this example, create a new Windows application. Put a <xref:System.Win
<exception cref="T:System.InvalidOperationException">
The
<see cref="T:Microsoft.Data.SqlClient.SqlConnection" />
closed or dropped during a streaming operation. For more information about streaming, see [SqlClient Streaming Support](/sql/connect/ado-net/sqlclient-streaming-support).
</exception>
closed or dropped during a streaming operation. For more information about streaming, see [SqlClient Streaming Support](/sql/connect/ado-net/sqlclient-streaming-support).

- or -

<see cref="P:Microssoft.Data.SqlClient.SqlCommand.EnableOptimizedParameterBinding" />
is set to true and a parameter with direction Output or InputOutput has been added to the <see cref="P:Microsoft.Data.SqlClient.SqlCommand.Parameters" /> collection.
</exception>
<exception cref="T:System.IO.IOException">
An error occurred in a
<see cref="T:System.IO.Stream" />
Expand Down Expand Up @@ -1343,6 +1383,33 @@ The <xref:Microsoft.Data.SqlClient.SqlCommand.CreateParameter%2A> method is a st
To be added.
</remarks>
</Dispose>
<EnableOptimizedParameterBinding>
<summary>
Gets or sets a value indicating whether the command object should optimize parameter performance by disabling Output and InputOutput directions when submitting the command to the SQL Server.
</summary>
<value>
A value indicating whether the command object should optimize parameter performance by disabling Output and InputOuput parameter directions when submitting the command to the SQL Server.
The default is <see langword="false" />.
</value>
<remarks>
<format type="text/markdown">
<![CDATA[
## Remarks
You must set the value for this property before the command is executed for it to take effect.

When a command is submitted to the server with parameters a list of the parameter names is sent as part of the submission. The list is used on the server to match Output and InputOutput parameters to the results of the query execution so that the values can be returned to the caller. This option disables the construction and submission of the parameter name list and as a consequence disables the use of Output and InputOutput parameters. The return parameter is not affected by this option.

A command sent with this option changes the way parameters are handled on the server, because there is no need to maintain an output parameter map. The result of this change is that queries with large numbers of input parameters may execute much faster.

The fewest number of parameters where this will take effect depends on the individual situation and should be detected by measuring query duration with and without the option enabled. Any query with more than 24 parameters may show lower overall query duration. Queries with parameter counts lower than 24 are unlikely to show a difference.
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you explain in which conditions it would have a negative perf effect (for input parameters)-not neutral- if there was such a condition?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There isn't a negative case. The only negative consequence is disabling output parameters. For small numbers of parameters the performance will be virtually identical it's only as the numbers rise that the difference becomes meaningful. Check the numbers in #1041 (comment)

Copy link
Contributor

Choose a reason for hiding this comment

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

If there is a match point to decide internally, my suggestion is to have this property with multiple configurations:

OptimizedParameterBinding

  • Disable (default)
  • Enable
  • Auto

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My concern with auto is that we make a change to the way the query is run without user interaction. This leads to the kind of situation where something with 30 parameters works fine but something with 31 works in some unexpected way and since the user can't see a difference in their code it is interpreted as random. The only way to see the problem at that point is probably to use wireshark which is normally a long long way down the list of debugging tools close to last resort because of the difficulty involved.

I view this as an advanced option. I don't believe that this is a casual option that would ever just be defaulted to true. If you opt in then we try to do what you asked and if we can't we throw an exception at you.
The decision making is all down to the user and by default we continue the behaviour that people have been using for the last 20 years so we don't accidentally cause some poor maintenance programmer a really really bad week (or more).


> [!NOTE]
If the option is enabled and a parameter with Direction Output or InputOutput is present in the Parameters collection an InvalidOperationException will be thrown when the command is executed.
Comment on lines +1406 to +1407
Copy link
Contributor

Choose a reason for hiding this comment

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

This property is going to optimize the command execution, I think it'd better to just skip it if it doesn't commit the requirements instead of raising an exception.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So just set the value to false? It's compatible but it will mean silent performance regression if someone adds an output parameter to a query that is expected to contain only input parameters. We'd essentially be silently disabling something that the user had to explicitly tell us that they wanted and I'm sure I'd be quite annoyed by that behaviour as a consumer having to track it down as a performance problem.


]]>
</format>
</remarks>
</EnableOptimizedParameterBinding>
<EndExecuteNonQuery name="IAsyncResult">
<param name="asyncResult">
The
Expand Down
2 changes: 2 additions & 0 deletions src/Microsoft.Data.SqlClient/netcore/ref/Microsoft.Data.SqlClient.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -535,6 +535,8 @@ public SqlCommand(string cmdText, Microsoft.Data.SqlClient.SqlConnection connect
[System.ComponentModel.DesignOnlyAttribute(true)]
[System.ComponentModel.EditorBrowsableAttribute(System.ComponentModel.EditorBrowsableState.Never)]
public override bool DesignTimeVisible { get { throw null; } set { } }
/// <include file='../../../../doc/snippets/Microsoft.Data.SqlClient/SqlCommand.xml' path='docs/members[@name="SqlCommand"]/EnableOptimizedParameterBinding/*'/>
public bool EnableOptimizedParameterBinding { get { throw null; } set { } }
/// <include file='../../../../doc/snippets/Microsoft.Data.SqlClient/SqlCommand.xml' path='docs/members[@name="SqlCommand"]/Parameters/*'/>
public new Microsoft.Data.SqlClient.SqlParameterCollection Parameters { get { throw null; } }
/// <include file='../../../../doc/snippets/Microsoft.Data.SqlClient/SqlCommand.xml' path='docs/members[@name="SqlCommand"]/Transaction/*'/>
Expand Down
3 changes: 3 additions & 0 deletions src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlCommand.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -692,6 +692,9 @@ public override bool DesignTimeVisible
}
}

/// <include file='../../../../../../../doc/snippets/Microsoft.Data.SqlClient/SqlCommand.xml' path='docs/members[@name="SqlCommand"]/EnableOptimizedParameterBinding/*'/>
public bool EnableOptimizedParameterBinding { get; set; }

/// <include file='../../../../../../../doc/snippets/Microsoft.Data.SqlClient/SqlCommand.xml' path='docs/members[@name="SqlCommand"]/Parameters/*'/>
new public SqlParameterCollection Parameters
{
Expand Down
5 changes: 5 additions & 0 deletions src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlUtil.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -495,6 +495,11 @@ internal static Exception ParameterCannotBeEmpty(string paramName)
return ADP.ArgumentNull(System.StringsHelper.GetString(Strings.SQL_ParameterCannotBeEmpty, paramName));
}

internal static Exception ParameterDirectionInvalidForOptimizedBinding(string paramName)
{
return ADP.InvalidOperation(System.StringsHelper.GetString(Strings.SQL_ParameterDirectionInvalidForOptimizedBinding, paramName));
}

internal static Exception ActiveDirectoryInteractiveTimeout()
{
return ADP.TimeoutException(Strings.SQL_Timeout_Active_Directory_Interactive_Authentication);
Expand Down
32 changes: 21 additions & 11 deletions src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParser.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8988,14 +8988,19 @@ internal Task TdsExecuteRPC(SqlCommand cmd, _SqlRPC[] rpcArray, int timeout, boo
int parametersLength = rpcext.userParamCount + rpcext.systemParamCount;

bool isAdvancedTraceOn = SqlClientEventSource.Log.IsAdvancedTraceOn();
bool enableOptimizedParameterBinding = cmd.EnableOptimizedParameterBinding;

for (int i = (ii == startRpc) ? startParam : 0; i < parametersLength; i++)
{
byte options = 0;
SqlParameter param = rpcext.GetParameterByIndex(i, out options);
// Since we are reusing the parameters array, we cannot rely on length to indicate no of parameters.
if (param == null)
{
break; // End of parameters for this execute
}

ParameterDirection parameterDirection = param.Direction;

// Throw an exception if ForceColumnEncryption is set on a parameter and the ColumnEncryption is not enabled on SqlConnection or SqlCommand
if (param.ForceColumnEncryption &&
Expand All @@ -9007,12 +9012,17 @@ internal Task TdsExecuteRPC(SqlCommand cmd, _SqlRPC[] rpcArray, int timeout, boo

// Check if the applications wants to force column encryption to avoid sending sensitive data to server
if (param.ForceColumnEncryption && param.CipherMetadata == null
&& (param.Direction == ParameterDirection.Input || param.Direction == ParameterDirection.InputOutput))
&& (parameterDirection == ParameterDirection.Input || parameterDirection == ParameterDirection.InputOutput))
{
// Application wants a parameter to be encrypted before sending it to server, however server doesnt think this parameter needs encryption.
throw SQL.ParamUnExpectedEncryptionMetadata(param.ParameterName, rpcext.GetCommandTextOrRpcName());
}

if (enableOptimizedParameterBinding && (parameterDirection == ParameterDirection.Output || parameterDirection == ParameterDirection.InputOutput))
{
throw SQL.ParameterDirectionInvalidForOptimizedBinding(param.ParameterName);
}

// Validate parameters are not variable length without size and with null value.
param.Validate(i, isCommandProc);

Expand All @@ -9021,7 +9031,7 @@ internal Task TdsExecuteRPC(SqlCommand cmd, _SqlRPC[] rpcArray, int timeout, boo

if (mt.IsNewKatmaiType)
{
WriteSmiParameter(param, i, 0 != (options & TdsEnums.RPC_PARAM_DEFAULT), stateObj, isAdvancedTraceOn);
WriteSmiParameter(param, i, 0 != (options & TdsEnums.RPC_PARAM_DEFAULT), stateObj, enableOptimizedParameterBinding, isAdvancedTraceOn);
continue;
}

Expand All @@ -9031,7 +9041,7 @@ internal Task TdsExecuteRPC(SqlCommand cmd, _SqlRPC[] rpcArray, int timeout, boo
throw ADP.VersionDoesNotSupportDataType(mt.TypeName);
}

Task writeParamTask = TDSExecuteRPCAddParameter(stateObj, param, mt, options, cmd);
Task writeParamTask = TDSExecuteRPCAddParameter(stateObj, param, mt, options, cmd, enableOptimizedParameterBinding);

if (!sync)
{
Expand Down Expand Up @@ -9148,7 +9158,7 @@ internal Task TdsExecuteRPC(SqlCommand cmd, _SqlRPC[] rpcArray, int timeout, boo
}
}

private Task TDSExecuteRPCAddParameter(TdsParserStateObject stateObj, SqlParameter param, MetaType mt, byte options, SqlCommand command)
private Task TDSExecuteRPCAddParameter(TdsParserStateObject stateObj, SqlParameter param, MetaType mt, byte options, SqlCommand command, bool isAnonymous)
{
int tempLen;
object value = null;
Expand All @@ -9173,7 +9183,7 @@ private Task TDSExecuteRPCAddParameter(TdsParserStateObject stateObj, SqlParamet
}
}

WriteParameterName(param.ParameterNameFixed, stateObj);
WriteParameterName(param.ParameterNameFixed, stateObj, isAnonymous);

// Write parameter status
stateObj.WriteByte(options);
Expand Down Expand Up @@ -9695,11 +9705,11 @@ private void ExecuteFlushTaskCallback(Task tsk, TdsParserStateObject stateObj, T
}


private void WriteParameterName(string parameterName, TdsParserStateObject stateObj)
private void WriteParameterName(string parameterName, TdsParserStateObject stateObj, bool isAnonymous)
{
// paramLen
// paramName
if (!string.IsNullOrEmpty(parameterName))
if (!isAnonymous && !string.IsNullOrEmpty(parameterName))
{
Debug.Assert(parameterName.Length <= 0xff, "parameter name can only be 255 bytes, shouldn't get to TdsParser!");
int tempLen = parameterName.Length & 0xff;
Expand All @@ -9712,7 +9722,7 @@ private void WriteParameterName(string parameterName, TdsParserStateObject state
}
}

private void WriteSmiParameter(SqlParameter param, int paramIndex, bool sendDefault, TdsParserStateObject stateObj, bool advancedTraceIsOn)
private void WriteSmiParameter(SqlParameter param, int paramIndex, bool sendDefault, TdsParserStateObject stateObj, bool isAnonymous, bool advancedTraceIsOn)
{
//
// Determine Metadata
Expand Down Expand Up @@ -9771,7 +9781,7 @@ private void WriteSmiParameter(SqlParameter param, int paramIndex, bool sendDefa
//
// Write parameter metadata
//
WriteSmiParameterMetaData(metaData, sendDefault, stateObj);
WriteSmiParameterMetaData(metaData, sendDefault, isAnonymous, stateObj);

//
// Now write the value
Expand All @@ -9790,7 +9800,7 @@ private void WriteSmiParameter(SqlParameter param, int paramIndex, bool sendDefa
}

// Writes metadata portion of parameter stream from an SmiParameterMetaData object.
private void WriteSmiParameterMetaData(SmiParameterMetaData metaData, bool sendDefault, TdsParserStateObject stateObj)
private void WriteSmiParameterMetaData(SmiParameterMetaData metaData, bool sendDefault, bool isAnonymous, TdsParserStateObject stateObj)
{
// Determine status
byte status = 0;
Expand All @@ -9805,7 +9815,7 @@ private void WriteSmiParameterMetaData(SmiParameterMetaData metaData, bool sendD
}

// Write everything out
WriteParameterName(metaData.Name, stateObj);
WriteParameterName(metaData.Name, stateObj, isAnonymous);
stateObj.WriteByte(status);
WriteSmiTypeInfo(metaData, stateObj);
}
Expand Down
Loading