Skip to content

Commit 4f7cf71

Browse files
bbowyersmythbbowyersmyth
authored
Reduce temporary string creation during types load (#5986)
1 parent 05e0427 commit 4f7cf71

5 files changed

Lines changed: 116 additions & 76 deletions

File tree

‎src/Npgsql/Internal/NpgsqlDatabaseInfo.cs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -241,9 +241,9 @@ internal void ProcessTypes()
241241
ByFullName[type.DataTypeName.Value] = type;
242242
// If more than one type exists with the same partial name, we place a null value.
243243
// This allows us to detect this case later and force the user to use full names only.
244-
ByName[type.InternalName] = ByName.ContainsKey(type.InternalName)
245-
? null
246-
: type;
244+
var typeInternalName = type.InternalName;
245+
if (!ByName.TryAdd(typeInternalName, type))
246+
ByName[typeInternalName] = null;
247247

248248
switch (type)
249249
{

‎src/Npgsql/Internal/Postgres/DataTypeName.cs‎

Lines changed: 69 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ namespace Npgsql.Internal.Postgres;
2727
if (!validated)
2828
{
2929
var schemaEndIndex = fullyQualifiedDataTypeName.IndexOf('.');
30-
if (schemaEndIndex == -1)
30+
if (schemaEndIndex is -1 or 0)
3131
throw new ArgumentException("Given value does not contain a schema.", nameof(fullyQualifiedDataTypeName));
3232

3333
// Friendly array syntax is the only fully qualified name quirk that's allowed by postgres (see FromDisplayName).
@@ -86,108 +86,108 @@ public DataTypeName ToArrayName()
8686
if (unqualifiedNameSpan.StartsWith("_".AsSpan(), StringComparison.Ordinal))
8787
return this;
8888

89-
var unqualifiedName = unqualifiedNameSpan.ToString();
90-
if (unqualifiedName.Length + "_".Length > NAMEDATALEN)
91-
unqualifiedName = unqualifiedName.Substring(0, NAMEDATALEN - "_".Length);
89+
if (unqualifiedNameSpan.Length + "_".Length > NAMEDATALEN)
90+
unqualifiedNameSpan = unqualifiedNameSpan.Slice(0, NAMEDATALEN - "_".Length);
9291

93-
return new(Schema + "._" + unqualifiedName);
92+
return new(string.Concat(Schema, "._", unqualifiedNameSpan));
9493
}
9594

9695
// Static transform as defined by https://www.postgresql.org/docs/current/sql-createtype.html#SQL-CREATETYPE-RANGE
9796
// Manual testing on PG confirmed it's only the first occurence of 'range' that gets replaced.
9897
public DataTypeName ToDefaultMultirangeName()
9998
{
100-
var unqualifiedNameSpan = UnqualifiedNameSpan;
101-
if (UnqualifiedNameSpan.IndexOf("multirange".AsSpan(), StringComparison.Ordinal) != -1)
99+
var nameSpan = UnqualifiedNameSpan;
100+
if (nameSpan.IndexOf("multirange".AsSpan(), StringComparison.Ordinal) is not -1)
102101
return this;
103102

104-
var unqualifiedName = unqualifiedNameSpan.ToString();
105-
var rangeIndex = unqualifiedName.IndexOf("range", StringComparison.Ordinal);
106-
if (rangeIndex != -1)
103+
if (nameSpan.IndexOf("range", StringComparison.Ordinal) is var rangeIndex and not -1)
107104
{
108-
var str = unqualifiedName.Substring(0, rangeIndex) + "multirange" + unqualifiedName.Substring(rangeIndex + "range".Length);
109-
110-
return new($"{Schema}." + (unqualifiedName.Length + "multi".Length > NAMEDATALEN
111-
? str.Substring(0, NAMEDATALEN - "multi".Length)
112-
: str));
105+
nameSpan = string.Concat(nameSpan.Slice(0, rangeIndex), "multirange", nameSpan.Slice(rangeIndex + "range".Length));
106+
return new(string.Concat(SchemaSpan, ".",
107+
nameSpan.Length > NAMEDATALEN ? nameSpan.Slice(0, NAMEDATALEN) : nameSpan));
113108
}
114109

115-
return new($"{Schema}." + (unqualifiedName.Length + "multi".Length > NAMEDATALEN
116-
? unqualifiedName.Substring(0, NAMEDATALEN - "_multirange".Length) + "_multirange"
117-
: unqualifiedName + "_multirange"));
110+
if (nameSpan.Length + "_multirange".Length > NAMEDATALEN)
111+
nameSpan = nameSpan.Slice(0, NAMEDATALEN - "_multirange".Length);
112+
113+
return new(string.Concat(SchemaSpan, ".", nameSpan, "_multirange"));
118114
}
119115

120116
// Create a DataTypeName from a broader range of valid names.
121117
// including SQL aliases like 'timestamp without time zone', trailing facet info etc.
122118
public static DataTypeName FromDisplayName(string displayName, string? schema = null)
119+
=> FromDisplayName(displayName, schema, assumeUnqualified: false); // user strings may come fully qualified.
120+
121+
// This method is used during type loading, it allows us to accept friendly names in constructors, without having to preconcatenate the schema.
122+
internal static DataTypeName FromDisplayName(string displayName, string? schema, bool assumeUnqualified)
123123
{
124124
var displayNameSpan = displayName.AsSpan().Trim();
125125

126-
// If we have a schema we're done, Postgres doesn't do display name conversions on fully qualified names.
127-
// There is one exception and that's array syntax, which is always resolvable in both ways, while we want the canonical name.
128126
var schemaEndIndex = displayNameSpan.IndexOf('.');
129-
if (schemaEndIndex is not -1 &&
130-
string.IsNullOrEmpty(schema) &&
131-
!displayNameSpan.Slice(schemaEndIndex).StartsWith("_".AsSpan(), StringComparison.Ordinal) &&
132-
!displayNameSpan.EndsWith("[]".AsSpan(), StringComparison.Ordinal))
133-
return new(displayName);
134-
135-
// First we strip the schema to get the type name.
136-
if (schemaEndIndex is not -1 &&
137-
string.IsNullOrEmpty(schema))
127+
ReadOnlySpan<char> schemaSpan;
128+
if (schemaEndIndex is not -1 && !assumeUnqualified)
138129
{
139-
schema = displayNameSpan.Slice(0, schemaEndIndex).ToString();
130+
if (schema is not null)
131+
throw new ArgumentException("Schema provided for a fully qualified name.");
132+
133+
schemaSpan = displayNameSpan.Slice(0, schemaEndIndex);
140134
displayNameSpan = displayNameSpan.Slice(schemaEndIndex + 1);
141135
}
136+
else
137+
{
138+
schemaSpan = schema is null ? "pg_catalog" : schema.AsSpan();
139+
}
142140

143141
// Then we strip either of the two valid array representations to get the base type name (with or without facets).
144142
var isArray = false;
145-
if (displayNameSpan.StartsWith("_".AsSpan()))
143+
if (displayNameSpan.StartsWith("_", StringComparison.Ordinal))
146144
{
147145
isArray = true;
148146
displayNameSpan = displayNameSpan.Slice(1);
149147
}
150-
else if (displayNameSpan.EndsWith("[]".AsSpan()))
148+
else if (displayNameSpan.EndsWith("[]", StringComparison.Ordinal))
151149
{
152150
isArray = true;
153151
displayNameSpan = displayNameSpan.Slice(0, displayNameSpan.Length - 2);
154152
}
155153

156-
string mapped;
157-
if (schemaEndIndex is -1)
154+
if (schemaEndIndex is not -1)
158155
{
159-
// Finally we strip the facet info.
160-
var parenIndex = displayNameSpan.IndexOf('(');
161-
if (parenIndex > -1)
162-
displayNameSpan = displayNameSpan.Slice(0, parenIndex);
163-
164-
// Map any aliases to the internal type name.
165-
mapped = displayNameSpan.ToString() switch
166-
{
167-
"boolean" => "bool",
168-
"character" => "bpchar",
169-
"decimal" => "numeric",
170-
"real" => "float4",
171-
"double precision" => "float8",
172-
"smallint" => "int2",
173-
"integer" => "int4",
174-
"bigint" => "int8",
175-
"time without time zone" => "time",
176-
"timestamp without time zone" => "timestamp",
177-
"time with time zone" => "timetz",
178-
"timestamp with time zone" => "timestamptz",
179-
"bit varying" => "varbit",
180-
"character varying" => "varchar",
181-
var value => value
182-
};
156+
// If we have a schema we're done, Postgres doesn't do display name conversions on fully qualified names.
157+
// There is one exception and that's array syntax, which is always resolvable in both ways, while we want the canonical name.
158+
return !isArray
159+
? new(displayName.Length == schemaEndIndex + displayNameSpan.Length
160+
? displayName
161+
: string.Concat(schemaSpan, ".", displayNameSpan))
162+
: new(string.Concat(schemaSpan, ".", "_", displayNameSpan));
183163
}
184-
else
164+
165+
// Finally we strip the facet info.
166+
var parenIndex = displayNameSpan.IndexOf('(');
167+
if (parenIndex > -1)
168+
displayNameSpan = displayNameSpan.Slice(0, parenIndex);
169+
170+
// Map any aliases to the internal type name.
171+
var mapped = displayNameSpan switch
185172
{
186-
// If we had a schema originally we stop here, see comment at schemaEndIndex.
187-
mapped = displayNameSpan.ToString();
188-
}
173+
"boolean" => "bool",
174+
"character" => "bpchar",
175+
"decimal" => "numeric",
176+
"real" => "float4",
177+
"double precision" => "float8",
178+
"smallint" => "int2",
179+
"integer" => "int4",
180+
"bigint" => "int8",
181+
"time without time zone" => "time",
182+
"timestamp without time zone" => "timestamp",
183+
"time with time zone" => "timetz",
184+
"timestamp with time zone" => "timestamptz",
185+
"bit varying" => "varbit",
186+
"character varying" => "varchar",
187+
var value => value
188+
};
189189

190-
return new((schema ?? "pg_catalog") + "." + (isArray ? "_" : "") + mapped);
190+
return new(string.Concat(schemaSpan, ".", isArray ? "_" : "", mapped));
191191
}
192192

193193
// The type names stored in a DataTypeName are usually the actual typname from the pg_type column.
@@ -197,8 +197,8 @@ public static DataTypeName FromDisplayName(string displayName, string? schema =
197197
// Alternatively some of the source lives at https://github.com/postgres/postgres/blob/c8e1ba736b2b9e8c98d37a5b77c4ed31baf94147/src/backend/utils/adt/format_type.c#L186
198198
static string ToDisplayName(ReadOnlySpan<char> unqualifiedName)
199199
{
200-
var isArray = unqualifiedName.IndexOf('_') == 0;
201-
var baseTypeName = isArray ? unqualifiedName.Slice(1).ToString() : unqualifiedName.ToString();
200+
var isArray = unqualifiedName.IndexOf('_') is 0;
201+
var baseTypeName = isArray ? unqualifiedName.Slice(1) : unqualifiedName;
202202

203203
var mappedBaseType = baseTypeName switch
204204
{
@@ -216,13 +216,12 @@ static string ToDisplayName(ReadOnlySpan<char> unqualifiedName)
216216
"timestamptz" => "timestamp with time zone",
217217
"varbit" => "bit varying",
218218
"varchar" => "character varying",
219-
_ => baseTypeName
219+
_ => null
220220
};
221221

222-
if (isArray)
223-
return mappedBaseType + "[]";
224-
225-
return mappedBaseType;
222+
return isArray
223+
? string.Concat(mappedBaseType ?? baseTypeName, "[]")
224+
: mappedBaseType ?? baseTypeName.ToString();
226225
}
227226

228227
internal static bool IsFullyQualified(ReadOnlySpan<char> dataTypeName) => dataTypeName.Contains(".".AsSpan(), StringComparison.Ordinal);

‎src/Npgsql/PostgresTypes/PostgresType.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ public abstract class PostgresType
2424
/// <param name="oid">The data type's OID.</param>
2525
private protected PostgresType(string ns, string name, uint oid)
2626
{
27-
DataTypeName = DataTypeName.FromDisplayName(name, ns);
27+
DataTypeName = DataTypeName.FromDisplayName(name, ns, assumeUnqualified: true);
2828
OID = oid;
2929
FullName = Namespace + "." + Name;
3030
}

‎src/Npgsql/PostgresTypes/PostgresUnknownType.cs‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
1-
namespace Npgsql.PostgresTypes;
1+
using Npgsql.Internal.Postgres;
2+
3+
namespace Npgsql.PostgresTypes;
24

35
/// <summary>
46
/// Represents a PostgreSQL data type that isn't known to Npgsql and cannot be handled.
@@ -10,5 +12,5 @@ public sealed class UnknownBackendType : PostgresType
1012
/// <summary>
1113
/// Constructs a the unknown backend type.
1214
/// </summary>
13-
UnknownBackendType() : base("", "<unknown>", 0) { }
15+
UnknownBackendType() : base(DataTypeName.Unspecified,0) { }
1416
}

‎test/Npgsql.Tests/DataTypeNameTests.cs‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,4 +23,43 @@ public void TooLongDataTypeName()
2323
var exception = Assert.Throws<ArgumentException>(() => new DataTypeName(fullyQualifiedDataTypeName));
2424
Assert.That(exception!.Message, Does.EndWith($": public.{new string('a', DataTypeName.NAMEDATALEN)}"));
2525
}
26+
27+
[TestCase("public.name", ExpectedResult = "public._name")]
28+
[TestCase("public._name", ExpectedResult = "public._name")]
29+
[TestCase("public.zzzaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa123", ExpectedResult = "public._zzzaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa12")]
30+
public string ToArrayName(string name)
31+
=> new DataTypeName(name).ToArrayName();
32+
33+
[TestCase("public.multirange", ExpectedResult = "public.multirange")]
34+
[TestCase("public.abcmultirange123", ExpectedResult = "public.abcmultirange123")]
35+
[TestCase("public.multiRANGE", ExpectedResult = "public.multiRANGE_multirange")]
36+
public string ToDefaultMultirangeNameHasMultiRange(string name)
37+
=> new DataTypeName(name).ToDefaultMultirangeName();
38+
39+
[TestCase("public.range", ExpectedResult = "public.multirange")]
40+
[TestCase("public.abcrange123", ExpectedResult = "public.abcmultirange123")]
41+
[TestCase("public.aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaarange", ExpectedResult = "public.aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaamultirange")] // Replace goes to max length
42+
[TestCase("public.aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaarange1", ExpectedResult = "public.aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaamultir")] // Replace goes over max length
43+
[TestCase("public.RANGE", ExpectedResult = "public.RANGE_multirange")]
44+
public string ToDefaultMultirangeNameHasRange(string name)
45+
=> new DataTypeName(name).ToDefaultMultirangeName();
46+
47+
[TestCase("public.name", null, ExpectedResult = "public.name")]
48+
[TestCase("public._name", null, ExpectedResult = "public._name")]
49+
[TestCase("public.name[]", null, ExpectedResult = "public._name")]
50+
[TestCase("public.integer", null, ExpectedResult = "public.integer")]
51+
[TestCase("name", null, ExpectedResult = "pg_catalog.name")]
52+
[TestCase("_name", null, ExpectedResult = "pg_catalog._name")]
53+
[TestCase("name[]", null, ExpectedResult = "pg_catalog._name")]
54+
[TestCase("character varying", null, ExpectedResult = "pg_catalog.varchar")]
55+
[TestCase("decimal(facet_name)", null, ExpectedResult = "pg_catalog.numeric")]
56+
[TestCase("name", "public", ExpectedResult = "public.name")]
57+
[TestCase("name ", "public", ExpectedResult = "public.name")]
58+
[TestCase("_name", "public", ExpectedResult = "public._name")]
59+
[TestCase("name[]", "public", ExpectedResult = "public._name")]
60+
[TestCase("timestamp with time zone", "public", ExpectedResult = "public.timestamptz")]
61+
[TestCase("boolean(facet_name)", "public", ExpectedResult = "public.bool")]
62+
[TestCase(" public.name ", null, ExpectedResult = "public.name")]
63+
public string FromDisplayName(string name, string? schema)
64+
=> DataTypeName.FromDisplayName(name, schema).Value;
2665
}

0 commit comments

Comments
 (0)