Superfluous SQL casts generated in FirebirdClientDriver

[catcrab]

I've discovered two problems:

1. In select statements, every parameter is 'casted'.
   When single entity selected by ID (Session.Get), SQL is good, but when selecting multiple entities using Restrictions.InG, every ID is casted. Searching internet, i've found one forum thread about this problem.
   SQL example:

SELECT this_.ID as ID19_0_,
        this_.NAME as NAME19_0_,
        this_.SHORT_NAME as SHORT3_19_0_,
        this_.CREATE_DATETIME as CREATE4_19_0_,
        this_.PARENT_ID as PARENT5_19_0_,
        this_.TYPE_ID as TYPE6_19_0_,
        this_.STATE_ID as STATE7_19_0_,
        this_.TYPE_ID as clazz_0_
    FROM L_TREE this_
    WHERE this_.ID in (cast(@p0 as VARCHAR(255)))

Parameters:
Name:@p0 Type:VarChar Used Value:c034e889-3b80-42d3-bdad-5f4e729a905b

More details here (Russian).

The problem is Firebird SQL server stops using primary key when parameter list is too long and every parameter is casted (it uses index, if I remove casts). For short parameter lists, casts does not affect query plan.

2. In insert statements, I can't insert long string (more than 255 chars) into some tables, because string parameter is casted to varchar(255):

INSERT INTO My_Table (Fio, SELECTED_MODE, STATE)
    VALUES (cast(@p0 as VARCHAR(255)), cast(@p1 as INTEGER), cast(@p2 as INTEGER))

After reading debug logs and source code, i've got explanation of this.
Looks like FirebirdClientDriver expected to cast parameters for 'first', 'between' and 'like' operators in select statements, but there are two mistakes in regexes.

The regex
string SELECT_CLAUSE_EXP = @"(?<=\bselect|\bwhere).*";
should match 'select' statements with 'where', but it matches any statement, containing word, starting with 'select' or 'where' (like INSERT INTO My_Table (Fio, SELECTED_MODE...) given above)

The regex
CAST_PARAMS_EXP = @"(?<![=<>]\s?|first\s?|skip\s?|between\s|between\s@\bp\w+\b\sand\s)@\bp\w+\b(?!\s?[=<>])";
should match parameters of first, between, like, etc. But matches every parameter that is not prepended with = < > characters.

Not a regex guru, so I have no idea, how to fix them right.
But completely removing parameter casting (GetStatementsWithCastCandidates, GetCastCandidates) have no issues with current versions of firebird (2.x, 3.0). I suspect that 'cast' was workaround for very old firebird versions, and are not actual today.

[fredericDelaporte]

This casting thing is quite a dirty hack, and I am not surprised it creates issues.

I had also previously spotted the string length hard limit of 255, but not knowing if that may have some other bad consequences to switch it to Dialect.DefaultCastLength (4000 by default, can be changed in NHibernate settings), I have done nothing about it. Still this one should not be too risky to change, and could be changed as a separated issue.

About completely removing the casts, have you tried running the NHibernate test suite without them? I think Firebird does still need them in many cases and that many NHibernate tests will start failing without them.

For the first regex mistake, adding a \b after select and where should do it, to be ascertained. For the second mistake, I am not sure it is a mistake, to begin with.

[fredericDelaporte]

I had also previously spotted the string length hard limit of 255, but not knowing if that may have some other bad consequences to switch it to Dialect.DefaultCastLength (4000 by default, can be changed in NHibernate settings), I have done nothing about it. Still this one should not be too risky to change, and could be changed as a separated issue.

I am wrong, it is changed since #709, so since NHibernate v5.0. It seems you are using an older version. So for the "can't insert long string (more than 255 chars)" part, it is an obsolete statement. It is now limited to 4000 by default, and this can be changed by configuration.

[catcrab]

What's the reason for this casts?

I'm using NHibernate 4.0 and completely removed casts with custom driver, inherited from FirebirdClientDriver and empty override
public override void AdjustCommand(IDbCommand command) { }

No issues on large CRM application.

[catcrab]

For the second mistake, I am not sure it is a mistake, to begin with.

Then why there is no cast in query
SELECT ... FROM L_TREE this_ WHERE this_.ID=@p0
And there is a cast in similar query
SELECT ... FROM L_TREE this_ WHERE this_.ID in (cast(@p0 as VARCHAR(255)))

[fredericDelaporte]

I have checked the test suite without the cast: 37 tests fail, with Firebird 3. Most of them (if not all) fail with the error:

FirebirdSql.Data.FirebirdClient.FbException : Dynamic SQL Error
SQL error code = -804
Data type unknown
FirebirdSql.Data.Common.IscException : Dynamic SQL Error
SQL error code = -804
Data type unknown

(I have not counted those checking the cast hack is done, I mean NHibernate.Test.DriverTest.FirebirdClientDriverFixture. And I have divided by two the remaining failures count, for not counting the async counterpart of each test.)

Example failing queries:

SELECT first @p0 skip @p1
        (case when this_.Name = @p2 then @p3 else @p4 end) as y0_
    FROM Student this_
    ORDER BY y0_ asc

SELECT this_.studentId as studentid1_2_0_,
        this_.Name as name2_2_0_,
        this_.address_city as address3_2_0_,
        this_.address_state as address4_2_0_,
        this_.preferredCourseCode as preferredcoursecode5_2_0_
    FROM Student this_
    ORDER BY (case when this_.studentId = @p0 then @p1 else @p2 end) asc

SELECT (this_.Name||@p0||this_.Name) as y0_
    FROM Person this_
    WHERE this_.Name = @p1

SELECT (this_.studentId + @p0) as y0_ FROM Student this_

