Skip to content

Time is incompatible with bigint for TimeAsTimeSpanType #2963

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

Closed
Bogatinov opened this issue Dec 22, 2021 · 10 comments · Fixed by #2964
Closed

Time is incompatible with bigint for TimeAsTimeSpanType #2963

Bogatinov opened this issue Dec 22, 2021 · 10 comments · Fixed by #2964

Comments

@Bogatinov
Copy link

Bogatinov commented Dec 22, 2021

Context

Updating v5.1.7 to 5.3.10 together with FluentNHibernate 3.1.0

NHibernate LINQ providers don't support DateTime.TimeOfDay to automatically convert to SQL time

Approach

RegisterFunction() to the rescue to adjust the dialect, where in 5.1.7 the following code works for MS SQL 2012 dialect

RegisterFunction(nameof(Ext.GetTime), 
                 new SQLFunctionTemplate(NHibernateUtil.TimeAsTimeSpan, "cast(?1 as time)"));

where Ext.GetTime only accesses TimeOfDay

public static class Ext
{
    public static TimeSpan GetTime(this DateTime dateTime)
    {
        return dateTime.TimeOfDay;
    }
}

Usage

var start = filter.Start.GetTime();
var end = filter.End.GetTime();

return Session.Query<Entity>().Where(x => x.Start.GetTime() < end && x.End.GetTime() > start).ToList();

class Entity
{
    public virtual DateTime Start { get; set; }
    public virtual DateTime End { get; set; }
}

Exception in 5.3.10

While v5.3.10 throws an exception

---- System.Data.SqlClient.SqlException : Operand type clash: time is incompatible with bigint

I understand TimeSpan can have more than 24h, whereas this method is used only for less than 24h intervals.

Related: TimeAsTimeSpanType has changes from v5.1.3 related to casting types to TimeSpan.

Questions

Trying to figure out

  1. Where InternalTicks from TimeOfDay tries to map to SQL type time in NHibernate
  2. Documentation related to this breaking change
  3. Why v5.1.7 works and what is the new approach in v5.3.10 version

Open to ideas how to contribute to documentation 😃

@gliljas
Copy link
Member

gliljas commented Dec 22, 2021

That seems a bit odd. The error indicates that the datetime column is stored as a bigint, and it's coming from SQL Server.

cast(?1 as time)

is not valid, in that scenario.

@bahusoid
Copy link
Member

Does it work properly in 5.2? I suspect it's a regression caused by #2365

So in expression x.Start.GetTime() < end end parameter is no longer detected as TimeAsTimeSpanType and treated as TimeSpanType (that's what gives bigint)

@Bogatinov
Copy link
Author

@bahusoid yes, that is correct. v5.2.7 doesn't have this issue 😄 At the same time, FluentNHibernate requires v5.3.3+dependency to fix Net 5.0: BinaryFormatter serialization and deserialization so that the Web app doesn't fail on startup.

Hi @gliljas, if I understand you correctly, I should change RegisterFunction to use a different type and not use ?1 as time

RegisterFunction(nameof(Ext.GetTime), 
                 new SQLFunctionTemplate(NHibernateUtil.WhichType, "sql_cast_conversion"));

resulting in Ext.GetTime to return a different type as well.

This is my first time on NHibernate issue, and I'm trying to figure out how to use the TimeAsTimeSpanType again in v5.3.10, by using C# TimeSpan data type in LINQ-to-SQL conversion by reading the documentation
https://nhibernate.info/doc/nhibernate-reference/mapping.html#mapping-types-basictypes

@bahusoid
Copy link
Member

FluentNHibernate requires v5.3.3+dependency

If it's the only reason to upgrade, try assembly binding redirect

@Bogatinov
Copy link
Author

Bogatinov commented Dec 24, 2021

Thank you for the assistance 😄

Until 5.3.11 release, it's best to revert to v5.2.7 and apply binding redirect for FluentNHibernate.

Tried couple of conversions, but the data types are not the same in comparison, resulting in SQL errors.

RegisterFunction(nameof(Ext.GetTime), 
                 new SQLFunctionTemplate(NHibernateUtil.TimeAsTimeSpan, "convert(char(8), ?1, 108)"));

and there is no easy + performance way to convert the Ticks into SQL, that NHibernate generates from TimeSpanType

v5.3.10

--part of WHERE
and cast(item0_.Start as time) < 36000000000 /* @p6 */

v5.2.7

--part of WHERE
and cast(item0_.Start as time) < '01:00:00' /* @p6 */

@bahusoid
Copy link
Member

So we don't guess type for custom hql generators since #2793
But it seems we also shouldn't guess types if hql generator is used in related expressions

public readonly Dictionary<Expression, HashSet<Expression>> RelatedExpressions =

@maca88 Can you take a look?

@bahusoid
Copy link
Member

@maca88
Copy link
Contributor

maca88 commented Dec 26, 2021

I've made a PR that uses the guessed type only when the hql logic is not able to determine it (similar as in SelectClause) or the value is a char. For a char value, hql logic will always detect it as a string, which casues an exception to be thrown when setting the DbParameter value. In NHibernate 5.2, char values were converted to string in ExpressionParameterVisitor, but I still think that it is more correct to not convert them.

@bahusoid
Copy link
Member

bahusoid commented Dec 26, 2021

So it's a variation of #2723 in the end.

In NHibernate 5.2, char values were converted to string in ExpressionParameterVisitor

I didn't know it. So this breakage of some char related tests made me think it's not safe for 5.3...

or the value is a char

What about the case when initial type is AnsiStringType. (something like entity.AnsiString[0] == charParam) It seems now (comparing to 5.2) we loose this information.

@bahusoid
Copy link
Member

bahusoid commented Jan 4, 2022

Fixed by #2964

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants