Skip to content

Fix invalid handling of null DateTime parameters in Npgsql 6+ #3299

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.

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
merged 1 commit into from
May 8, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
79 changes: 79 additions & 0 deletions src/NHibernate.Test/Async/NHSpecificTest/GH3291/Fixture.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
//------------------------------------------------------------------------------
// <auto-generated>
// This code was generated by AsyncGenerator.
//
// Changes to this file may cause incorrect behavior and will be lost if
// the code is regenerated.
// </auto-generated>
//------------------------------------------------------------------------------


using System;
using System.Linq;
using NHibernate.Criterion;
using NUnit.Framework;
using NHibernate.Linq;

namespace NHibernate.Test.NHSpecificTest.GH3291
{
using System.Threading.Tasks;
[TestFixture]
public class FixtureAsync : BugTestCase
{
protected override void OnSetUp()
{
using var session = OpenSession();
using var transaction = session.BeginTransaction();

var e1 = new Person { Name = "Bob", DateOfBirth = new DateTime(2009, 12, 23) };
session.Save(e1);

var e2 = new Person { Name = "Sally", DateOfBirth = new DateTime(2018, 9, 30) };
session.Save(e2);

transaction.Commit();
}

protected override void OnTearDown()
{
using var session = OpenSession();
using var transaction = session.BeginTransaction();

session.CreateQuery("delete from System.Object").ExecuteUpdate();

transaction.Commit();
}

[Test]
public async Task LinqAsync()
{
using var session = OpenSession();
using var _ = session.BeginTransaction();

DateTime? dateOfSearch = null;

var result = await ((
from person in session.Query<Person>()
where dateOfSearch == null || person.DateOfBirth > dateOfSearch
select person).ToListAsync());

Assert.That(result, Has.Count.EqualTo(2));
}

[Test]
public async Task HqlAsync()
{
using var session = OpenSession();
using var _ = session.BeginTransaction();

DateTime? dateOfSearch = null;

var result =
await (session.CreateQuery("from Person where :DateOfSearch is null OR DateOfBirth > :DateOfSearch")
.SetParameter("DateOfSearch", dateOfSearch, NHibernateUtil.DateTime)
.ListAsync<Person>());

Assert.That(result, Has.Count.EqualTo(2));
}
}
}
67 changes: 67 additions & 0 deletions src/NHibernate.Test/NHSpecificTest/GH3291/Fixture.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
using System;
using System.Linq;
using NHibernate.Criterion;
using NUnit.Framework;

namespace NHibernate.Test.NHSpecificTest.GH3291
{
[TestFixture]
public class Fixture : BugTestCase
{
protected override void OnSetUp()
{
using var session = OpenSession();
using var transaction = session.BeginTransaction();

var e1 = new Person { Name = "Bob", DateOfBirth = new DateTime(2009, 12, 23) };
session.Save(e1);

var e2 = new Person { Name = "Sally", DateOfBirth = new DateTime(2018, 9, 30) };
session.Save(e2);

transaction.Commit();
}

protected override void OnTearDown()
{
using var session = OpenSession();
using var transaction = session.BeginTransaction();

session.CreateQuery("delete from System.Object").ExecuteUpdate();

transaction.Commit();
}

[Test]
public void Linq()
{
using var session = OpenSession();
using var _ = session.BeginTransaction();

DateTime? dateOfSearch = null;

var result = (
from person in session.Query<Person>()
where dateOfSearch == null || person.DateOfBirth > dateOfSearch
select person).ToList();

Assert.That(result, Has.Count.EqualTo(2));
}

[Test]
public void Hql()
{
using var session = OpenSession();
using var _ = session.BeginTransaction();

DateTime? dateOfSearch = null;

var result =
session.CreateQuery("from Person where :DateOfSearch is null OR DateOfBirth > :DateOfSearch")
.SetParameter("DateOfSearch", dateOfSearch, NHibernateUtil.DateTime)
.List<Person>();

Assert.That(result, Has.Count.EqualTo(2));
}
}
}
11 changes: 11 additions & 0 deletions src/NHibernate.Test/NHSpecificTest/GH3291/Mappings.hbm.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<?xml version="1.0" encoding="utf-8" ?>
<hibernate-mapping xmlns="urn:nhibernate-mapping-2.2" assembly="NHibernate.Test"
namespace="NHibernate.Test.NHSpecificTest.GH3291">