SELECT (case when this_.studentId = @p0 then @p1 else @p2 end) as y0_
    FROM Student this_

SELECT (this_.Name||@p0||this_.Name) as y0_ FROM Student this_

SELECT (cast( this_.studentId as VARCHAR(4000))||@p0||this_.Name) as y0_ FROM Student this_

insert into Animal ( id, description, body_weight, mother_id )
    select gen_id(hibernate_sequence, 1 ),
            human0_2_.description as col_0_0_,
            human0_2_.body_weight as col_1_0_,
            ? as col_2_0_
        from Human human0_
        inner join Mammal human0_1_ on human0_.mammal=human0_1_.animal
        inner join Animal human0_2_ on human0_.mammal=human0_2_.id

insert into Animal ( id, description, body_weight )
    select gen_id(hibernate_sequence, 1 ),
            animal0_.description as col_0_0_,
            ? as col_1_0_
        from Animal animal0_
        where animal0_.body_weight<?

select animal0_.Id as id1_0_,
        animal0_.Description as description2_0_,
        animal0_.BodyWeight as bodyweight3_0_,
        animal0_1_.name_first as name2_1_,
        animal0_1_.name_initial as name3_1_,
        animal0_1_.name_last as name4_1_,
        animal0_1_.NickName as nickname5_1_,
        animal0_1_.Birthdate as birthdate6_1_,
        case
            when animal0_1_.animalId is not null then 1
            when animal0_.Id is not null then 0 end as clazz_
    from Animal animal0_
    left outer join Human animal0_1_ on animal0_.Id=animal0_1_.animalId
    where cast(@p0+animal0_.BodyWeight as DOUBLE PRECISION)>0

select cast((animal0_.BodyWeight+50)/@p0 as INTEGER) as col_0_0_
    from Animal animal0_

select animal0_.Id as id1_0_,
        animal0_.Description as description2_0_,
        animal0_.BodyWeight as bodyweight3_0_,
        animal0_1_.name_first as name2_1_,
        animal0_1_.name_initial as name3_1_,
        animal0_1_.name_last as name4_1_,
        animal0_1_.NickName as nickname5_1_,
        animal0_1_.Birthdate as birthdate6_1_,
        case
            when animal0_1_.animalId is not null then 1
            when animal0_.Id is not null then 0 end as clazz_
    from Animal animal0_
    left outer join Human animal0_1_ on animal0_.Id=animal0_1_.animalId
    where substring(animal0_.Description from @p0 for @p1)=@p2

select lower(employee0_.FirstName) as col_0_0_
    from Employees employee0_
    where position(@p0, lower(employee0_.FirstName), cast(@p1+1 as int))-1=@p2

select employee0_.EmployeeId as employeeid1_1_,
        employee0_.LastName as lastname2_1_,
        employee0_.FirstName as firstname3_1_,
        employee0_.Title as title4_1_,
        employee0_.TitleOfCourtesy as titleofcourtesy5_1_,
        employee0_.BirthDate as birthdate6_1_,
        employee0_.HireDate as hiredate7_1_,
        employee0_.Address as address8_1_,
        employee0_.City as city9_1_,
        employee0_.Region as region10_1_,
        employee0_.PostalCode as postalcode11_1_,
        employee0_.Country as country12_1_,
        employee0_.HomePhone as homephone13_1_,
        employee0_.Extension as extension14_1_,
        employee0_.Notes as notes15_1_,
        employee0_.ReportsTo as reportsto16_1_
    from Employees employee0_
    where substring(employee0_.FirstName from @p0+1)=@p1

If you wish to see for which cases the cast is required, here is the list of tests generating the above queries.

• NHibernate.Test.Criteria.CriteriaQueryTest.OrderProjectionAliasedTest
• NHibernate.Test.Criteria.CriteriaQueryTest.OrderProjectionTest
• NHibernate.Test.Criteria.Lambda.FunctionsIntegrationFixture.Concat
• NHibernate.Test.Criteria.ProjectionsTest.CanUseParametersWithProjections
• NHibernate.Test.Criteria.ProjectionsTest.UsingConditionals
• NHibernate.Test.Criteria.ProjectionsTest.UsingSqlFunctions_Concat
• NHibernate.Test.Criteria.ProjectionsTest.UsingSqlFunctions_Concat_WithCast
• NHibernate.Test.Hql.Ast.BulkManipulation.InsertWithManyToOneAsParameter
• NHibernate.Test.Hql.Ast.HqlFixture.InsertIntoFromSelect_WithSelectClauseParameters
• NHibernate.Test.Hql.HQLFunctions.Cast
• NHibernate.Test.Hql.HQLFunctions.CastNH1979
• NHibernate.Test.Hql.HQLFunctions.SubString
• NHibernate.Test.Linq.FunctionTests.CharIndexOffsetNegativeFunction
• NHibernate.Test.Linq.FunctionTests.SubstringFunction1

As you can see, there are many different cases affected by the removing of the cast hack, including polymorphism, basic LINQ translation of methods like SubString, miscellaneous computations in queries.

So this hack is not going to be removed.

[fredericDelaporte]

The regex
CAST_PARAMS_EXP = @"(?<![=<>]\s?|first\s?|skip\s?|between\s|between\s@\bp\w+\b\sand\s)@\bp\w+\b(?!\s?[=<>])";
should match parameters of first, between, like, etc. But matches every parameter that is not prepended with = < > characters.

No, that is a zero-witdh negative look behind assertion (grouping construct (?<!...)). It is meant to prevent matching parameters used for first, skip and between, along with those directly compared to.

There are no mistakes here. But it should be possible to add a condition there to also not match parameters used in in conditions.