Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.
21 changes: 15 additions & 6 deletions src/Common/src/System/Data/Common/AdapterUtil.cs
Original file line number Diff line number Diff line change
Expand Up @@ -361,24 +361,33 @@ internal static Exception StreamClosed([CallerMemberName] string method = "")

internal static string BuildQuotedString(string quotePrefix, string quoteSuffix, string unQuotedString)
{
var resultString = new StringBuilder();
var resultString = new StringBuilder(unQuotedString.Length + quoteSuffix.Length + quoteSuffix.Length);
AppendQuotedString(resultString, quotePrefix, quoteSuffix, unQuotedString);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Isn't the result of AppendQuotedString same value as resultString.ToString()?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The output is the same, The new path doesn't allocate a second stringbuilder or a string for that segment it just writes everything to the stringbuilder, That also allows the replace call to avoid a string allocation because it can be done in the internal buffer.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

My idea was that BuildQuotedString method can look like this:
`internal static string BuildQuotedString(string quotePrefix, string quoteSuffix, string unQuotedString)
{
var resultString = new StringBuilder(unQuotedString.Length + quoteSuffix.Length + quoteSuffix.Length);

        return AppendQuotedString(resultString, quotePrefix, quoteSuffix, unQuotedString);
    }`

But maybe I don't see something.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It could but that would change AppendQuotedString to be allocating when there's no need to. If you want to produce a multipart name you could call AppendQuotedString up to 4 times with . separators in between them. I chose to provide flexibility by having one method responsible for the lifetime of the builder and the other simply using it.

return resultString.ToString();
}

internal static string AppendQuotedString(StringBuilder buffer, string quotePrefix, string quoteSuffix, string unQuotedString)
{

if (!string.IsNullOrEmpty(quotePrefix))
{
resultString.Append(quotePrefix);
buffer.Append(quotePrefix);
}

// Assuming that the suffix is escaped by doubling it. i.e. foo"bar becomes "foo""bar".
if (!string.IsNullOrEmpty(quoteSuffix))
{
resultString.Append(unQuotedString.Replace(quoteSuffix, quoteSuffix + quoteSuffix));
resultString.Append(quoteSuffix);
int start = buffer.Length;
buffer.Append(unQuotedString);
buffer.Replace(quoteSuffix, quoteSuffix + quoteSuffix, start, unQuotedString.Length);
buffer.Append(quoteSuffix);
}
else
{
resultString.Append(unQuotedString);
buffer.Append(unQuotedString);
}

return resultString.ToString();
return buffer.ToString();
}

//
Expand Down
33 changes: 31 additions & 2 deletions src/System.Data.SqlClient/src/System/Data/SqlClient/SqlCommand.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2222,6 +2222,13 @@ internal void DeriveParameters()
p.TypeName = r[colNames[(int)ProcParamsColIndex.TypeCatalogName]] + "." +
r[colNames[(int)ProcParamsColIndex.TypeSchemaName]] + "." +
r[colNames[(int)ProcParamsColIndex.TypeName]];

// the constructed type name above is incorrectly formatted, it should be a 2 part name not 3
// for compatibility we can't change this because the bug has existed for a long time and been
// worked around by users, so identify that it is present and catch it later in the execution
// process once users can no longer interact with with the parameter type name
p.IsDerivedParameterTypeName = true;

}

// XmlSchema name for Xml types
Expand Down Expand Up @@ -3321,6 +3328,23 @@ private void SetUpRPCParameters(_SqlRPC rpc, int startCount, bool inSchema, SqlP
// set default value bit
if (parameter.Direction != ParameterDirection.Output)
{
// detect incorrectly derived type names unchanged by the caller and fix them
if (parameter.IsDerivedParameterTypeName)
{
string[] parts = MultipartIdentifier.ParseMultipartIdentifier(parameter.TypeName, "[\"", "]\"", SR.SQL_TDSParserTableName, false);
if (parts != null && parts.Length==4) // will always return int[4] right justified
{
if (
parts[3] != null && // name must not be null
parts[2] != null && // schema must not be null
parts[1] != null // server should not be null or we don't need to remove it
)
{
parameter.TypeName = QuoteIdentifier(parts.AsSpan(2,2));
}
}
}

// remember that null == Convert.IsEmpty, DBNull.Value is a database null!

// Don't assume a default value exists for parameters in the case when
Expand Down Expand Up @@ -3665,9 +3689,14 @@ internal string BuildParamList(TdsParser parser, SqlParameterCollection paramete

// Adds quotes to each part of a SQL identifier that may be multi-part, while leaving
// the result as a single composite name.
private string ParseAndQuoteIdentifier(string identifier, bool isUdtTypeName)
private static string ParseAndQuoteIdentifier(string identifier, bool isUdtTypeName)
{
string[] strings = SqlParameter.ParseTypeName(identifier, isUdtTypeName);
return QuoteIdentifier(strings);
}

private static string QuoteIdentifier(ReadOnlySpan<string> strings)
{
StringBuilder bld = new StringBuilder();

// Stitching back together is a little tricky. Assume we want to build a full multi-part name
Expand All @@ -3682,7 +3711,7 @@ private string ParseAndQuoteIdentifier(string identifier, bool isUdtTypeName)
}
if (null != strings[i] && 0 != strings[i].Length)
{
bld.Append(ADP.BuildQuotedString("[", "]", strings[i]));
ADP.AppendQuotedString(bld, "[", "]", strings[i]);
}
}

Expand Down
Loading