Skip to content

Fixes for arrays over nullable DateTime #4341

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

Closed
roji wants to merge 1 commit into from
Closed

Fixes for arrays over nullable DateTime #4341

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
18 changes: 14 additions & 4 deletions src/Npgsql/TypeMapping/BuiltInTypeHandlerResolver.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Collections.Specialized;
using System.Data;
using System.Linq;
using System.Net;
using System.Net.NetworkInformation;
using System.Numerics;
Expand Down Expand Up @@ -396,9 +396,7 @@ static BuiltInTypeHandlerResolver()
{ typeof(JsonDocument), "jsonb" },

// Date/time types
// The DateTime entry is for LegacyTimestampBehavior mode only. In regular mode we resolve through
// ResolveValueDependentValue below
{ typeof(DateTime), "timestamp without time zone" },
// DateTime is added below, only for LegacyTimestampBehavior
{ typeof(DateTimeOffset), "timestamp with time zone" },
#if NET6_0_OR_GREATER
{ typeof(DateOnly), "date" },
Expand Down Expand Up @@ -498,6 +496,12 @@ static BuiltInTypeHandlerResolver()
// For arrays/lists, return timestamp or timestamptz based on the kind of the first DateTime; if the user attempts to
// mix incompatible Kinds, that will fail during validation. For empty arrays it doesn't matter.
IList<DateTime> array => ArrayHandler(array.Count == 0 ? DateTimeKind.Unspecified : array[0].Kind),
IList<DateTime?> array => ArrayHandler(
array.Count == 0
? DateTimeKind.Unspecified
: array.FirstOrDefault(d => d is not null) is { } dt
? dt.Kind
: DateTimeKind.Unspecified),

NpgsqlRange<DateTime> range => RangeHandler(!range.LowerBoundInfinite ? range.LowerBound.Kind :
!range.UpperBoundInfinite ? range.UpperBound.Kind : DateTimeKind.Unspecified),
Expand Down Expand Up @@ -558,6 +562,12 @@ static DateTimeKind GetMultirangeKind(IList<NpgsqlRange<DateTime>> multirange)
? "timestamp without time zone[]"
: array[0].Kind == DateTimeKind.Utc ? "timestamp with time zone[]" : "timestamp without time zone[]",

IList<DateTime?> array => array.Count == 0
? "timestamp without time zone[]"
: array.FirstOrDefault(d => d is not null) is { } dt
? dt.Kind == DateTimeKind.Utc ? "timestamp with time zone[]" : "timestamp without time zone[]"
: "timestamp without time zone[]",

NpgsqlRange<DateTime> range => GetRangeKind(range) == DateTimeKind.Utc ? "tstzrange" : "tsrange",

NpgsqlRange<DateTime>[] multirange => GetMultirangeKind(multirange) == DateTimeKind.Utc ? "tstzmultirange" : "tsmultirange",
Expand Down
8 changes: 3 additions & 5 deletions src/Npgsql/TypeMapping/ConnectorTypeMapper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
using Npgsql.Internal.TypeHandling;
using Npgsql.Internal.TypeMapping;
using Npgsql.PostgresTypes;
using Npgsql.Util;
using NpgsqlTypes;

namespace Npgsql.TypeMapping;
Expand Down Expand Up @@ -396,19 +397,16 @@ internal NpgsqlTypeHandler ResolveByClrType(Type type)
{
var typeInfo = type.GetTypeInfo();
if (typeInfo.IsArray)
return GetUnderlyingType(type.GetElementType()!); // The use of bang operator is justified here as Type.GetElementType() only returns null for the Array base class which can't be mapped in a useful way.
return type.GetElementType()!.UnwrapNullable(); // The use of bang operator is justified here as Type.GetElementType() only returns null for the Array base class which can't be mapped in a useful way.

var ilist = typeInfo.ImplementedInterfaces.FirstOrDefault(x => x.GetTypeInfo().IsGenericType && x.GetGenericTypeDefinition() == typeof(IList<>));
if (ilist != null)
return GetUnderlyingType(ilist.GetGenericArguments()[0]);
return ilist.GetGenericArguments()[0].UnwrapNullable();

if (typeof(IList).IsAssignableFrom(type))
throw new NotSupportedException("Non-generic IList is a supported parameter, but the NpgsqlDbType parameter must be set on the parameter");

return null;

Type GetUnderlyingType(Type t)
=> Nullable.GetUnderlyingType(t) ?? t;
}
}

