NH-3017 - Paging query(hql/criteria) with use_sql_comments set to true throws exception

[nhibernate-bot]

Luis Fernando created an issue — 16th January 2012, 21:13:12:

I'm tried to implement a method that will create all the criterias I needed to execute and return the data as paged list, but it fails if the pageIndex is equal to 0 (the first page).

Here's the method.

public static IList<T> GetPagedListByCriteria(ICriteria criteria, int pageIndex, int pageSize, out long totalCount, ISession Session)
{
    ICriteria recordsCriteria = CriteriaTransformer.Clone(criteria);

    // Paging.
     recordsCriteria.SetFirstResult(pageIndex * pageSize);
     recordsCriteria.SetMaxResults(pageSize);

    // Count criteria.
    ICriteria countCriteria = CriteriaTransformer.TransformToRowCount(criteria);

    // Perform multi criteria to get both results and count in one trip to the database.
    IMultiCriteria multiCriteria = Session.CreateMultiCriteria();
    multiCriteria.Add(recordsCriteria);
    multiCriteria.Add(countCriteria);
    IList multiResult = multiCriteria.List();

    var untypedRecords = multiResult<0> as IList;
    IList<T> records = new List<T>();
    if (untypedRecords != null)
    {
        foreach (T obj in untypedRecords)
        {
             records.Add(obj);
        }
    }

    totalCount = Convert.ToInt64(((IList)multiResult<1>)[0]);

    return records;
}

---

Luis Fernando added a comment — 16th January 2012, 21:17:10:

Related Stackoverflow question: http://stackoverflow.com/questions/8844164/nhibernate-multicriteria-throws-exception-when-trying-to-do-a-paging-query

---

Luis Fernando added a comment — 16th January 2012, 22:03:37:

The QueryOver suffers the same problem.

---

Luis Fernando added a comment — 16th January 2012, 22:04:38:

QueryOver sample:

var pagedList = SessionFactory.OpenSession()
      .QueryOver<MyEntity>().Skip(0).Take(10).List();

---

Luis Fernando added a comment — 16th January 2012, 22:11:22:

After going back and looking at this issue, i figured out what happened. I noticed that NHibernate was actually throwing an error. The message was "The query should start with 'SELECT' or 'SELECT DISTINCT", and found out that the following NHibernate configuration property was causing this bug:

<property name="use*sql*comments">true</property>

It's certainly a bug, because the GetAfterSelectInsertPoint method doesn't take into account that SQL comments may be prepended to the SQL query.

Just set the usesqlcomments property to false and the problem disappears, I wish it would be fixed in the next version.

I do not know how to do a patch, but maybe this blog post can help: http://www.codewrecks.com/blog/index.php/2011/07/28/a-strange-bug-in-nhibernate-2-1/

Wow, did you see? This occurs since version 2.1 and perhaps before that... XD

---

Luis Fernando added a comment — 16th January 2012, 23:11:17:

Well, I ended up opting for a solution that I consider more elegant than removing a feature. So I decided to create an extension to the MsSql2008Dialect (which is where the bug is located) as follows:

public class MsSql2008DialectBugFix : MsSql2008Dialect
{
    public override SqlString GetLimitString(SqlString queryString, SqlString offset, SqlString limit)
    {
        var result = new SqlStringBuilder();

        if (offset == null)
        {
            var insertPoint = GetAfterSelectInsertPoint(queryString);

            result
                .Add(queryString.Substring(0, insertPoint))
                .Add(" TOP (")
                .Add(limit)
                .Add(") ")
                .Add(queryString.Substring(insertPoint));
                    
            return result.ToSqlString();
        }

        return base.GetLimitString(queryString, offset, limit);
    }

    private static int GetAfterSelectInsertPoint(SqlString sql)
    {
        Int32 selectPosition;

        if ((selectPosition = sql.IndexOfCaseInsensitive("select distinct")) >= 0)
        {
            return selectPosition <ins> 15; // "select distinct".Length;
        }
        if ((selectPosition = sql.IndexOfCaseInsensitive("select")) >= 0)
        {
            return selectPosition </ins> 6; // "select".Length;
        }

        throw new NotSupportedException("The query should start with 'SELECT' or 'SELECT DISTINCT'");
    }
}

---

Oskar Berggren added a comment — 17th January 2012, 9:53:59:

NH-2977 is also related to better SQL parsing related to the limit strings, and has a patch which will probably go in before 3.3.

---

Oskar Berggren added a comment — 17th January 2012, 9:56:01:

Luis, would you mind creating simple test case that expose the problem?

---

Luis Fernando added a comment — 17th January 2012, 23:21:29:

Of course I can do it, maybe in one or two days, ok.

---

Julian Maughan added a comment — 6th April 2012, 5:12:44:

Maybe Skip(0) should be removed from the query before it gets to the dialect - its completely redundant.

---

Julian Maughan added a comment — 6th April 2012, 5:13:35:

Since there is a simple workaround for this issue - i.e. 'usesqlcomments' to false - I'm changing the priority of this issue.

[fredericDelaporte]

MultiCriteria has been obsoleted in favor of QueryBatch, which has tests about auto-flush.

See #1783.

[fredericDelaporte]

Sorry, I have post on the wrong page.

[fredericDelaporte]

In fact, this has been fixed for SQL server "by the way" when NH-2977 was implemented (NH v3.4). But the trouble remains for other dialects, like Firebird, Sybase, Informix, SqlServerCe.

This trouble is not bound to multi-query/criteria.

[bernatgy]

This just took a whole day out of my life. Please make this issue pop in Google somehow......... I don't really care for the actual fix, as the AutoCommentSql was accidentally turned on for me, but I couldn't find this issue for a whole 18 hours straight, while looking at extremely simple queries generating to 2-3 words in the console...... then the Firebird driver throwing all the relevant exceptions.