Skip to content

Remove multipass resolution #5559

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.

Sign up for GitHub

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

Merged
NinoFloris merged 5 commits into from
Nov 10, 2024
Merged

Remove multipass resolution #5559

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
896ed68
Remove remaining dependencies on multipass resolution
NinoFloris Feb 1, 2024
f4c2243
Remove feature from cache
NinoFloris Feb 1, 2024
4419134
Also remove negative match caching
NinoFloris Feb 1, 2024
f9ad280
Improve boxing info type compatibility check
NinoFloris Feb 1, 2024
fedfede
Add comment
NinoFloris Nov 10, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 18 additions & 18 deletions src/Npgsql/BackendMessages/RowDescriptionMessage.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ internal FieldDescription(FieldDescription source)
DataFormat = source.DataFormat;
PostgresType = source.PostgresType;
Field = source.Field;
_objectOrDefaultInfo = source._objectOrDefaultInfo;
_objectInfo = source._objectInfo;
}

internal void Populate(
Expand All @@ -250,7 +250,7 @@ internal void Populate(
DataFormat = dataFormat;
PostgresType = _serializerOptions.DatabaseInfo.FindPostgresType((Oid)TypeOID)?.GetRepresentationalType() ?? UnknownBackendType.Instance;
Field = new(Name, _serializerOptions.ToCanonicalTypeId(PostgresType), TypeModifier);
_objectOrDefaultInfo = default;
_objectInfo = default;
}

/// <summary>
Expand Down Expand Up @@ -296,18 +296,18 @@ internal void Populate(

internal PostgresType PostgresType { get; private set; }

internal Type FieldType => ObjectOrDefaultInfo.TypeToConvert;
internal Type FieldType => ObjectInfo.TypeToConvert;

ColumnInfo _objectOrDefaultInfo;
internal PgConverterInfo ObjectOrDefaultInfo
ColumnInfo _objectInfo;
internal PgConverterInfo ObjectInfo
{
get
{
if (!_objectOrDefaultInfo.ConverterInfo.IsDefault)
return _objectOrDefaultInfo.ConverterInfo;
if (!_objectInfo.ConverterInfo.IsDefault)
return _objectInfo.ConverterInfo;

ref var info = ref _objectOrDefaultInfo;
GetInfo(null, ref _objectOrDefaultInfo);
ref var info = ref _objectInfo;
GetInfoCore(null, ref _objectInfo);
return info.ConverterInfo;
}
}
Expand All @@ -320,7 +320,8 @@ internal FieldDescription Clone()
return field;
}

internal void GetInfo(Type? type, ref ColumnInfo lastColumnInfo)
internal void GetInfo(Type type, ref ColumnInfo lastColumnInfo) => GetInfoCore(type, ref lastColumnInfo);
void GetInfoCore(Type? type, ref ColumnInfo lastColumnInfo)
{
Debug.Assert(lastColumnInfo.ConverterInfo.IsDefault || (
ReferenceEquals(_serializerOptions, lastColumnInfo.ConverterInfo.TypeInfo.Options) && (
Expand All @@ -332,20 +333,20 @@ internal void GetInfo(Type? type, ref ColumnInfo lastColumnInfo)
if (!lastColumnInfo.ConverterInfo.IsDefault && lastColumnInfo.ConverterInfo.TypeToConvert == type)
return;

var odfInfo = DataFormat is DataFormat.Text && type is not null ? ObjectOrDefaultInfo : _objectOrDefaultInfo.ConverterInfo;
if (odfInfo is { IsDefault: false })
var objectInfo = DataFormat is DataFormat.Text && type is not null ? ObjectInfo : _objectInfo.ConverterInfo;
if (objectInfo is { IsDefault: false })
{
if (typeof(object) == type)
{
lastColumnInfo = new(odfInfo, DataFormat, true);
lastColumnInfo = new(objectInfo, DataFormat, true);
return;
}
if (odfInfo.TypeToConvert == type)
if (objectInfo.TypeToConvert == type)
{
// As TypeInfoMappingCollection is always adding object mappings for
// default/datatypename mappings, we'll also check Converter.TypeToConvert.
// If we have an exact match we are still able to use e.g. a converter for ints in an unboxed fashion.
lastColumnInfo = new(odfInfo, DataFormat, odfInfo.IsBoxingConverter && odfInfo.Converter.TypeToConvert != type);
lastColumnInfo = new(objectInfo, DataFormat, objectInfo.IsBoxingConverter && objectInfo.Converter.TypeToConvert != type);
return;
}
}
Expand Down Expand Up @@ -390,9 +391,8 @@ void GetInfoSlow(Type? type, out ColumnInfo lastColumnInfo)

// We delay initializing ObjectOrDefaultInfo until after the first lookup (unless it is itself the first lookup).
// When passed in an unsupported type it allows the error to be more specific, instead of just having object/null to deal with.
if (_objectOrDefaultInfo.ConverterInfo.IsDefault && type is not null)
_ = ObjectOrDefaultInfo;

if (_objectInfo.ConverterInfo.IsDefault && type is not null)
_ = ObjectInfo;
}

// DataFormat.Text today exclusively signals that we executed with an UnknownResultTypeList.
Expand Down
2 changes: 1 addition & 1 deletion src/Npgsql/Internal/AdoSerializerHelpers.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ public static PgTypeInfo GetTypeInfoForReading(Type type, PgTypeId pgTypeId, PgS
Exception? inner = null;
try
{
typeInfo = type == typeof(object) ? options.GetObjectOrDefaultTypeInfoInternal(pgTypeId) : options.GetTypeInfoInternal(type, pgTypeId);
typeInfo = options.GetTypeInfoInternal(type, pgTypeId);
}
catch (Exception ex)
{
Expand Down
8 changes: 6 additions & 2 deletions src/Npgsql/Internal/Converters/RecordConverter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ namespace Npgsql.Internal.Converters;

sealed class RecordConverter<T>(PgSerializerOptions options, Func<object[], T>? factory = null) : PgStreamingConverter<T>
{
static bool IsObjectArrayRecord => typeof(T) == typeof(object[]);

public override T Read(PgReader reader)
=> Read(async: false, reader, CancellationToken.None).GetAwaiter().GetResult();

Expand Down Expand Up @@ -34,9 +36,11 @@ async ValueTask<T> Read(bool async, PgReader reader, CancellationToken cancellat
var postgresType =
options.DatabaseInfo.GetPostgresType(typeOid).GetRepresentationalType()
?? throw new NotSupportedException($"Reading isn't supported for record field {i} (unknown type OID {typeOid}");

var pgTypeId = options.ToCanonicalTypeId(postgresType);
var typeInfo = options.GetObjectOrDefaultTypeInfoInternal(pgTypeId)

// TODO resolve based on types expected by _factory (pass in a Type[] during construcion)
// Only allow object polymorphism for object[] records, valuetuple records are always strongly typed.
var typeInfo = (IsObjectArrayRecord ? options.GetTypeInfo(typeof(object), pgTypeId) : options.GetDefaultTypeInfo(pgTypeId))
?? throw new NotSupportedException(
$"Reading isn't supported for record field {i} (PG type '{postgresType.DisplayName}'");

Expand Down
17 changes: 7 additions & 10 deletions src/Npgsql/Internal/PgSerializerOptions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,25 +78,22 @@ public static bool IsWellKnownTextType(Type type)
// This also makes it easier to realize it should be a cached value if infos for different CLR types are requested for the same
// pgTypeId. Effectively it should be 'impossible' to get the wrong kind via any PgConverterOptions api which is what this is mainly
// for.
PgTypeInfo? GetTypeInfoCore(Type? type, PgTypeId? pgTypeId, bool defaultTypeFallback)
PgTypeInfo? GetTypeInfoCore(Type? type, PgTypeId? pgTypeId)
=> PortableTypeIds
? ((TypeInfoCache<DataTypeName>)(_typeInfoCache ??= new TypeInfoCache<DataTypeName>(this))).GetOrAddInfo(type, pgTypeId?.DataTypeName, defaultTypeFallback)
: ((TypeInfoCache<Oid>)(_typeInfoCache ??= new TypeInfoCache<Oid>(this))).GetOrAddInfo(type, pgTypeId?.Oid, defaultTypeFallback);
? ((TypeInfoCache<DataTypeName>)(_typeInfoCache ??= new TypeInfoCache<DataTypeName>(this))).GetOrAddInfo(type, pgTypeId?.DataTypeName)
: ((TypeInfoCache<Oid>)(_typeInfoCache ??= new TypeInfoCache<Oid>(this))).GetOrAddInfo(type, pgTypeId?.Oid);

internal PgTypeInfo? GetTypeInfoInternal(Type? type, PgTypeId? pgTypeId)
=> GetTypeInfoCore(type, pgTypeId, false);

internal PgTypeInfo? GetObjectOrDefaultTypeInfoInternal(PgTypeId pgTypeId)
=> GetTypeInfoCore(typeof(object), pgTypeId, true);
=> GetTypeInfoCore(type, pgTypeId);

public PgTypeInfo? GetDefaultTypeInfo(Type type)
=> GetTypeInfoCore(type, null, false);
=> GetTypeInfoCore(type, null);

public PgTypeInfo? GetDefaultTypeInfo(PgTypeId pgTypeId)
=> GetTypeInfoCore(null, GetCanonicalTypeId(pgTypeId), false);
=> GetTypeInfoCore(null, GetCanonicalTypeId(pgTypeId));

public PgTypeInfo? GetTypeInfo(Type type, PgTypeId pgTypeId)
=> GetTypeInfoCore(type, GetCanonicalTypeId(pgTypeId), false);
=> GetTypeInfoCore(type, GetCanonicalTypeId(pgTypeId));

// If a given type id is in the opposite form than what was expected it will be mapped according to the requirement.
internal PgTypeId GetCanonicalTypeId(PgTypeId pgTypeId)
Expand Down
12 changes: 6 additions & 6 deletions src/Npgsql/Internal/ResolverFactories/AdoTypeInfoResolverFactory.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,13 +38,12 @@ class Resolver : IPgTypeInfoResolver

static PgTypeInfo? GetEnumTypeInfo(Type? type, DataTypeName dataTypeName, PgSerializerOptions options)
{
if (type is not null && type != typeof(string))
if (type is not null && type != typeof(object) && type != typeof(string)
|| options.DatabaseInfo.GetPostgresType(dataTypeName) is not PostgresEnumType)
return null;

if (options.DatabaseInfo.GetPostgresType(dataTypeName) is not PostgresEnumType)
return null;

return new PgTypeInfo(options, new StringTextConverter(options.TextEncoding), dataTypeName);
return new PgTypeInfo(options, new StringTextConverter(options.TextEncoding), dataTypeName,
unboxedType: type == typeof(object) ? typeof(string) : null);
}

static TypeInfoMappingCollection AddMappings(TypeInfoMappingCollection mappings)
Expand Down Expand Up @@ -511,7 +510,8 @@ static TypeInfoMappingCollection AddMappings(TypeInfoMappingCollection mappings)

static PgTypeInfo? GetEnumArrayTypeInfo(Type? elementType, PostgresType pgElementType, Type? type, DataTypeName dataTypeName, PgSerializerOptions options)
{
if ((type != typeof(object) && elementType is not null && elementType != typeof(string)) || pgElementType is not PostgresEnumType enumType)
if ((type is not null && type != typeof(object) && elementType != typeof(string))
|| pgElementType is not PostgresEnumType enumType)
return null;

var mappings = new TypeInfoMappingCollection();
Expand Down
66 changes: 24 additions & 42 deletions src/Npgsql/Internal/TypeInfoCache.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
using System;
using System.Collections.Concurrent;
using System.Runtime.CompilerServices;
using Npgsql.Internal.Postgres;

namespace Npgsql.Internal;
Expand All @@ -13,7 +12,7 @@ sealed class TypeInfoCache<TPgTypeId>(PgSerializerOptions options, bool validate

// Used for reading, occasionally for parameter writing where a db type was given.
// 8ns, about 10ns total to scan an array with 6, 7 different clr types under one pg type
readonly ConcurrentDictionary<TPgTypeId, (Type? Type, PgTypeInfo? Info)[]> _cacheByPgTypeId = new();
readonly ConcurrentDictionary<TPgTypeId, (Type? Type, PgTypeInfo Info)[]> _cacheByPgTypeId = new();

static TypeInfoCache()
{
Expand All @@ -26,55 +25,40 @@ static TypeInfoCache()
/// </summary>
/// <param name="type"></param>
/// <param name="pgTypeId"></param>
/// <param name="defaultTypeFallback">
/// When this flag is true, and both type and pgTypeId are non null, a default info for the pgTypeId can be returned if an exact match
/// can't be found.
/// </param>
/// <returns></returns>
/// <exception cref="InvalidOperationException"></exception>
public PgTypeInfo? GetOrAddInfo(Type? type, TPgTypeId? pgTypeId, bool defaultTypeFallback = false)
public PgTypeInfo? GetOrAddInfo(Type? type, TPgTypeId? pgTypeId)
{
if (pgTypeId is { } id)
{
if (_cacheByPgTypeId.TryGetValue(id, out var infos))
if (FindMatch(type, infos, defaultTypeFallback) is { } info)
if (FindMatch(type, infos) is { } info)
return info;

return AddEntryById(type, id, infos, defaultTypeFallback);
return AddEntryById(type, id, infos);
}

if (type is not null)
return _cacheByClrType.TryGetValue(type, out var info) ? info : AddByType(type);

return null;

PgTypeInfo? FindMatch(Type? type, (Type? Type, PgTypeInfo? Info)[] infos, bool defaultTypeFallback)
PgTypeInfo? FindMatch(Type? type, (Type? Type, PgTypeInfo Info)[] infos)
{
PgTypeInfo? defaultInfo = null;
var negativeExactMatch = false;
for (var i = 0; i < infos.Length; i++)
{
ref var item = ref infos[i];
if (item.Type == type)
{
if (item.Info is not null || !defaultTypeFallback)
return item.Info;
negativeExactMatch = true;
}

if (defaultTypeFallback && item.Type is null)
defaultInfo = item.Info;
return item.Info;
}

// We can only return default info if we've seen a negative match (type: typeof(object), info: null)
// Otherwise we might return a previously requested default while the resolvers could produce the exact match.
return negativeExactMatch ? defaultInfo : null;
return null;
}

PgTypeInfo? AddByType(Type type)
{
// We don't pass PgTypeId as we're interested in default converters here.
var info = CreateInfo(type, null, options, defaultTypeFallback: false, validatePgTypeIds);
var info = CreateInfo(type, null, options, validatePgTypeIds);

return info is null
? null
Expand All @@ -83,18 +67,18 @@ static TypeInfoCache()
: _cacheByClrType[type];
}

PgTypeInfo? AddEntryById(Type? type, TPgTypeId pgTypeId, (Type? Type, PgTypeInfo? Info)[]? infos, bool defaultTypeFallback)
PgTypeInfo? AddEntryById(Type? type, TPgTypeId pgTypeId, (Type? Type, PgTypeInfo Info)[]? infos)
{
// We cache negatives (null info) to allow 'object or default' checks to never hit the resolvers after the first lookup.
var info = CreateInfo(type, pgTypeId, options, defaultTypeFallback, validatePgTypeIds);
if (CreateInfo(type, pgTypeId, options, validatePgTypeIds) is not { } info)
return null;

var isDefaultInfo = type is null && info is not null;
var isDefaultInfo = type is null;
if (infos is null)
{
// Also add defaults by their info type to save a future resolver lookup + resize.
infos = isDefaultInfo
? new [] { (type, info), (info!.Type, info) }
: [(type, info)];
? new [] { (type, info), (info.Type, info) }
: new [] { (type, info) };

if (_cacheByPgTypeId.TryAdd(pgTypeId, infos))
return info;
Expand All @@ -104,7 +88,7 @@ static TypeInfoCache()
while (true)
{
infos = _cacheByPgTypeId[pgTypeId];
if (FindMatch(type, infos, defaultTypeFallback) is { } racedInfo)
if (FindMatch(type, infos) is { } racedInfo)
return racedInfo;

// Also add defaults by their info type to save a future resolver lookup + resize.
Expand All @@ -113,39 +97,37 @@ static TypeInfoCache()
if (isDefaultInfo)
{
foreach (var oldInfo in oldInfos)
if (oldInfo.Type == info!.Type)
if (oldInfo.Type == info.Type)
hasExactType = true;
}
Array.Resize(ref infos, oldInfos.Length + (isDefaultInfo && !hasExactType ? 2 : 1));
infos[oldInfos.Length] = (type, info);
if (isDefaultInfo && !hasExactType)
infos[oldInfos.Length + 1] = (info!.Type, info);
infos[oldInfos.Length + 1] = (info.Type, info);

if (_cacheByPgTypeId.TryUpdate(pgTypeId, infos, oldInfos))
return info;
}
}

static PgTypeInfo? CreateInfo(Type? type, TPgTypeId? typeId, PgSerializerOptions options, bool defaultTypeFallback, bool validatePgTypeIds)
static PgTypeInfo? CreateInfo(Type? type, TPgTypeId? typeId, PgSerializerOptions options, bool validatePgTypeIds)
{
var pgTypeId = AsPgTypeId(typeId);
// Validate that we only pass data types that are supported by the backend.
var dataTypeName = pgTypeId is { } id ? (DataTypeName?)options.DatabaseInfo.GetDataTypeName(id, validate: validatePgTypeIds) : null;
var info = options.TypeInfoResolver.GetTypeInfo(type, dataTypeName, options);
if (info is null && defaultTypeFallback)
{
type = null;
info = options.TypeInfoResolver.GetTypeInfo(type, dataTypeName, options);
}

if (info is null)
return null;

if (pgTypeId is not null && info.PgTypeId != pgTypeId)
throw new InvalidOperationException("A Postgres type was passed but the resolved PgTypeInfo does not have an equal PgTypeId.");

if (type is not null && !info.IsBoxing && info.Type != type)
throw new InvalidOperationException($"A CLR type '{type}' was passed but the resolved PgTypeInfo does not have an equal Type: {info.Type}.");
if (type is not null && info.Type != type)
{
// Types were not equal, throw for IsBoxing = false, otherwise we throw when the returned type isn't assignable to the requested type (after unboxing).
if (!info.IsBoxing || !info.Type.IsAssignableTo(type))
throw new InvalidOperationException($"A CLR type '{type}' was passed but the resolved PgTypeInfo does not have an equal Type: {info.Type}.");
}

return info;
}
Expand Down
4 changes: 2 additions & 2 deletions src/Npgsql/NpgsqlDataReader.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2131,8 +2131,8 @@ DataFormat GetDefaultInfo(int ordinal, out PgConverter converter, out Size buffe
{
var field = RowDescription![ordinal];

converter = field.ObjectOrDefaultInfo.Converter;
bufferRequirement = field.ObjectOrDefaultInfo.BufferRequirement;
converter = field.ObjectInfo.Converter;
bufferRequirement = field.ObjectInfo.BufferRequirement;
return field.DataFormat;
}

Expand Down
2 changes: 1 addition & 1 deletion src/Npgsql/Schema/DbColumnSchemaGenerator.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,7 @@ void ColumnPostConfig(NpgsqlDbColumn column, int typeModifier)
var serializerOptions = _connection.Connector!.SerializerOptions;

column.NpgsqlDbType = column.PostgresType.DataTypeName.ToNpgsqlDbType();
if (serializerOptions.GetObjectOrDefaultTypeInfoInternal(serializerOptions.ToCanonicalTypeId(column.PostgresType)) is { } typeInfo)
if (serializerOptions.GetDefaultTypeInfo(serializerOptions.ToCanonicalTypeId(column.PostgresType)) is { } typeInfo)
{
column.DataType = typeInfo.Type;
column.IsLong = column.PostgresType.DataTypeName == DataTypeNames.Bytea;
Expand Down