ISQLQuery.ExecuteUpdate produces an exception in certain cases

[dimako]

NHibernate v5.5.2, .Net Framework 4.8

Table definition:
sql CREATE TABLE JustText(c nvarchar(max))

Failing test

        [TestMethod]
        public void ItShouldInsertTwoRecords()
        {
            var config = new Configuration();
            config.SetProperty(NHibernate.Cfg.Environment.ConnectionString, "Server=localhost;Database=Sandbox;Trusted_Connection=True;TrustServerCertificate=True");
            config.SetProperty(NHibernate.Cfg.Environment.ConnectionDriver, "NHibernate.Driver.SqlClientDriver");
            config.SetProperty(NHibernate.Cfg.Environment.Dialect, "NHibernate.Dialect.MsSql2012Dialect, NHibernate");
            using (var factory = config.BuildSessionFactory())
            {
                using (var session = factory.OpenSession())
                {
                    session.CreateSQLQuery(@"begin
INSERT INTO JustText (c)VALUES (N'
--');
INSERT INTO JustText (c)VALUES (N'
P:
');
end;").ExecuteUpdate();
                }
            }

        }

The test is failing with the exception:

NHibernate.Exceptions.GenericADOException: could not execute native bulk manipulation query:begin
INSERT INTO JustText (c)VALUES (N'
--');
INSERT INTO JustText (c)VALUES (N'
P:
');
end;[SQL: SQL not available] ---> System.Collections.Generic.KeyNotFoundException: The given key was not present in the dictionary.

  Stack Trace: 
ThrowHelper.ThrowKeyNotFoundException()
Dictionary`2.get_Item(TKey key)
NamedParameterSpecification.SetEffectiveType(QueryParameters queryParameters)
ParametersBackTrackExtensions.ResetEffectiveExpectedType(IEnumerable`1 parameterSpecs, QueryParameters queryParameters)
NativeSQLQueryPlan.PerformExecuteUpdate(QueryParameters queryParameters, ISessionImplementor session)
--- End of inner exception stack trace ---
NativeSQLQueryPlan.PerformExecuteUpdate(QueryParameters queryParameters, ISessionImplementor session)
SessionImpl.ExecuteNativeUpdate(NativeSQLQuerySpecification nativeQuerySpecification, QueryParameters queryParameters)
SqlQueryImpl.ExecuteUpdate()

[rodrigo-web-developer]

I think it's a your plain SQL string that contains colon (:). Because NHibernate will handle as parameters.
It is the expected behavior.

You can see more in docs: https://nhibernate.info/doc/nhibernate-reference/batch.html

Section 14.3 gives the example.

using (ISession session = sessionFactory.OpenSession())
using (ITransaction tx = session.BeginTransaction())
{
    string hqlUpdate = "update Customer c set c.name = :newName where c.name = :oldName";
    // or string hqlUpdate = "update Customer set name = :newName where name = :oldName";
    int updatedEntities = s.CreateQuery(hqlUpdate)
        .SetString("newName", newName)
        .SetString("oldName", oldName)
        .ExecuteUpdate();
    tx.Commit();
}

I had this problem when using string with colon when running this kind of SQL. You can change your SQL string to not allow colon character or you can call another method that hasn't parameter handling like create DbCommand.

var sqlCommand = session.Connection.CreateCommand(); //session is a ISession instance
sqlCommand.CommandText = "INSERT INTO table(column) VALUES (':this_is_not_a_parameter')";
sqlCommand.ExecuteNonQuery();

[dimako]

I am pretty sure the bug I am logging could not possible be the expected functionality. And let me try to explain why.
In your example you're using a properly parameterized query
"UPDATE Customer set name = :newName..." - here I would expect NH to parse the query and expect the "newName" parameter to be provided.
The other example would be something like this:
"UPDATE Customer set name = ':newName',," - here newName is not a parameter - it's a part of the final string that I expect to see in the database as it is. Of course the best practice tells us to use parametrized queries but NH should be able to cope with hardcoded queries.

But the bug that I am reporting is not even about that. The bug happens where there is a combination of the ":" character, new lines and "--" characters in different parts of the query.

[rodrigo-web-developer]

@dimako it seems that depends on query NHibernate will perform as parameter or not.

            using (var session = NHibernateHelper.OpenSession())
            {
                session.CreateSQLQuery(@"
INSERT INTO Table (description) VALUES (N'
--');
INSERT INTO Table (description) VALUES (N'
X:
');
").ExecuteUpdate();
            }

The code above will cause the same error. But if I change the first insert with a break line character, it works fine:

            using (var session = NHibernateHelper.OpenSession())
            {
                session.CreateSQLQuery(@"
INSERT INTO Table (description) VALUES (N'
--
');
INSERT INTO Table (description) VALUES (N'
X:
');
").ExecuteUpdate();
            }

It seems that NHibernate thinks the 'X:' in the second query is within quotes, but in the first case, NHibernate perform as 'X:' is outside quotes.

It happens on NHibernate.Engine.Query.ParameterParser.Parse(string query, IRecognizer recognizer) method.

The sequence of reading is simple, the problem is the '--' characters, because it is the common line comments on SQL.

So in the first case, NHibernate reads like:

• INSERT INTO Table (description) VALUES (N': OK, you start a quote here, so we have inQuote = true, lets continue
• --'); : OK you comment this line, so I will ignore it
• INSERT INTO Table (description) VALUES (N': second insert, we have a quote here, but if inQuote == true, so we closing it here. isQuote = false
• X:: OK, so isQuote == false, then this string is a parameter. ERROR is here.

The second case, NHibernate will read like:

• INSERT INTO Table (description) VALUES (N': OK, you start a quote here, so we have inQuote = true, lets continue
• --: OK you comment this line, so I will ignore it
• ');: There is a quote here, so if inQuote == true, you closing it here. inQuote = false
• INSERT INTO Table (description) VALUES (N': second insert, we have a quote here, but if inQuote == false, so we opening a new one here. isQuote = true
• X:: OK, so inQuote == true, then this string is NOT a parameter.

The problem happens on line 65, which consider that if we start a new line with --, so it is a comment.

if (afterNewLine && (indx + 1 < stringLength) && sqlString.Substring(indx, 2) == "--")

But the correct way is verify if we are not inQuote, because will not be a comment:

if (afterNewLine && !inQuote && (indx + 1 < stringLength) && sqlString.Substring(indx, 2) == "--")

If you has not parameter at all. You can perform the SQL with another method as DbCommand in my first answer.

But if you need to perform your SQL with named parameters replace, you will have problems in the case of closing quotes after line which starts with --.

[dimako]

Thanks @rodrigo-web-developer for such a detailed explanation. I hope it will make it easier for the NH team to address that. Meanwhile we're going to come up with a workaround.
Thank you!

[rodrigo-web-developer]

I had problems before with ExecuteUpdate() because I work with PostgreSQL and it has a CAST operator ::, so it is usual to have colon outside quotes like INSERT INTO table(id) VALUES ('123'::INTEGER) <- NHibernate will cause error here.

I prefer to not use it, creating functions, views, updates, inserts etc will cause error when casting in Postgres. If I dont need named parameters here, DbCommand will works fine. All my SQL strings are plain SQL with no interpolation and has no string.Replace calls.

But if you need to write SQL with named parameters, this will be a problem.

I have no time now to create a PR to solve your problem, but I hope with my debugging NH devs can fix this for you asap.

Best regards.