Expand Down
11 changes: 6 additions & 5 deletions src/Npgsql/TypeMapping/GlobalTypeMapper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
using Npgsql.Internal.TypeHandling;
using Npgsql.Internal.TypeMapping;
using Npgsql.NameTranslation;
using Npgsql.Util;
using NpgsqlTypes;
using static Npgsql.Util.Statics;

Expand Down Expand Up @@ -214,15 +215,15 @@ internal bool TryResolveMappingByValue(object value, [NotNullWhen(true)] out Typ
// over DateTime), and the results cannot be cached.
// 3. Uncached by-type lookup (for the very first resolution of a given type)

var type = value.GetType();
var type = value.GetType().UnwrapNullable();
if (_mappingsByClrType.TryGetValue(type, out typeMapping))
return true;

foreach (var resolverFactory in ResolverFactories)
if ((typeMapping = resolverFactory.GetMappingByValueDependentValue(value)) is not null)
return true;

return TryResolveMappingByClrType(value.GetType(), out typeMapping);
return TryResolveMappingByClrType(type, out typeMapping);
}
finally
{
Expand All @@ -245,7 +246,7 @@ bool TryResolveMappingByClrType(Type clrType, [NotNullWhen(true)] out TypeMappin

if (clrType.IsArray)
{
if (TryResolveMappingByClrType(clrType.GetElementType()!, out var elementMapping))
if (TryResolveMappingByClrType(clrType.GetElementType()!.UnwrapNullable(), out var elementMapping))
{
_mappingsByClrType[clrType] = typeMapping = new(
NpgsqlDbType.Array | elementMapping.NpgsqlDbType,
Expand All @@ -263,7 +264,7 @@ bool TryResolveMappingByClrType(Type clrType, [NotNullWhen(true)] out TypeMappin
x.GetTypeInfo().IsGenericType && x.GetGenericTypeDefinition() == typeof(IList<>));
if (ilist != null)
{
if (TryResolveMappingByClrType(ilist.GetGenericArguments()[0], out var elementMapping))
if (TryResolveMappingByClrType(ilist.GetGenericArguments()[0].UnwrapNullable(), out var elementMapping))
{
_mappingsByClrType[clrType] = typeMapping = new(
NpgsqlDbType.Array | elementMapping.NpgsqlDbType,
Expand All @@ -277,7 +278,7 @@ bool TryResolveMappingByClrType(Type clrType, [NotNullWhen(true)] out TypeMappin

if (typeInfo.IsGenericType && clrType.GetGenericTypeDefinition() == typeof(NpgsqlRange<>))
{
if (TryResolveMappingByClrType(clrType.GetGenericArguments()[0], out var elementMapping))
if (TryResolveMappingByClrType(clrType.GetGenericArguments()[0].UnwrapNullable(), out var elementMapping))
{
_mappingsByClrType[clrType] = typeMapping = new(
NpgsqlDbType.Range | elementMapping.NpgsqlDbType,
Expand Down
6 changes: 6 additions & 0 deletions src/Npgsql/Util/PGUtil.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,12 @@ internal static string Join(this IEnumerable<string> values, string separator)
}
}

static class TypeExtensions
{
public static Type UnwrapNullable(this Type type)
=> Nullable.GetUnderlyingType(type) ?? type;
}

static class ExceptionExtensions
{
internal static Exception UnwrapAggregate(this Exception exception)
Expand Down
14 changes: 13 additions & 1 deletion test/Npgsql.Tests/Types/DateTimeTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -310,6 +310,19 @@ await AssertType(
NpgsqlDbType.TimestampTzMultirange);
}

[Test]
public async Task Array_of_nullable_timestamptz()
=> await AssertType(
new DateTime?[]
{
new DateTime(1998, 4, 12, 13, 26, 38, DateTimeKind.Utc),
null
},
@"{""1998-04-12 15:26:38+02"",NULL}",
"timestamp with time zone[]",
NpgsqlDbType.TimestampTz | NpgsqlDbType.Array,
isDefaultForReading: false);

[Test]
public Task Cannot_mix_DateTime_Kinds_in_array()
=> AssertTypeUnsupportedWrite<DateTime[], Exception>(new[]
Expand All @@ -318,7 +331,6 @@ public Task Cannot_mix_DateTime_Kinds_in_array()
new DateTime(1998, 4, 12, 13, 26, 38, DateTimeKind.Local),
});


[Test]
public Task Cannot_mix_DateTime_Kinds_in_range()
=> AssertTypeUnsupportedWrite(new NpgsqlRange<DateTime>(
Expand Down