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

Strange logic in System.Data.ProviderBase.DbMetaDataFactory.FindMetaDataCollectionRow() #39620

Closed
yyjdelete opened this issue Jul 20, 2020 · 3 comments · Fixed by dotnet/SqlClient#668

Comments

@yyjdelete
Copy link

Description

internal DataRow FindMetaDataCollectionRow(string collectionName)
{
bool versionFailure;
bool haveExactMatch;
bool haveMultipleInexactMatches;
string candidateCollectionName;
DataTable metaDataCollectionsTable = _metaDataCollectionsDataSet.Tables[DbMetaDataCollectionNames.MetaDataCollections];
if (metaDataCollectionsTable == null)
{
throw ADP.InvalidXml();
}
DataColumn collectionNameColumn = metaDataCollectionsTable.Columns[DbMetaDataColumnNames.CollectionName];
if ((null == collectionNameColumn) || (typeof(string) != collectionNameColumn.DataType))
{
throw ADP.InvalidXmlMissingColumn(DbMetaDataCollectionNames.MetaDataCollections, DbMetaDataColumnNames.CollectionName);
}
DataRow requestedCollectionRow = null;
string exactCollectionName = null;
// find the requested collection
versionFailure = false;
haveExactMatch = false;
haveMultipleInexactMatches = false;
foreach (DataRow row in metaDataCollectionsTable.Rows)
{
candidateCollectionName = row[collectionNameColumn, DataRowVersion.Current] as string;
if (string.IsNullOrEmpty(candidateCollectionName))
{
throw ADP.InvalidXmlInvalidValue(DbMetaDataCollectionNames.MetaDataCollections, DbMetaDataColumnNames.CollectionName);
}
if (ADP.CompareInsensitiveInvariant(candidateCollectionName, collectionName))
{
if (SupportedByCurrentVersion(row) == false)
{
versionFailure = true;
}
else
{
if (collectionName == candidateCollectionName)
{
if (haveExactMatch == true)
{
throw ADP.CollectionNameIsNotUnique(collectionName);
}
requestedCollectionRow = row;
exactCollectionName = candidateCollectionName;
haveExactMatch = true;
}
else
{
// have an inexact match - ok only if it is the only one
if (exactCollectionName != null)
{
// can't fail here becasue we may still find an exact match
haveMultipleInexactMatches = true;
}
requestedCollectionRow = row;
exactCollectionName = candidateCollectionName;
}
}
}
}
if (requestedCollectionRow == null)
{
if (versionFailure == false)
{
throw ADP.UndefinedCollection(collectionName);
}
else
{
throw ADP.UnsupportedVersion(collectionName);
}
}
if ((haveExactMatch == false) && (haveMultipleInexactMatches == true))
{
throw ADP.AmbigousCollectionName(collectionName);
}
return requestedCollectionRow;
}

Seems the code here want to do an case-sensitive match first, and fallback to an case-insensitive match, and throw if get more than one matches(in case-sensitive or case-insensitive).
But if it first hit an case-sensitive match, and then some case-insensitive match, var requestedCollectionRow will be overwrite by the last case-insensitive match, but won't throw AmbigousCollectionName since haveExactMatch is true, and finally return the case-insensitive result instead of the extra match one.


Is it the expected logic? Maybe it should be else if (!haveExactMatch) here instead?

Regression?

No. And didn't hit any problem with this, just found this when do some research for the behavior of DbConnection.GetScmema().

Other information

Note: There is also some copy of this file in OleDb and MD.SqlClient, which have the same problem.
https://github.com/dotnet/runtime/search?q=FindMetaDataCollectionRow&unscoped_q=FindMetaDataCollectionRow
https://github.com/dotnet/sqlclient/search?q=FindMetaDataCollectionRow&unscoped_q=FindMetaDataCollectionRow

@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added area-System.Data untriaged New issue has not been triaged by the area owner labels Jul 20, 2020
@ghost
Copy link

ghost commented Jul 20, 2020

Tagging subscribers to this area: @roji, @ajcvickers
Notify danmosemsft if you want to be subscribed.

@roji
Copy link
Member

roji commented Jul 20, 2020

I agree this seems like a bug... @ajcvickers?

@ajcvickers ajcvickers added this to the Future milestone Jul 20, 2020
@ajcvickers ajcvickers removed the untriaged New issue has not been triaged by the area owner label Jul 20, 2020
@roji
Copy link
Member

roji commented Jul 27, 2020

@yyjdelete are you interested in submitting a PR to fix this?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
4 participants