<class name="Person">
<id name="Id" generator="guid.comb"/>
<property name="Name"/>
<property name="DateOfBirth" />
</class>

</hibernate-mapping>
11 changes: 11 additions & 0 deletions src/NHibernate.Test/NHSpecificTest/GH3291/Person.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
using System;

namespace NHibernate.Test.NHSpecificTest.GH3291
{
class Person
{
public virtual Guid Id { get; set; }
public virtual string Name { get; set; }
public virtual DateTime? DateOfBirth { get; set; }
}
}
6 changes: 1 addition & 5 deletions src/NHibernate.TestDatabaseSetup/TestDatabaseSetup.cs
Original file line number Diff line number Diff line change
Expand Up @@ -182,11 +182,7 @@ private static void SetupNpgsql(Cfg.Configuration cfg)

using (var cmd = conn.CreateCommand())
{
cmd.CommandText =
@"CREATE OR REPLACE FUNCTION uuid_generate_v4()
RETURNS uuid
AS '$libdir/uuid-ossp', 'uuid_generate_v4'
VOLATILE STRICT LANGUAGE C;";
cmd.CommandText = "CREATE EXTENSION IF NOT EXISTS \"uuid-ossp\";";

cmd.ExecuteNonQuery();
}
Expand Down
21 changes: 18 additions & 3 deletions src/NHibernate/Driver/NpgsqlDriver.cs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
using System;
using System.Data;
using System.Data.Common;
using NHibernate.AdoNet;
Expand Down Expand Up @@ -74,14 +75,28 @@ protected override void InitializeParameter(DbParameter dbParam, string name, Sq
// Since the .NET currency type has 4 decimal places, we use a decimal type in PostgreSQL instead of its native 2 decimal currency type.
dbParam.DbType = DbType.Decimal;
}
else if (DriverVersionMajor < 6 || sqlType.DbType != DbType.DateTime)
else
{
dbParam.DbType = sqlType.DbType;
}
else
}

public override void AdjustCommand(DbCommand command)
{
if (DriverVersionMajor >= 6)
{
// Let Npgsql 6 driver to decide parameter type
for (var i = 0; i < command.Parameters.Count; i++)
{
var parameter = command.Parameters[i];
if (parameter.Value is DateTime)
Copy link
Member

@bahusoid bahusoid May 5, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I like this runtime type check for all parameters values.

It's not clear what original issue #2994 is about as no details are provided. But DateTime can represent multiple DbType types (Date, DateTime, Time). Original fix #3064 was applied only for DBType.DateTime.
So is it omission in original fix or other types are not affected and no fix is required? I wouldn't extend scope of fix unless it's really necessary.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not clear what original issue #2994 is about as no details are provided.

There are breaking changes in Npgsql 6 that require a DateTime of certain Kind to match timestamp or timestamptz through a "help" of DbType.DateTime2 type. Here are some explanations: https://www.npgsql.org/doc/release-notes/6.0.html#detailed-notes and here: https://www.roji.org/postgresql-dotnet-timestamp-mapping

It is really hard to explain their motivation and the implementation. But it does not align with the type system used by NHibernate.

But DateTime can represent multiple DbType types (Date, DateTime, Time).

I don't think it applies here. However, I think, it is possible to write a more specific conversion.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, it does apply to DbType.Date. I'll adjust the fix.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

{
// Let Npgsql 6 driver to decide parameter type
parameter.ResetDbType();
}
}
}

base.AdjustCommand(command);
}

// Prior to v3, Npgsql was expecting DateTime for time.
Expand Down