Skip to content

Commit c97e004

Browse files
rojiroji
committed
[MEVD] Map DateTime to timestamptz on PostgreSQL
Closes #10641
1 parent b0d621f commit c97e004

11 files changed

Lines changed: 489 additions & 35 deletions

‎dotnet/src/VectorData/PgVector/PostgresCollection.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -375,7 +375,7 @@ public override async Task DeleteAsync(IEnumerable<TKey> keys, CancellationToken
375375

376376
using var connection = await this._dataSource.OpenConnectionAsync(cancellationToken).ConfigureAwait(false);
377377
using var command = connection.CreateCommand();
378-
PostgresSqlBuilder.BuildDeleteBatchCommand(command, this._schema, this.Name, this._model.KeyProperty.StorageName, listOfKeys);
378+
PostgresSqlBuilder.BuildDeleteBatchCommand(command, this._schema, this.Name, this._model.KeyProperty, listOfKeys);
379379

380380
await this.RunOperationAsync("DeleteBatch", () => command.ExecuteNonQueryAsync(cancellationToken)).ConfigureAwait(false);
381381
}

‎dotnet/src/VectorData/PgVector/PostgresFilterTranslator.cs‎

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
using System;
44
using System.Collections;
55
using System.Collections.Generic;
6+
using System.Diagnostics;
67
using System.Globalization;
78
using System.Linq.Expressions;
89
using System.Text;
@@ -29,18 +30,29 @@ protected override void TranslateConstant(object? value, bool isSearchCondition)
2930
{
3031
switch (value)
3132
{
32-
// TODO: This aligns with our mapping of DateTime to PG's timestamp (as opposed to timestamptz) - we probably want to
33-
// change that to timestamptz (aligning with Npgsql and EF). See #10641.
3433
case DateTime dateTime:
35-
this._sql.Append('\'').Append(dateTime.ToString("yyyy-MM-ddTHH:mm:ss.FFFFFF", CultureInfo.InvariantCulture)).Append('\'');
36-
return;
34+
switch (dateTime.Kind)
35+
{
36+
case DateTimeKind.Utc:
37+
this._sql.Append('\'').Append(dateTime.ToString("yyyy-MM-ddTHH:mm:ss.FFFFFFZ", CultureInfo.InvariantCulture)).Append('\'');
38+
return;
39+
40+
case DateTimeKind.Unspecified:
41+
case DateTimeKind.Local:
42+
this._sql.Append('\'').Append(dateTime.ToString("yyyy-MM-ddTHH:mm:ss.FFFFFF", CultureInfo.InvariantCulture)).Append('\'');
43+
return;
44+
45+
default:
46+
throw new UnreachableException();
47+
}
48+
3749
case DateTimeOffset dateTimeOffset:
3850
if (dateTimeOffset.Offset != TimeSpan.Zero)
3951
{
40-
throw new NotSupportedException("DateTimeOffset with non-zero offset is not supported with PostgreSQL");
52+
throw new NotSupportedException("DateTimeOffset with non-zero offset is not supported with PostgreSQL. Use DateTimeOffset.UtcNow or convert to UTC.");
4153
}
4254

43-
this._sql.Append('\'').Append(dateTimeOffset.ToString("yyyy-MM-ddTHH:mm:ss.FFFFFF", CultureInfo.InvariantCulture)).Append("Z'");
55+
this._sql.Append('\'').Append(dateTimeOffset.ToString("yyyy-MM-ddTHH:mm:ss.FFFFFFZ", CultureInfo.InvariantCulture)).Append('\'');
4456
return;
4557

4658
// Array constants (ARRAY[1, 2, 3])

‎dotnet/src/VectorData/PgVector/PostgresModelBuilder.cs‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
using System.Collections.Generic;
66
using System.Diagnostics.CodeAnalysis;
77
using Microsoft.Extensions.AI;
8+
using Microsoft.Extensions.VectorData;
89
using Microsoft.Extensions.VectorData.ProviderServices;
910
using Pgvector;
1011

@@ -92,6 +93,22 @@ internal static bool IsVectorPropertyTypeValidCore(Type type, [NotNullWhen(false
9293
type == typeof(SparseVector);
9394
}
9495

96+
/// <inheritdoc />
97+
protected override void ValidateProperty(PropertyModel propertyModel, VectorStoreCollectionDefinition? definition)
98+
{
99+
base.ValidateProperty(propertyModel, definition);
100+
101+
if (propertyModel.IsTimestampWithoutTimezone())
102+
{
103+
var type = Nullable.GetUnderlyingType(propertyModel.Type) ?? propertyModel.Type;
104+
if (type != typeof(DateTime))
105+
{
106+
throw new NotSupportedException(
107+
$"Property '{propertyModel.ModelName}' has store type 'timestamp' configured, but this is only supported for DateTime properties. The property type is '{propertyModel.Type.Name}'.");
108+
}
109+
}
110+
}
111+
95112
/// <inheritdoc />
96113
protected override Type? ResolveEmbeddingType(
97114
VectorPropertyModel vectorProperty,

‎dotnet/src/VectorData/PgVector/PostgresPropertyExtensions.cs‎

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
// Copyright (c) Microsoft. All rights reserved.
22

3+
using System;
34
using Microsoft.Extensions.VectorData;
45
using Microsoft.Extensions.VectorData.ProviderServices;
56

@@ -11,6 +12,9 @@ namespace Microsoft.SemanticKernel.Connectors.PgVector;
1112
public static class PostgresPropertyExtensions
1213
{
1314
private const string FullTextSearchLanguageKey = "Postgres:FullTextSearchLanguage";
15+
private const string StoreTypeKey = "Postgres:StoreType";
16+
17+
#region Full-text search lanaguage
1418

1519
/// <summary>
1620
/// Sets the PostgreSQL full-text search language for a data property.
@@ -50,4 +54,69 @@ internal static string GetFullTextSearchLanguageOrDefault(this DataPropertyModel
5054
=> property.ProviderAnnotations?.TryGetValue(FullTextSearchLanguageKey, out var value) == true && value is string language
5155
? language
5256
: PostgresConstants.DefaultFullTextSearchLanguage;
57+
58+
#endregion Full-text search lanaguage
59+
60+
#region Store type
61+
62+
/// <summary>
63+
/// Sets the PostgreSQL store type for a property, overriding the default type mapping.
64+
/// </summary>
65+
/// <param name="property">The property to configure.</param>
66+
/// <param name="storeType">
67+
/// The PostgreSQL type name. Currently, only <c>"timestamp"</c> and <c>"timestamp without time zone"</c>
68+
/// are supported, and only on <see cref="DateTime"/> properties. This causes the property to be stored as
69+
/// PostgreSQL <c>timestamp</c> (without time zone) instead of the default <c>timestamptz</c>.
70+
/// </param>
71+
/// <returns>The same property instance for method chaining.</returns>
72+
/// <remarks>
73+
/// <para>
74+
/// By default, .NET <see cref="DateTime"/> properties are mapped to PostgreSQL <c>timestamptz</c> (timestamp with time zone),
75+
/// which requires UTC values. Use this method to map to <c>timestamp</c> (without time zone) instead, which stores
76+
/// local/unspecified date-time values without time zone information.
77+
/// </para>
78+
/// <para>
79+
/// When using <c>timestamp</c>, <see cref="DateTime"/> values with <see cref="DateTimeKind.Unspecified"/> or
80+
/// <see cref="DateTimeKind.Local"/> kind should be used. Values read back from the database will have
81+
/// <see cref="DateTimeKind.Unspecified"/>.
82+
/// </para>
83+
/// </remarks>
84+
/// <exception cref="NotSupportedException">Thrown if the <paramref name="storeType"/> is not a supported value.</exception>
85+
public static TProperty WithStoreType<TProperty>(this TProperty property, string storeType)
86+
where TProperty : VectorStoreProperty
87+
{
88+
if (!IsTimestampStoreType(storeType))
89+
{
90+
throw new NotSupportedException(
91+
$"Store type '{storeType}' is not supported. Only 'timestamp' and 'timestamp without time zone' are supported.");
92+
}
93+
94+
property.ProviderAnnotations ??= [];
95+
property.ProviderAnnotations[StoreTypeKey] = storeType;
96+
return property;
97+
}
98+
99+
/// <summary>
100+
/// Gets the PostgreSQL store type configured for a property.
101+
/// </summary>
102+
/// <param name="property">The property to read from.</param>
103+
/// <returns>The configured store type, or <see langword="null"/> if not set.</returns>
104+
public static string? GetStoreType(this VectorStoreProperty property)
105+
=> property.ProviderAnnotations?.TryGetValue(StoreTypeKey, out var value) == true
106+
? value as string
107+
: null;
108+
109+
/// <summary>
110+
/// Gets whether the property model has been configured with a <c>timestamp</c> (without time zone) store type.
111+
/// </summary>
112+
internal static bool IsTimestampWithoutTimezone(this PropertyModel property)
113+
=> property.ProviderAnnotations?.TryGetValue(StoreTypeKey, out var value) == true
114+
&& value is string storeType
115+
&& IsTimestampStoreType(storeType);
116+
117+
private static bool IsTimestampStoreType(string storeType)
118+
=> string.Equals(storeType, "timestamp", StringComparison.OrdinalIgnoreCase)
119+
|| string.Equals(storeType, "timestamp without time zone", StringComparison.OrdinalIgnoreCase);
120+
121+
#endregion Store type
53122
}

‎dotnet/src/VectorData/PgVector/PostgresPropertyMapping.cs‎

Lines changed: 35 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,11 @@ internal static class PostgresPropertyMapping
3838
var value => throw new NotSupportedException($"Mapping for type '{value.GetType().Name}' to a vector is not supported.")
3939
};
4040

41-
public static NpgsqlDbType? GetNpgsqlDbType(Type propertyType) =>
42-
(Nullable.GetUnderlyingType(propertyType) ?? propertyType) switch
41+
/// <summary>
42+
/// Gets the NpgsqlDbType for a property, taking into account any store type annotation.
43+
/// </summary>
44+
internal static NpgsqlDbType? GetNpgsqlDbType(PropertyModel property)
45+
=> (Nullable.GetUnderlyingType(property.Type) ?? property.Type) switch
4346
{
4447
Type t when t == typeof(bool) => NpgsqlDbType.Boolean,
4548
Type t when t == typeof(short) => NpgsqlDbType.Smallint,
@@ -50,19 +53,21 @@ internal static class PostgresPropertyMapping
5053
Type t when t == typeof(decimal) => NpgsqlDbType.Numeric,
5154
Type t when t == typeof(string) => NpgsqlDbType.Text,
5255
Type t when t == typeof(byte[]) => NpgsqlDbType.Bytea,
53-
Type t when t == typeof(DateTime) => NpgsqlDbType.Timestamp,
54-
Type t when t == typeof(DateTimeOffset) => NpgsqlDbType.TimestampTz,
5556
Type t when t == typeof(Guid) => NpgsqlDbType.Uuid,
57+
Type t when t == typeof(DateTimeOffset) => NpgsqlDbType.TimestampTz,
58+
59+
// DateTime properties map to PG's "timestamp with time zone" (UTC timestamps) by default, aligning with Npgsql/EF/etc.
60+
// Users can explicitly opt into "timestamp without time zone".
61+
Type t when t == typeof(DateTime) && property.IsTimestampWithoutTimezone() => NpgsqlDbType.Timestamp,
62+
Type t when t == typeof(DateTime) => NpgsqlDbType.TimestampTz,
5663

5764
_ => null
5865
};
5966

6067
/// <summary>
61-
/// Maps a .NET type to a PostgreSQL type name.
68+
/// Maps a .NET type to a PostgreSQL type name, taking into account any store type annotation on the property.
6269
/// </summary>
63-
/// <param name="propertyType">The .NET type.</param>
64-
/// <returns>Tuple of the the PostgreSQL type name and whether it can be NULL</returns>
65-
public static (string PgType, bool IsNullable) GetPostgresTypeName(Type propertyType)
70+
internal static (string PgType, bool IsNullable) GetPostgresTypeName(PropertyModel property)
6671
{
6772
static bool TryGetBaseType(Type type, [NotNullWhen(true)] out string? typeName)
6873
{
@@ -77,7 +82,7 @@ static bool TryGetBaseType(Type type, [NotNullWhen(true)] out string? typeName)
7782
Type t when t == typeof(decimal) => "NUMERIC",
7883
Type t when t == typeof(string) => "TEXT",
7984
Type t when t == typeof(byte[]) => "BYTEA",
80-
Type t when t == typeof(DateTime) => "TIMESTAMP",
85+
Type t when t == typeof(DateTime) => "TIMESTAMPTZ",
8186
Type t when t == typeof(DateTimeOffset) => "TIMESTAMPTZ",
8287
Type t when t == typeof(Guid) => "UUID",
8388
_ => null
@@ -86,30 +91,43 @@ static bool TryGetBaseType(Type type, [NotNullWhen(true)] out string? typeName)
8691
return typeName is not null;
8792
}
8893

94+
var propertyType = property.Type;
95+
8996
// TODO: Handle NRTs properly via NullabilityInfoContext
9097

98+
(string PgType, bool IsNullable) result;
99+
91100
if (TryGetBaseType(propertyType, out string? pgType))
92101
{
93-
return (pgType, !propertyType.IsValueType);
102+
result = (pgType, !propertyType.IsValueType);
94103
}
95-
96104
// Handle nullable types (e.g. Nullable<int>)
97-
if (Nullable.GetUnderlyingType(propertyType) is Type unwrappedType
105+
else if (Nullable.GetUnderlyingType(propertyType) is Type unwrappedType
98106
&& TryGetBaseType(unwrappedType, out string? underlyingPgType))
99107
{
100-
return (underlyingPgType, true);
108+
result = (underlyingPgType, true);
101109
}
102-
103110
// Handle collections
104-
if ((propertyType.IsArray && TryGetBaseType(propertyType.GetElementType()!, out string? elementPgType))
111+
else if ((propertyType.IsArray && TryGetBaseType(propertyType.GetElementType()!, out string? elementPgType))
105112
|| (propertyType.IsGenericType
106113
&& propertyType.GetGenericTypeDefinition() == typeof(List<>)
107114
&& TryGetBaseType(propertyType.GetGenericArguments()[0], out elementPgType)))
108115
{
109-
return (elementPgType + "[]", true);
116+
result = (elementPgType + "[]", true);
117+
}
118+
else
119+
{
120+
throw new NotSupportedException($"Type {propertyType.Name} is not supported by this store.");
121+
}
122+
123+
if (property.IsTimestampWithoutTimezone())
124+
{
125+
// Replace TIMESTAMPTZ with TIMESTAMP in the PG type name.
126+
// This handles both "TIMESTAMPTZ" and "TIMESTAMPTZ[]" cases.
127+
result = ("TIMESTAMP", result.IsNullable);
110128
}
111129

112-
throw new NotSupportedException($"Type {propertyType.Name} is not supported by this store.");
130+
return result;
113131
}
114132

115133
/// <summary>

‎dotnet/src/VectorData/PgVector/PostgresSqlBuilder.cs‎

Lines changed: 22 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ internal static string BuildCreateTableSql(string schema, string tableName, Coll
6464
createTableCommand.AppendIdentifier(schema).Append('.').AppendIdentifier(tableName).AppendLine(" (");
6565

6666
// Add the key column
67-
var keyStoreType = PostgresPropertyMapping.GetPostgresTypeName(model.KeyProperty.Type).PgType;
67+
var keyStoreType = PostgresPropertyMapping.GetPostgresTypeName(model.KeyProperty).PgType;
6868
createTableCommand.Append(" ").AppendIdentifier(keyName).Append(' ').Append(keyStoreType);
6969
if (model.KeyProperty.IsAutoGenerated)
7070
{
@@ -90,7 +90,7 @@ internal static string BuildCreateTableSql(string schema, string tableName, Coll
9090
// Add the data columns
9191
foreach (var dataProperty in model.DataProperties)
9292
{
93-
var dataPgTypeInfo = PostgresPropertyMapping.GetPostgresTypeName(dataProperty.Type);
93+
var dataPgTypeInfo = PostgresPropertyMapping.GetPostgresTypeName(dataProperty);
9494
createTableCommand.Append(" ").AppendIdentifier(dataProperty.StorageName).Append(' ').Append(dataPgTypeInfo.PgType);
9595
if (!dataPgTypeInfo.IsNullable)
9696
{
@@ -224,7 +224,22 @@ internal static bool BuildUpsertCommand<TKey>(
224224
value = PostgresPropertyMapping.MapVectorForStorageModel(value);
225225
}
226226

227-
batchCommand.Parameters.Add(new() { Value = value ?? DBNull.Value });
227+
NpgsqlDbType? npgsqlDbType = null;
228+
229+
if (property.Type == typeof(DateTime))
230+
{
231+
npgsqlDbType = property.IsTimestampWithoutTimezone()
232+
? NpgsqlDbType.Timestamp
233+
// DateTime properties map to PG's "timestamp with time zone" (timestamptz) by default.
234+
// Npgsql would silently send non-UTC DateTimes as 'timestamp' (without timezone), and PG would
235+
// implicitly cast to timestamptz using the session timezone, producing incorrect results.
236+
// Explicitly set NpgsqlDbType.TimestampTz to ensure Npgsql rejects non-UTC DateTimes.
237+
: NpgsqlDbType.TimestampTz;
238+
}
239+
240+
batchCommand.Parameters.Add(npgsqlDbType.HasValue
241+
? new() { Value = value ?? DBNull.Value, NpgsqlDbType = npgsqlDbType.Value }
242+
: new() { Value = value ?? DBNull.Value });
228243
}
229244

230245
batch.BatchCommands.Add(batchCommand);
@@ -357,7 +372,7 @@ internal static void BuildGetCommand<TKey>(NpgsqlCommand command, string schema,
357372
internal static void BuildGetBatchCommand<TKey>(NpgsqlCommand command, string schema, string tableName, CollectionModel model, List<TKey> keys, bool includeVectors = false)
358373
where TKey : notnull
359374
{
360-
NpgsqlDbType? keyType = PostgresPropertyMapping.GetNpgsqlDbType(model.KeyProperty.Type) ?? throw new UnreachableException($"Unsupported key type {model.KeyProperty.Type.Name}");
375+
NpgsqlDbType? keyType = PostgresPropertyMapping.GetNpgsqlDbType(model.KeyProperty) ?? throw new UnreachableException($"Unsupported key type {model.KeyProperty.Type.Name}");
361376

362377
StringBuilder sql = new();
363378
sql.Append("SELECT ");
@@ -403,9 +418,9 @@ internal static void BuildDeleteCommand<TKey>(NpgsqlCommand command, string sche
403418
}
404419

405420
/// <inheritdoc />
406-
internal static void BuildDeleteBatchCommand<TKey>(NpgsqlCommand command, string schema, string tableName, string keyColumn, List<TKey> keys)
421+
internal static void BuildDeleteBatchCommand<TKey>(NpgsqlCommand command, string schema, string tableName, KeyPropertyModel keyProperty, List<TKey> keys)
407422
{
408-
NpgsqlDbType? keyType = PostgresPropertyMapping.GetNpgsqlDbType(typeof(TKey)) ?? throw new ArgumentException($"Unsupported key type {typeof(TKey).Name}");
423+
NpgsqlDbType? keyType = PostgresPropertyMapping.GetNpgsqlDbType(keyProperty) ?? throw new ArgumentException($"Unsupported key type {typeof(TKey).Name}");
409424

410425
for (int i = 0; i < keys.Count; i++)
411426
{
@@ -417,7 +432,7 @@ internal static void BuildDeleteBatchCommand<TKey>(NpgsqlCommand command, string
417432

418433
StringBuilder sql = new();
419434
sql.Append("DELETE FROM ").AppendIdentifier(schema).Append('.').AppendIdentifier(tableName).AppendLine()
420-
.Append("WHERE ").AppendIdentifier(keyColumn).Append(" = ANY($1);");
435+
.Append("WHERE ").AppendIdentifier(keyProperty.StorageName).Append(" = ANY($1);");
421436

422437
command.CommandText = sql.ToString();
423438
Debug.Assert(command.Parameters.Count == 0);

‎dotnet/test/VectorData/PgVector.ConformanceTests/TypeTests/PostgresDataTypeTests.cs‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,23 @@ namespace PgVector.ConformanceTests.TypeTests;
1010
public class PostgresDataTypeTests(PostgresDataTypeTests.Fixture fixture)
1111
: DataTypeTests<Guid, DataTypeTests<Guid>.DefaultRecord>(fixture), IClassFixture<PostgresDataTypeTests.Fixture>
1212
{
13+
// Npgsql maps DateTime to timestamptz by default, and so requires UTC DateTimes (the base test uses Unspecified).
14+
public override Task DateTime()
15+
=> Assert.ThrowsAsync<ArgumentException>(base.DateTime);
16+
17+
public virtual Task DateTime_utc()
18+
=> this.Test<DateTime>(
19+
"DateTime",
20+
System.DateTime.SpecifyKind(new DateTime(2020, 1, 1, 12, 30, 45), DateTimeKind.Utc),
21+
System.DateTime.SpecifyKind(new DateTime(2021, 2, 3, 13, 40, 55), DateTimeKind.Utc),
22+
instantiationExpression: () => System.DateTime.SpecifyKind(new DateTime(2020, 1, 1, 12, 30, 45), DateTimeKind.Utc));
23+
1324
// PostgreSQL does not support representing an offset, so only DateTimeOffsets with offset=0 are supported.
1425
public override Task DateTimeOffset()
26+
=> Assert.ThrowsAsync<ArgumentException>(base.DateTimeOffset);
27+
28+
// PostgreSQL does not support representing an offset, so only DateTimeOffsets with offset=0 are supported.
29+
public virtual Task DateTimeOffset_with_offset_zero()
1530
=> this.Test<DateTimeOffset>(
1631
"DateTimeOffset",
1732
new DateTimeOffset(2020, 1, 1, 12, 30, 45, TimeSpan.FromHours(0)),

0 commit comments

Comments
 (0)