Skip to content

Commit d7d4cb7

Browse files
NinoFlorisNinoFloris
authored
Move parameter collection to case-insenstive matching (#4229)
* Move parameter collection to case-insenstive matching * Remove exceptions and address feedback * Whitespace * Enable test for both modes * Fix missing clone for second dictionary
1 parent 075312b commit d7d4cb7

4 files changed

Lines changed: 175 additions & 87 deletions

File tree

‎src/Npgsql/NpgsqlParameter.cs‎

Lines changed: 17 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -35,8 +35,9 @@ public class NpgsqlParameter : DbParameter, IDbDataParameter, ICloneable
3535
private protected object? _value;
3636
private protected string _sourceColumn;
3737

38-
internal string TrimmedName { get; private protected set; } = string.Empty;
39-
38+
internal string TrimmedName { get; private protected set; } = PositionalName;
39+
internal const string PositionalName = "";
40+
4041
/// <summary>
4142
/// Can be used to communicate a value from the validation phase to the writing phase.
4243
/// To be used by type handlers only.
@@ -246,20 +247,23 @@ public sealed override string ParameterName
246247
get => _name;
247248
set
248249
{
249-
var oldName = _name;
250-
var oldTrimmedName = TrimmedName;
251-
// ReSharper disable once ConditionIsAlwaysTrueOrFalse
252-
if (value == null)
253-
_name = TrimmedName = string.Empty;
254-
else if (value.Length > 0 && (value[0] == ':' || value[0] == '@'))
255-
TrimmedName = (_name = value).Substring(1);
256-
else
257-
_name = TrimmedName = value;
258-
259-
Collection?.ChangeParameterName(this, oldName, oldTrimmedName);
250+
if (Collection is not null)
251+
Collection.ChangeParameterName(this, value);
252+
else
253+
ChangeParameterName(value);
260254
}
261255
}
262256

257+
internal void ChangeParameterName(string? value)
258+
{
259+
if (value == null)
260+
_name = TrimmedName = PositionalName;
261+
else if (value.Length > 0 && (value[0] == ':' || value[0] == '@'))
262+
TrimmedName = (_name = value).Substring(1);
263+
else
264+
_name = TrimmedName = value;
265+
}
266+
263267
internal bool IsPositional => ParameterName.Length == 0;
264268

265269
#endregion Name

‎src/Npgsql/NpgsqlParameterCollection.cs‎

Lines changed: 69 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -20,18 +20,18 @@ public sealed class NpgsqlParameterCollection : DbParameterCollection, IList<Npg
2020

2121
internal List<NpgsqlParameter> InternalList { get; } = new(5);
2222
#if DEBUG
23-
internal static bool CaseInsensitiveCompatMode;
23+
internal static bool TwoPassCompatMode;
2424
#else
25-
internal static readonly bool CaseInsensitiveCompatMode;
25+
internal static readonly bool TwoPassCompatMode;
2626
#endif
2727

2828
static NpgsqlParameterCollection()
29-
=> CaseInsensitiveCompatMode = AppContext.TryGetSwitch("Npgsql.EnableLegacyCaseInsensitiveDbParameters", out var enabled)
30-
&& enabled;
29+
=> TwoPassCompatMode = AppContext.TryGetSwitch("Npgsql.EnableLegacyCaseInsensitiveDbParameters", out var enabled)
30+
&& enabled;
3131

32-
// Dictionary lookups for GetValue to improve performance
33-
Dictionary<string, int>? _lookup;
32+
// Dictionary lookups for GetValue to improve performance. _caseSensitiveLookup is only ever used in legacy two-pass mode.
3433
Dictionary<string, int>? _caseInsensitiveLookup;
34+
Dictionary<string, int>? _caseSensitiveLookup;
3535

3636
/// <summary>
3737
/// Initializes a new instance of the NpgsqlParameterCollection class.
@@ -42,33 +42,39 @@ internal NpgsqlParameterCollection() {}
4242

4343
void LookupClear()
4444
{
45-
_lookup?.Clear();
4645
_caseInsensitiveLookup?.Clear();
46+
_caseSensitiveLookup?.Clear();
4747
}
4848

4949
void LookupAdd(string name, int index)
5050
{
51-
if (_lookup is not null && !_lookup.ContainsKey(name))
52-
_lookup[name] = index;
51+
if (_caseInsensitiveLookup is null)
52+
return;
53+
54+
if (TwoPassCompatMode && !_caseSensitiveLookup!.ContainsKey(name))
55+
_caseSensitiveLookup[name] = index;
5356

54-
if (CaseInsensitiveCompatMode && _caseInsensitiveLookup is not null && !_caseInsensitiveLookup.ContainsKey(name))
57+
if (!_caseInsensitiveLookup.ContainsKey(name))
5558
_caseInsensitiveLookup[name] = index;
5659
}
5760

5861
void LookupInsert(string name, int index)
5962
{
60-
if (_lookup is not null && (!_lookup.TryGetValue(name, out var indexCs) || index < indexCs))
63+
if (_caseInsensitiveLookup is null)
64+
return;
65+
66+
if (TwoPassCompatMode &&
67+
(!_caseSensitiveLookup!.TryGetValue(name, out var indexCs) || index < indexCs))
6168
{
62-
foreach (var kv in _lookup)
69+
foreach (var kv in _caseSensitiveLookup)
6370
{
6471
if (index <= kv.Value)
65-
_lookup[kv.Key] = kv.Value + 1;
72+
_caseSensitiveLookup[kv.Key] = kv.Value + 1;
6673
}
67-
_lookup[name] = index;
74+
_caseSensitiveLookup[name] = index;
6875
}
6976

70-
if (CaseInsensitiveCompatMode && _caseInsensitiveLookup is not null &&
71-
(!_caseInsensitiveLookup.TryGetValue(name, out var indexCi) || index < indexCi))
77+
if (!_caseInsensitiveLookup.TryGetValue(name, out var indexCi) || index < indexCi)
7278
{
7379
foreach (var kv in _caseInsensitiveLookup)
7480
{
@@ -81,26 +87,27 @@ void LookupInsert(string name, int index)
8187

8288
void LookupRemove(string name, int index)
8389
{
84-
if (_lookup is not null && _lookup.ContainsKey(name))
90+
if (_caseInsensitiveLookup is null)
91+
return;
92+
93+
if (TwoPassCompatMode && _caseSensitiveLookup!.Remove(name))
8594
{
86-
_lookup.Remove(name);
87-
foreach (var kv in _lookup)
95+
foreach (var kv in _caseSensitiveLookup)
8896
{
8997
if (index < kv.Value)
90-
_lookup[kv.Key] = kv.Value - 1;
98+
_caseSensitiveLookup[kv.Key] = kv.Value - 1;
9199
}
92100
}
93101

94-
if (CaseInsensitiveCompatMode && _caseInsensitiveLookup is not null &&
95-
_caseInsensitiveLookup.TryGetValue(name, out var indexCi) && indexCi == index)
102+
if (_caseInsensitiveLookup.Remove(name))
96103
{
97-
_caseInsensitiveLookup.Remove(name);
98104
foreach (var kv in _caseInsensitiveLookup)
99105
{
100106
if (index < kv.Value)
101107
_caseInsensitiveLookup[kv.Key] = kv.Value - 1;
102108
}
103109

110+
// Fix-up the case-insensitive lookup to point to the next match, if any.
104111
for (var i = 0; i < InternalList.Count; i++)
105112
{
106113
var value = InternalList[i];
@@ -111,11 +118,12 @@ void LookupRemove(string name, int index)
111118
}
112119
}
113120
}
121+
114122
}
115123

116124
void LookupChangeName(NpgsqlParameter parameter, string oldName, string oldTrimmedName, int index)
117125
{
118-
if (string.Equals(oldTrimmedName, parameter.TrimmedName, CaseInsensitiveCompatMode ? StringComparison.OrdinalIgnoreCase : StringComparison.Ordinal))
126+
if (string.Equals(oldTrimmedName, parameter.TrimmedName, StringComparison.OrdinalIgnoreCase))
119127
return;
120128

121129
if (oldName.Length != 0)
@@ -124,9 +132,13 @@ void LookupChangeName(NpgsqlParameter parameter, string oldName, string oldTrimm
124132
LookupAdd(parameter.TrimmedName, index);
125133
}
126134

127-
internal void ChangeParameterName(NpgsqlParameter parameter, string oldName, string oldTrimmedName)
135+
internal void ChangeParameterName(NpgsqlParameter parameter, string? value)
128136
{
129-
if (_lookup is null || _lookup.Count == 0)
137+
var oldName = parameter.ParameterName;
138+
var oldTrimmedName = parameter.TrimmedName;
139+
parameter.ChangeParameterName(value);
140+
141+
if (_caseInsensitiveLookup is null || _caseInsensitiveLookup.Count == 0)
130142
return;
131143

132144
var index = IndexOf(parameter);
@@ -169,6 +181,9 @@ internal void ChangeParameterName(NpgsqlParameter parameter, string oldName, str
169181
if (index == -1)
170182
throw new ArgumentException("Parameter not found");
171183

184+
if (!string.Equals(parameterName, value.TrimmedName, StringComparison.OrdinalIgnoreCase))
185+
throw new ArgumentException("Parameter name must be a case-insensitive match with the property 'ParameterName' on the given NpgsqlParameter", nameof(parameterName));
186+
172187
var oldValue = InternalList[index];
173188
LookupChangeName(value, oldValue.ParameterName, oldValue.TrimmedName, index);
174189

@@ -193,7 +208,7 @@ internal void ChangeParameterName(NpgsqlParameter parameter, string oldName, str
193208

194209
var oldValue = InternalList[index];
195210

196-
if (oldValue == value)
211+
if (ReferenceEquals(oldValue, value))
197212
return;
198213

199214
LookupChangeName(value, oldValue.ParameterName, oldValue.TrimmedName, index);
@@ -348,38 +363,45 @@ public override int IndexOf(string parameterName)
348363
// For string equality this is the case after ~3 items so we take a decent compromise going with 5.
349364
if (LookupEnabled && parameterName.Length != 0)
350365
{
351-
if (_lookup is null)
366+
if (_caseInsensitiveLookup is null)
352367
BuildLookup();
353368

354-
if (_lookup!.TryGetValue(parameterName, out var indexCs))
369+
if (TwoPassCompatMode && _caseSensitiveLookup!.TryGetValue(parameterName, out var indexCs))
355370
return indexCs;
356371

357-
if (CaseInsensitiveCompatMode && _caseInsensitiveLookup!.TryGetValue(parameterName, out var indexCi))
372+
if (_caseInsensitiveLookup!.TryGetValue(parameterName, out var indexCi))
358373
return indexCi;
359374

360375
return -1;
361376
}
362377

378+
// Start with case-sensitive search in two pass mode.
379+
if (TwoPassCompatMode)
380+
{
381+
for (var i = 0; i < InternalList.Count; i++)
382+
{
383+
var name = InternalList[i].TrimmedName;
384+
if (string.Equals(parameterName, InternalList[i].TrimmedName))
385+
return i;
386+
}
387+
}
388+
389+
// Then do case-insensitive search.
363390
for (var i = 0; i < InternalList.Count; i++)
364391
{
365392
var name = InternalList[i].TrimmedName;
366-
if (ReferenceEquals(parameterName, name) || string.Equals(parameterName, name))
393+
if (ReferenceEquals(parameterName, name) || string.Equals(parameterName, name, StringComparison.OrdinalIgnoreCase))
367394
return i;
368395
}
369396

370-
// Fall back to a case-insensitive search.
371-
if (CaseInsensitiveCompatMode)
372-
for (var i = 0; i < InternalList.Count; i++)
373-
if (string.Equals(parameterName, InternalList[i].TrimmedName, StringComparison.OrdinalIgnoreCase))
374-
return i;
375-
376397
return -1;
377398

378399
void BuildLookup()
379400
{
380-
_lookup = new Dictionary<string, int>(InternalList.Count, StringComparer.Ordinal);
381-
if (CaseInsensitiveCompatMode)
382-
_caseInsensitiveLookup = new Dictionary<string, int>(InternalList.Count, StringComparer.OrdinalIgnoreCase);
401+
if (TwoPassCompatMode)
402+
_caseSensitiveLookup = new Dictionary<string, int>(InternalList.Count, StringComparer.Ordinal);
403+
404+
_caseInsensitiveLookup = new Dictionary<string, int>(InternalList.Count, StringComparer.OrdinalIgnoreCase);
383405

384406
for (var i = 0; i < InternalList.Count; i++)
385407
{
@@ -640,7 +662,13 @@ internal void CloneTo(NpgsqlParameterCollection other)
640662
newParam.Collection = this;
641663
other.InternalList.Add(newParam);
642664
}
643-
other._lookup = _lookup;
665+
666+
if (LookupEnabled)
667+
{
668+
other._caseInsensitiveLookup = new Dictionary<string, int>(_caseInsensitiveLookup!, StringComparer.OrdinalIgnoreCase);
669+
if (TwoPassCompatMode)
670+
other._caseSensitiveLookup = new Dictionary<string, int>(_caseSensitiveLookup!, StringComparer.Ordinal);
671+
}
644672
}
645673

646674
internal void ValidateAndBind(ConnectorTypeMapper typeMapper)

0 commit comments

Comments
 (0)