Skip to content

Commit b9811a2

Browse files
NinoFlorisNinoFloris
authored
Remove unknown pgtype dependency (#5596)
Fixes #5503
1 parent 709e762 commit b9811a2

9 files changed

Lines changed: 117 additions & 23 deletions

File tree

‎src/Npgsql/BackendMessages/RowDescriptionMessage.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -368,7 +368,7 @@ void GetInfoSlow(Type? type, out ColumnInfo lastColumnInfo)
368368
// For text we'll fall back to any available text converter for the expected clr type or throw.
369369
if (!typeInfo.TryBind(Field, DataFormat, out converterInfo))
370370
{
371-
typeInfo = AdoSerializerHelpers.GetTypeInfoForReading(type ?? typeof(string), _serializerOptions.UnknownPgType, _serializerOptions);
371+
typeInfo = AdoSerializerHelpers.GetTypeInfoForReading(type ?? typeof(string), _serializerOptions.TextPgType, _serializerOptions);
372372
converterInfo = typeInfo.Bind(Field, DataFormat);
373373
lastColumnInfo = new(converterInfo, DataFormat, type != converterInfo.TypeToConvert || converterInfo.IsBoxingConverter);
374374
}

‎src/Npgsql/Internal/NpgsqlDatabaseInfo.cs‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -228,6 +228,11 @@ public bool TryGetPostgresTypeByName(string pgName, [NotNullWhen(true)] out Post
228228

229229
internal void ProcessTypes()
230230
{
231+
var unspecified = new PostgresBaseType(DataTypeName.Unspecified, Oid.Unspecified);
232+
ByOID[Oid.Unspecified.Value] = unspecified;
233+
ByFullName[unspecified.DataTypeName.Value] = unspecified;
234+
ByName[unspecified.InternalName] = unspecified;
235+
231236
foreach (var type in GetTypes())
232237
{
233238
ByOID[type.OID] = type;

‎src/Npgsql/Internal/PgSerializerOptions.cs‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,11 +25,13 @@ internal PgSerializerOptions(NpgsqlDatabaseInfo databaseInfo, PgTypeInfoResolver
2525
_resolverChain = resolverChain ?? new();
2626
_timeZoneProvider = timeZoneProvider;
2727
DatabaseInfo = databaseInfo;
28-
UnknownPgType = databaseInfo.GetPostgresType("unknown");
28+
UnspecifiedDBNullTypeInfo = new(this, new Converters.Internal.VoidConverter(), DataTypeName.Unspecified, unboxedType: typeof(DBNull));
2929
}
3030

31-
// Represents the 'unknown' type, which can be used for reading and writing arbitrary text values.
32-
public PostgresType UnknownPgType { get; }
31+
internal PgTypeInfo UnspecifiedDBNullTypeInfo { get; }
32+
33+
PostgresType? _textPgType;
34+
internal PostgresType TextPgType => _textPgType ??= DatabaseInfo.GetPostgresType(DataTypeNames.Text);
3335

3436
// Used purely for type mapping, where we don't have a full set of types but resolvers might know enough.
3537
readonly bool _introspectionInstance;

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

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -48,9 +48,9 @@ public DataTypeName(string fullyQualifiedDataTypeName)
4848
internal static DataTypeName ValidatedName(string fullyQualifiedDataTypeName)
4949
=> new(fullyQualifiedDataTypeName, validated: true);
5050

51-
// Includes schema unless it's pg_catalog.
51+
// Includes schema unless it's pg_catalog or the name is unspecified.
5252
public string DisplayName =>
53-
Value.StartsWith("pg_catalog", StringComparison.Ordinal)
53+
Value.StartsWith("pg_catalog", StringComparison.Ordinal) || Value == Unspecified
5454
? UnqualifiedDisplayName
5555
: Schema + "." + UnqualifiedDisplayName;
5656

@@ -63,11 +63,13 @@ internal static DataTypeName ValidatedName(string fullyQualifiedDataTypeName)
6363
public string Value => _value is null ? ThrowDefaultException() : _value;
6464

6565
static string ThrowDefaultException() =>
66-
throw new InvalidOperationException($"This operation cannot be performed on a default instance of {nameof(DataTypeName)}.");
66+
throw new InvalidOperationException($"This operation cannot be performed on a default value of {nameof(DataTypeName)}.");
6767

6868
public static implicit operator string(DataTypeName value) => value.Value;
6969

70-
public bool IsDefault => _value is null;
70+
// This contains two invalid sql identifiers (schema and name are both separate identifiers, and would both have to be quoted to be valid).
71+
// Given this is an invalid name it's fine for us to represent a fully qualified 'unspecified' name with it.
72+
public static DataTypeName Unspecified => new("-.-", validated: true);
7173

7274
public bool IsArray => UnqualifiedNameSpan.StartsWith("_".AsSpan(), StringComparison.Ordinal);
7375

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ namespace Npgsql.Internal.Postgres;
99
public static explicit operator uint(Oid oid) => oid.Value;
1010
public static implicit operator Oid(uint oid) => new(oid);
1111
public uint Value { get; init; }
12+
public static Oid Unspecified => new(0);
1213

1314
public override string ToString() => Value.ToString();
1415
public bool Equals(Oid other) => Value == other.Value;

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,8 @@ namespace Npgsql.Internal.Postgres;
1515
public PgTypeId(Oid oid) => _oid = oid;
1616

1717
[MemberNotNullWhen(true, nameof(_dataTypeName))]
18-
public bool IsDataTypeName => !_dataTypeName.IsDefault;
19-
public bool IsOid => _dataTypeName.IsDefault;
18+
public bool IsDataTypeName => _dataTypeName != default;
19+
public bool IsOid => _dataTypeName == default;
2020

2121
public DataTypeName DataTypeName
2222
=> IsDataTypeName ? _dataTypeName : throw new InvalidOperationException("This value does not describe a DataTypeName.");
@@ -42,4 +42,6 @@ public bool Equals(PgTypeId other)
4242
public override int GetHashCode() => IsOid ? _oid.GetHashCode() : _dataTypeName.GetHashCode();
4343
public static bool operator ==(PgTypeId left, PgTypeId right) => left.Equals(right);
4444
public static bool operator !=(PgTypeId left, PgTypeId right) => !left.Equals(right);
45+
46+
internal bool IsUnspecified => IsOid && _oid == Oid.Unspecified || _dataTypeName == DataTypeName.Unspecified;
4547
}

‎src/Npgsql/NpgsqlParameter.cs‎

Lines changed: 28 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -40,10 +40,10 @@ public class NpgsqlParameter : DbParameter, IDbDataParameter, ICloneable
4040
internal string TrimmedName { get; private protected set; } = PositionalName;
4141
internal const string PositionalName = "";
4242

43-
internal PgTypeInfo? TypeInfo { get; private set; }
43+
private protected PgTypeInfo? TypeInfo { get; private set; }
4444

4545
internal PgTypeId PgTypeId { get; private set; }
46-
internal PgConverter? Converter { get; private set; }
46+
private protected PgConverter? Converter { get; private set; }
4747

4848
internal DataFormat Format { get; private protected set; }
4949
private protected Size? WriteSize { get; set; }
@@ -278,7 +278,7 @@ public override object? Value
278278
get => _value;
279279
set
280280
{
281-
if (value is null || _value?.GetType() != value.GetType())
281+
if (ShouldResetObjectTypeInfo(value))
282282
ResetTypeInfo();
283283
else
284284
ResetBindingInfo();
@@ -497,6 +497,17 @@ public sealed override string SourceColumn
497497

498498
Type? GetValueType(Type staticValueType) => staticValueType != typeof(object) ? staticValueType : Value?.GetType();
499499

500+
internal bool ShouldResetObjectTypeInfo(object? value)
501+
{
502+
var currentType = TypeInfo?.Type;
503+
if (currentType is null || value is null)
504+
return false;
505+
506+
var valueType = value.GetType();
507+
// We don't want to reset the type info when the value is a DBNull, we're able to write it out with any type info.
508+
return valueType != typeof(DBNull) && currentType != valueType;
509+
}
510+
500511
internal void GetResolutionInfo(out PgTypeInfo? typeInfo, out PgConverter? converter, out PgTypeId pgTypeId)
501512
{
502513
typeInfo = TypeInfo;
@@ -540,6 +551,7 @@ _npgsqlDbType is { } npgsqlDbType
540551
pgTypeId = options.ToCanonicalTypeId(pgType.GetRepresentationalType());
541552
}
542553

554+
var unspecifiedDBNull = false;
543555
var valueType = StaticValueType;
544556
if (valueType == typeof(object))
545557
{
@@ -551,22 +563,31 @@ _npgsqlDbType is { } npgsqlDbType
551563
}
552564

553565
// We treat object typed DBNull values as default info.
566+
// Unless we don't have a pgTypeId either, at which point we'll use an 'unspecified' PgTypeInfo to help us write a NULL.
554567
if (valueType == typeof(DBNull))
555568
{
556-
valueType = null;
557-
pgTypeId ??= options.ToCanonicalTypeId(options.UnknownPgType);
569+
if (pgTypeId is null)
570+
{
571+
unspecifiedDBNull = true;
572+
typeInfo = options.UnspecifiedDBNullTypeInfo;
573+
}
574+
else
575+
valueType = null;
558576
}
559577
}
560578

561-
TypeInfo = typeInfo = AdoSerializerHelpers.GetTypeInfoForWriting(valueType, pgTypeId, options, _npgsqlDbType);
579+
if (!unspecifiedDBNull)
580+
typeInfo = AdoSerializerHelpers.GetTypeInfoForWriting(valueType, pgTypeId, options, _npgsqlDbType);
581+
582+
TypeInfo = typeInfo;
562583
}
563584

564585
// This step isn't part of BindValue because we need to know the PgTypeId beforehand for things like SchemaOnly with null values.
565586
// We never reuse resolutions for resolvers across executions as a mutable value itself may influence the result.
566587
// TODO we could expose a property on a Converter/TypeInfo to indicate whether it's immutable, at that point we can reuse.
567588
if (!previouslyResolved || typeInfo!.IsResolverInfo)
568589
{
569-
ResetBindingInfo(); // No need for ResetConverterResolution as we'll mutate those fields directly afterwards.
590+
ResetBindingInfo();
570591
var resolution = ResolveConverter(typeInfo!);
571592
Converter = resolution.Converter;
572593
PgTypeId = resolution.PgTypeId;
@@ -735,11 +756,6 @@ public override void ResetDbType()
735756
private protected void ResetTypeInfo()
736757
{
737758
TypeInfo = null;
738-
ResetConverterResolution();
739-
}
740-
741-
void ResetConverterResolution()
742-
{
743759
_asObject = false;
744760
Converter = null;
745761
PgTypeId = default;

‎src/Npgsql/NpgsqlParameter`.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ public T? TypedValue
2626
get => _typedValue;
2727
set
2828
{
29-
if (typeof(T) == typeof(object) && (value is null || _typedValue?.GetType() != value.GetType()))
29+
if (typeof(T) == typeof(object) && ShouldResetObjectTypeInfo(value))
3030
ResetTypeInfo();
3131
else
3232
ResetBindingInfo();

‎test/Npgsql.Tests/NpgsqlParameterTests.cs‎

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
using System.Data;
55
using System.Data.Common;
66
using System.Threading.Tasks;
7+
using Npgsql.Internal.Postgres;
78

89
namespace Npgsql.Tests;
910

@@ -693,6 +694,71 @@ public void Null_value_with_nullable_type()
693694
Assert.That(reader.GetFieldValue<int?>(0), Is.Null);
694695
}
695696

697+
[Test]
698+
public void DBNull_reuses_type_info([Values]bool generic)
699+
{
700+
// Bootstrap datasource.
701+
using (var _ = OpenConnection()) {}
702+
703+
var param = generic ? new NpgsqlParameter<object> { Value = "value" } : new NpgsqlParameter { Value = "value" };
704+
param.ResolveTypeInfo(DataSource.SerializerOptions);
705+
param.GetResolutionInfo(out var typeInfo, out _, out _);
706+
Assert.That(typeInfo, Is.Not.Null);
707+
708+
// Make sure we don't reset the type info when setting DBNull.
709+
param.Value = DBNull.Value;
710+
param.GetResolutionInfo(out var secondTypeInfo, out _, out _);
711+
Assert.That(typeInfo, Is.SameAs(secondTypeInfo));
712+
713+
// Make sure we don't resolve a different type info either.
714+
param.ResolveTypeInfo(DataSource.SerializerOptions);
715+
param.GetResolutionInfo(out var thirdTypeInfo, out _, out _);
716+
Assert.That(secondTypeInfo, Is.SameAs(thirdTypeInfo));
717+
}
718+
719+
[Test]
720+
public void DBNull_followed_by_non_null_reresolves([Values]bool generic)
721+
{
722+
// Bootstrap datasource.
723+
using (var _ = OpenConnection()) {}
724+
725+
var param = generic ? new NpgsqlParameter<object> { Value = DBNull.Value } : new NpgsqlParameter { Value = DBNull.Value };
726+
param.ResolveTypeInfo(DataSource.SerializerOptions);
727+
param.GetResolutionInfo(out var typeInfo, out _, out var pgTypeId);
728+
Assert.That(typeInfo, Is.Not.Null);
729+
Assert.That(pgTypeId.IsUnspecified, Is.True);
730+
731+
param.Value = "value";
732+
param.GetResolutionInfo(out var secondTypeInfo, out _, out _);
733+
Assert.That(secondTypeInfo, Is.Null);
734+
735+
// Make sure we don't resolve the same type info either.
736+
param.ResolveTypeInfo(DataSource.SerializerOptions);
737+
param.GetResolutionInfo(out var thirdTypeInfo, out _, out _);
738+
Assert.That(typeInfo, Is.Not.SameAs(thirdTypeInfo));
739+
}
740+
741+
[Test]
742+
public void Changing_value_type_reresolves([Values]bool generic)
743+
{
744+
// Bootstrap datasource.
745+
using (var _ = OpenConnection()) {}
746+
747+
var param = generic ? new NpgsqlParameter<object> { Value = "value" } : new NpgsqlParameter { Value = "value" };
748+
param.ResolveTypeInfo(DataSource.SerializerOptions);
749+
param.GetResolutionInfo(out var typeInfo, out _, out _);
750+
Assert.That(typeInfo, Is.Not.Null);
751+
752+
param.Value = 1;
753+
param.GetResolutionInfo(out var secondTypeInfo, out _, out _);
754+
Assert.That(secondTypeInfo, Is.Null);
755+
756+
// Make sure we don't resolve a different type info either.
757+
param.ResolveTypeInfo(DataSource.SerializerOptions);
758+
param.GetResolutionInfo(out var thirdTypeInfo, out _, out _);
759+
Assert.That(typeInfo, Is.Not.SameAs(thirdTypeInfo));
760+
}
761+
696762
#if NeedsPorting
697763
[Test]
698764
[Category ("NotWorking")]

0 commit comments

Comments
 (0)