Skip to content

Commit 63cfeb7

Browse files
NinoFlorisNinoFloris
authored
Move type loading to use the data type name constructors (#6507)
1 parent 0eb4e95 commit 63cfeb7

4 files changed

Lines changed: 68 additions & 47 deletions

File tree

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

Lines changed: 30 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -52,13 +52,15 @@ public DataTypeName(string fullyQualifiedDataTypeName)
5252
internal static DataTypeName ValidatedName(string fullyQualifiedDataTypeName)
5353
=> new(fullyQualifiedDataTypeName, validated: true);
5454

55+
bool IsUnqualifiedDisplayName => SchemaSpan is "pg_catalog" || IsUnqualified;
56+
5557
// Includes schema unless it's pg_catalog or the schema is an invalid character used to represent an unspecified schema.
5658
public string DisplayName =>
57-
Value.StartsWith("pg_catalog", StringComparison.Ordinal) || IsUnqualified
59+
IsUnqualifiedDisplayName
5860
? UnqualifiedDisplayName
5961
: Schema + "." + UnqualifiedDisplayName;
6062

61-
public string UnqualifiedDisplayName => ToDisplayName(UnqualifiedNameSpan);
63+
public string UnqualifiedDisplayName => ToDisplayName(UnqualifiedNameSpan, mapAliases: IsUnqualifiedDisplayName);
6264

6365
internal ReadOnlySpan<char> SchemaSpan => Value.AsSpan(0, _value.IndexOf('.'));
6466
public string Schema => Value.Substring(0, _value.IndexOf('.'));
@@ -124,27 +126,20 @@ public DataTypeName ToDefaultMultirangeName()
124126

125127
// Create a DataTypeName from a broader range of valid names.
126128
// including SQL aliases like 'timestamp without time zone', trailing facet info etc.
127-
public static DataTypeName FromDisplayName(string displayName, string? schema = null)
128-
=> FromDisplayName(displayName, schema, assumeUnqualified: false); // user strings may come fully qualified.
129-
130-
// This method is used during type loading, it allows us to accept friendly names in constructors, without having to preconcatenate the schema.
131-
internal static DataTypeName FromDisplayName(string displayName, string? schema, bool assumeUnqualified)
129+
public static DataTypeName FromDisplayName(string displayName)
132130
{
133131
var displayNameSpan = displayName.AsSpan().Trim();
134132

135133
var schemaEndIndex = displayNameSpan.IndexOf('.');
136134
ReadOnlySpan<char> schemaSpan;
137-
if (schemaEndIndex is not -1 && !assumeUnqualified)
135+
if (schemaEndIndex is not -1)
138136
{
139-
if (schema is not null)
140-
throw new ArgumentException("Schema provided for a fully qualified name.");
141-
142137
schemaSpan = displayNameSpan.Slice(0, schemaEndIndex);
143138
displayNameSpan = displayNameSpan.Slice(schemaEndIndex + 1);
144139
}
145140
else
146141
{
147-
schemaSpan = schema is null ? $"{InvalidIdentifier}" : schema.AsSpan();
142+
schemaSpan = $"{InvalidIdentifier}";
148143
}
149144

150145
// Then we strip either of the two valid array representations to get the base type name (with or without facets).
@@ -196,7 +191,7 @@ internal static DataTypeName FromDisplayName(string displayName, string? schema,
196191
var value => value
197192
};
198193

199-
if (schema is null && DataTypeNames.IsWellKnownUnqualifiedName(mapped))
194+
if (DataTypeNames.IsWellKnownUnqualifiedName(mapped))
200195
schemaSpan = "pg_catalog".AsSpan();
201196

202197
return new(string.Concat(schemaSpan, ".", isArray ? "_" : "", mapped));
@@ -207,29 +202,33 @@ internal static DataTypeName FromDisplayName(string displayName, string? schema,
207202
// Additionally array types have a '_' prefix while for readability their element type should be postfixed with '[]'.
208203
// See the table for all the aliases https://www.postgresql.org/docs/current/static/datatype.html#DATATYPE-TABLE
209204
// Alternatively some of the source lives at https://github.com/postgres/postgres/blob/c8e1ba736b2b9e8c98d37a5b77c4ed31baf94147/src/backend/utils/adt/format_type.c#L186
210-
static string ToDisplayName(ReadOnlySpan<char> unqualifiedName)
205+
static string ToDisplayName(ReadOnlySpan<char> unqualifiedName, bool mapAliases)
211206
{
212207
var isArray = unqualifiedName.IndexOf('_') is 0;
213208
var baseTypeName = isArray ? unqualifiedName.Slice(1) : unqualifiedName;
214209

215-
var mappedBaseType = baseTypeName switch
210+
string? mappedBaseType = null;
211+
if (mapAliases)
216212
{
217-
"bool" => "boolean",
218-
"bpchar" => "character",
219-
"decimal" => "numeric",
220-
"float4" => "real",
221-
"float8" => "double precision",
222-
"int2" => "smallint",
223-
"int4" => "integer",
224-
"int8" => "bigint",
225-
"time" => "time without time zone",
226-
"timestamp" => "timestamp without time zone",
227-
"timetz" => "time with time zone",
228-
"timestamptz" => "timestamp with time zone",
229-
"varbit" => "bit varying",
230-
"varchar" => "character varying",
231-
_ => null
232-
};
213+
mappedBaseType = baseTypeName switch
214+
{
215+
"bool" => "boolean",
216+
"bpchar" => "character",
217+
"decimal" => "numeric",
218+
"float4" => "real",
219+
"float8" => "double precision",
220+
"int2" => "smallint",
221+
"int4" => "integer",
222+
"int8" => "bigint",
223+
"time" => "time without time zone",
224+
"timestamp" => "timestamp without time zone",
225+
"timetz" => "time with time zone",
226+
"timestamptz" => "timestamp with time zone",
227+
"varbit" => "bit varying",
228+
"varchar" => "character varying",
229+
_ => null
230+
};
231+
}
233232

234233
return isArray
235234
? string.Concat(mappedBaseType ?? baseTypeName, "[]")

‎src/Npgsql/PostgresDatabaseInfo.cs‎

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
using Microsoft.Extensions.Logging.Abstractions;
99
using Npgsql.BackendMessages;
1010
using Npgsql.Internal;
11+
using Npgsql.Internal.Postgres;
1112
using Npgsql.PostgresTypes;
1213
using Npgsql.Util;
1314
using static Npgsql.Util.Statics;
@@ -523,7 +524,7 @@ bool TryAddPostgresType(PostgresTypeDefinition postgresTypeDefinition, Dictionar
523524
switch (postgresTypeDefinition.Type)
524525
{
525526
case 'b': // Normal base type
526-
var baseType = new PostgresBaseType(postgresTypeDefinition.Namespace, postgresTypeDefinition.Name, postgresTypeDefinition.OID);
527+
var baseType = new PostgresBaseType(postgresTypeDefinition.DataTypeName, postgresTypeDefinition.OID);
527528
byOID[baseType.OID] = baseType;
528529
return true;
529530

@@ -537,7 +538,7 @@ bool TryAddPostgresType(PostgresTypeDefinition postgresTypeDefinition, Dictionar
537538
return false;
538539
}
539540

540-
var arrayType = new PostgresArrayType(postgresTypeDefinition.Namespace, postgresTypeDefinition.Name, postgresTypeDefinition.OID, elementPostgresType);
541+
var arrayType = new PostgresArrayType(postgresTypeDefinition.DataTypeName, postgresTypeDefinition.OID, elementPostgresType);
541542
byOID[arrayType.OID] = arrayType;
542543
return true;
543544
}
@@ -552,7 +553,7 @@ bool TryAddPostgresType(PostgresTypeDefinition postgresTypeDefinition, Dictionar
552553
return false;
553554
}
554555

555-
var rangeType = new PostgresRangeType(postgresTypeDefinition.Namespace, postgresTypeDefinition.Name, postgresTypeDefinition.OID, subtypePostgresType);
556+
var rangeType = new PostgresRangeType(postgresTypeDefinition.DataTypeName, postgresTypeDefinition.OID, subtypePostgresType);
556557
byOID[rangeType.OID] = rangeType;
557558
return true;
558559
}
@@ -573,17 +574,17 @@ bool TryAddPostgresType(PostgresTypeDefinition postgresTypeDefinition, Dictionar
573574
return false;
574575
}
575576

576-
var multirangeType = new PostgresMultirangeType(postgresTypeDefinition.Namespace, postgresTypeDefinition.Name, postgresTypeDefinition.OID, rangePostgresType);
577+
var multirangeType = new PostgresMultirangeType(postgresTypeDefinition.DataTypeName, postgresTypeDefinition.OID, rangePostgresType);
577578
byOID[multirangeType.OID] = multirangeType;
578579
return true;
579580

580581
case 'e': // Enum
581-
var enumType = new PostgresEnumType(postgresTypeDefinition.Namespace, postgresTypeDefinition.Name, postgresTypeDefinition.OID);
582+
var enumType = new PostgresEnumType(postgresTypeDefinition.DataTypeName, postgresTypeDefinition.OID);
582583
byOID[enumType.OID] = enumType;
583584
return true;
584585

585586
case 'c': // Composite
586-
var compositeType = new PostgresCompositeType(postgresTypeDefinition.Namespace, postgresTypeDefinition.Name, postgresTypeDefinition.OID);
587+
var compositeType = new PostgresCompositeType(postgresTypeDefinition.DataTypeName, postgresTypeDefinition.OID);
587588
byOID[compositeType.OID] = compositeType;
588589
return true;
589590

@@ -596,7 +597,7 @@ bool TryAddPostgresType(PostgresTypeDefinition postgresTypeDefinition, Dictionar
596597
return false;
597598
}
598599

599-
var domainType = new PostgresDomainType(postgresTypeDefinition.Namespace, postgresTypeDefinition.Name, postgresTypeDefinition.OID, basePostgresType, postgresTypeDefinition.NotNull);
600+
var domainType = new PostgresDomainType(postgresTypeDefinition.DataTypeName, postgresTypeDefinition.OID, basePostgresType, postgresTypeDefinition.NotNull);
600601
byOID[domainType.OID] = domainType;
601602
return true;
602603

@@ -610,4 +611,7 @@ bool TryAddPostgresType(PostgresTypeDefinition postgresTypeDefinition, Dictionar
610611
}
611612
}
612613

613-
readonly record struct PostgresTypeDefinition(string Namespace, uint OID, string Name, char Type, bool NotNull, uint ElemTypeOID);
614+
readonly record struct PostgresTypeDefinition(string Namespace, uint OID, string Name, char Type, bool NotNull, uint ElemTypeOID)
615+
{
616+
public DataTypeName DataTypeName => DataTypeName.CreateFullyQualifiedName(Namespace + "." + Name);
617+
}

‎src/Npgsql/PostgresTypes/PostgresType.cs‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
using System;
2+
using System.Diagnostics.CodeAnalysis;
23
using Npgsql.Internal.Postgres;
34

45
namespace Npgsql.PostgresTypes;
@@ -20,13 +21,12 @@ public abstract class PostgresType
2021
/// Constructs a representation of a PostgreSQL data type.
2122
/// </summary>
2223
/// <param name="ns">The data type's namespace (or schema).</param>
23-
/// <param name="name">The data type's name.</param>
24+
/// <param name="name">The data type's display name.</param>
2425
/// <param name="oid">The data type's OID.</param>
2526
private protected PostgresType(string ns, string name, uint oid)
2627
{
27-
DataTypeName = DataTypeName.FromDisplayName(name, ns, assumeUnqualified: true);
28+
DataTypeName = DataTypeName.FromDisplayName(ns is null or "pg_catalog" ? name : ns + "." + name);
2829
OID = oid;
29-
FullName = Namespace + "." + Name;
3030
}
3131

3232
/// <summary>
@@ -38,7 +38,6 @@ private protected PostgresType(DataTypeName dataTypeName, Oid oid)
3838
{
3939
DataTypeName = dataTypeName;
4040
OID = oid.Value;
41-
FullName = Namespace + "." + Name;
4241
}
4342

4443
#endregion
@@ -67,7 +66,8 @@ private protected PostgresType(DataTypeName dataTypeName, Oid oid)
6766
/// <summary>
6867
/// The full name of the backend type, including its namespace.
6968
/// </summary>
70-
public string FullName { get; }
69+
[field: MaybeNull]
70+
public string FullName => field ??= Namespace + "." + Name;
7171

7272
internal DataTypeName DataTypeName { get; }
7373

‎test/Npgsql.Tests/DataTypeNameTests.cs‎

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -60,9 +60,27 @@ public string ToDefaultMultirangeNameHasRange(string name)
6060
[TestCase("name ", "public", ExpectedResult = "public.name")]
6161
[TestCase("_name", "public", ExpectedResult = "public._name")]
6262
[TestCase("name[]", "public", ExpectedResult = "public._name")]
63-
[TestCase("timestamp with time zone", "public", ExpectedResult = "public.timestamptz")]
64-
[TestCase("boolean(facet_name)", "public", ExpectedResult = "public.bool")]
63+
[TestCase("timestamp with time zone", "public", ExpectedResult = "public.timestamp with time zone")]
64+
[TestCase("timestamp with time zone", "pg_catalog", ExpectedResult = "pg_catalog.timestamptz")]
65+
[TestCase("timestamp with time zone", null, ExpectedResult = "pg_catalog.timestamptz")]
66+
[TestCase("boolean(facet_name)", "public", ExpectedResult = "public.boolean(facet_name)")]
67+
[TestCase("boolean(facet_name)", "pg_catalog", ExpectedResult = "pg_catalog.bool")]
68+
[TestCase("boolean(facet_name)", null, ExpectedResult = "pg_catalog.bool")]
6569
[TestCase(" public.name ", null, ExpectedResult = "public.name")]
70+
[TestCase("decimal", "public", ExpectedResult = "public.decimal")]
71+
[TestCase("numeric", "public", ExpectedResult = "public.numeric")]
6672
public string FromDisplayName(string name, string? schema)
67-
=> DataTypeName.FromDisplayName(name, schema).Value;
73+
=> DataTypeName.FromDisplayName(schema is null or "pg_catalog" ? name : schema + "." + name).Value;
74+
75+
[TestCase("pg_catalog.bool", ExpectedResult = "boolean")]
76+
[TestCase("public.bool", ExpectedResult = "bool")]
77+
[TestCase("pg_catalog.numeric", ExpectedResult = "numeric")]
78+
[TestCase("pg_catalog._numeric", ExpectedResult = "numeric[]")]
79+
[TestCase("pg_catalog.decimal", ExpectedResult = "numeric")]
80+
[TestCase("public.numeric", ExpectedResult = "numeric")]
81+
[TestCase("public._numeric", ExpectedResult = "numeric[]")]
82+
[TestCase("public.decimal", ExpectedResult = "decimal")]
83+
[TestCase("public._decimal", ExpectedResult = "decimal[]")]
84+
public string UnqualifiedDisplayName(string fullyQualifiedName)
85+
=> new DataTypeName(fullyQualifiedName).UnqualifiedDisplayName;
6886
}

0 commit comments

Comments
 (0)