MS Sql Server dialects

[CSharper]

Hi,

I experimented a little with upcoming SQL Server 2012 and found that
there are some existing red unit tests for SQL Server in HQL
functions. I've copied two function definitions from
SybaseASE15Dialect to SQL 2005 dialect (I don't know if it would work
already in SQL Server 2000) and did a quick test implementation for
the new recommended syntax for using pagination in SQL Server 2012
like
SELECT xxx FROM yyy
ORDER BY zzz OFFSET @p1 ROWS FETCH NEXT @p2 ROWS ONLY
It works, the pagination unit tests are all green (the others too).

But why should we change that paging behaviour?

The new syntax is less intrusive for the SQL string, it still works
when executing a native SQL query containing a common table
expression. If the original select is
WITH q AS (aaa)
SELECT xxx FROM yyy
the paging in SQL Server 2005 dialect fails with an exception because
the sql string does not start with a select. The new paging syntax
should work without any problems for such cases.

How to continue with this? Are there opinions about it? Is somebody
interested in applying a patch and introducing an SQL Server 2012
dialect already?

The changes are as follows:

MsSql2005Dialect.cs
@@ -3,10 +3,11 @@ using System.Collections.Generic;
using System.Data;
using NHibernate.Driver;
using NHibernate.Mapping;
using NHibernate.SqlCommand;
using NHibernate.Util;
+using NHibernate.Dialect.Function;

namespace NHibernate.Dialect
{
public class MsSql2005Dialect : MsSql2000Dialect
{
@@ -34,10 +35,17 @@ namespace NHibernate.Dialect
{
base.RegisterKeywords();
RegisterKeyword("xml");
}

+ protected override void RegisterFunctions()
+ {
+ base.RegisterFunctions();
+ RegisterFunction("extract", new
SQLFunctionTemplate(NHibernateUtil.Int32, "datepart(?1, ?3)"));
+ RegisterFunction("bit_length", new
SQLFunctionTemplate(NHibernateUtil.Int32, "datalength(?1) * 8"));
+ }
+
public override SqlString GetLimitString(SqlString queryString,
SqlString offset, SqlString limit)
{
var result = new SqlStringBuilder();

if (offset == null)
@@ -168,11 +176,11 @@ namespace NHibernate.Dialect
/// Indicates whether the string fragment contains matching
parenthesis
/// </summary>
/// <param name="statement"> the statement to evaluate</param>
/// <returns>true if the statment contains no parenthesis or an
equal number of
/// opening and closing parenthesis;otherwise false </returns>
- private static bool HasMatchingParens(IEnumerable<char> statement)
+ protected static bool HasMatchingParens(IEnumerable<char>
statement)
{
//unmatched paren count
int unmatchedParen = 0;

//increment the counts based in the opening and closing parens in
the statement


MsSql2012Dialect.cs:
new file mode 100644
@@ -0,0 +1,38 @@
+using System.Data;
+using NHibernate.Cfg;
+using NHibernate.Dialect.Function;
+using NHibernate.Driver;
+using NHibernate.SqlCommand;
+
+namespace NHibernate.Dialect
+{
+ public class MsSql2012Dialect : MsSql2008Dialect
+ {
+ public override SqlString GetLimitString(SqlString
queryString, SqlString offset, SqlString limit)
+ {
+ var result = new SqlStringBuilder(queryString);
+
+ int orderIndex = queryString.LastIndexOfCaseInsensitive("
order by ");
+
+ //don't use the order index if it is contained within a
larger statement(assuming
+ //a statement with non matching parenthesis is part of a
larger block)
+ if (orderIndex < 0 || !
HasMatchingParens(queryString.Substring(orderIndex).ToString()))
+ {
+ // Use order by first column if no explicit ordering
is provided
+ result.Add( " ORDER BY ")
+ .Add("1");
+ }
+
+ result.Add(" OFFSET ")
+ .Add(offset ?? new SqlString("0"))
+ .Add(" ROWS");
+
+ if (limit != null)
+ {
+ result.Add(" FETCH FIRST ").Add(limit).Add(" ROWS
ONLY");
+ }
+
+ return result.ToSqlString();
+ }
+ }
+}
\ No newline at end of file


NHibernate.csproj:
@@ -132,10 +132,11 @@
<Compile Include="Connection\ConnectionProviderFactory.cs" />
<Compile Include="Connection\DriverConnectionProvider.cs" />
<Compile Include="Connection\IConnectionProvider.cs" />
<Compile Include="Connection\UserSuppliedConnectionProvider.cs" /
>
<Compile Include="Criterion\IEnhancedProjection.cs" />
+ <Compile Include="Dialect\MsSql2012Dialect.cs" />
<Compile Include="Dialect\DB2Dialect.cs" />
<Compile Include="Dialect\Dialect.cs" />
<Compile Include="Dialect\FirebirdDialect.cs" />
<Compile Include="Dialect\Function\AvgQueryFunctionInfo.cs" />
<Compile Include="Dialect\Function\CountQueryFunctionInfo.cs" />

[Oskar Berggren]

Do you have a reference for the changes in syntax for 2012?

Also, what are your thoughts on how this relates to this pull request?
https://github.com/nhibernate/nhibernate-core/pull/40

/Oskar

2011/12/31 CSharper <csharp...@googlemail.com>:

[CSharper]

Hi,

The link to the new ORDER BY syntax in Denali is
http://msdn.microsoft.com/en-us/library/ms188385%28v=SQL.110%29.aspx

I saw this pull request only after implementing this new
MsSql2012Dialect.

Maybe both changes could drive the NHibernate support for MS SQL
Server forward. A better parsing of SQL strings containing common
table expressions, line or block comments would be very valuable for
being able to modify it as required. But I did not look into the
detailed implementation of the pull request yet.

Even for this MsSql2012Dialect there would be the need to have better
parsing ability because currently it implements a semantic change over
the other MS SQL Server dialects: If the ORDER BY clause is not yet
there, it adds an "ORDER BY 1" meaning it sorts according to the first
column. It does so because if it's a DISTINCT query, it is not allowed
to sort by something that is not part of the SELECT list.

So a
"SELECT a, b FROM t ORDER BY CURRENT_TIMESTAMP() OFFSET x ROWS
FETCH FIRST n ROWS ONLY"
is allowed but
"SELECT DISTINCT a, b FROM t ORDER BY CURRENT_TIMESTAMP() OFFSET x
ROWS FETCH FIRST n ROWS ONLY"
is not. To get it working I added an ORDER BY 1 so it's sorted by the
"a" column.

To have the semantic unchanged, something like
"SELECT DISTINCT a, b, 1 AS __dummyorderby__ FROM t ORDER BY
__dummyorderby__ OFFSET x ROWS FETCH FIRST n ROWS ONLY"
would be needed.

I think the cases where arbitrary sorting is wanted are quite rare but
they may exist so we should keep it working: It's like "give me some
10 objects from the million-item todo list with a certain criteria but
give me the first ones you find according to your fastest execution
plan, I don't care about sorting".

On 6 Jan., 16:50, Oskar Berggren <oskar.bergg...@gmail.com> wrote:
> Do you have a reference for the changes in syntax for 2012?
>
> Also, what are your thoughts on how this relates to this pull request?https://github.com/nhibernate/nhibernate-core/pull/40
>
> /Oskar
>

> 2011/12/31 CSharper <csharper2...@googlemail.com>:

[Patrick Earl]

I don't think it's worth putting much effort into preserving the
non-ordered paging case. If there really is a need for a random 10
rows from 10 million, one could write the query in SQL. I've never
encountered a need for such a query, nor have I even heard of such a
thing before. It doesn't seem like there's much call for the ORM to
support it in an convenient manner.

Patrick Earl

[Patrick Earl]

I thought 2008 also introduced a new syntax, or was I just remembering
what I was reading about v.Next?

Patrick Earl

[Gerke Geurts]

The dialect for SQL Server 2005 currently deals with the absence of an ORDER BY statement by ordering on CURRENT_TIMESTAMP when an order is required to generate valid SQL for a paging query.

[Gerke Geurts]

As far as I am aware SQL Server 2008 did not introduce new paging syntax.

[CSharper]

Yep, that was the reason why I wrote there's a small semantic change
in the dialect.

I added that because the old way did not work for the new syntax.
Adding an ORDER BY 1 to the query leads to sorting by the first result
expression.
In the case with DISTINCT it is not possible to sort by some arbitrary
expression without modification to the SELECT list thus needing better
SQL parsing because all sort order expressions must be part of the
SELECT list in this case. The good thing about the new paging
implementation suggested above is that it's just not required to do
remarkable parsing and manipulations on the SQL string.

And I think that with the SQL generated from current SQL Server
Dialects, the DISTINCT leads to problems because the ROW_NUMBER()
function used there makes each pair of rows distinct.

So the following select that does something like those dialects leads
to duplicate type rows:
with q as (select distinct row_number() over (order by
current_timestamp) rownum, type from sys.tables)
select top 10 * from q order by rownum

So even if there are reasons for not doing this ORDER BY 1 thing or
the better SQL parsing discussed in another thread is available so an
intrusive paging implementation is not a problem any more, I would
suggest implementing a paging based on new "Denali" syntax because
then it is easy to make it obvious in the SQL string that there is no
sorting at all:
select distinct type from sys.tables
could then be converted for paging to
with q as (select distinct type, 1 as __nohibernatesorting__)
select * from q order by __nohibernatesorting__ offset 0 fetch first
10 rows only

This one avoids the row_number problem described above.

[CSharper]

I've seen that there is a jira issue now: https://nhibernate.jira.com/browse/NH-3038

And so I created a pull request: https://github.com/nhibernate/nhibernate-core/pull/78

[Gerke Geurts]

Hello all,

 

I have updated pull request https://github.com/nhibernate/nhibernate-core/pull/63 for https://nhibernate.jira.com/browse/NH-2977 with a MsSql2012Dialect implementation and associated tests that also covers https://nhibernate.jira.com/browse/NH-3038.

 

It would be great if this fix could still be considered/reviewed for NH 3.3.

 

Kind Regards,

Gerke.

[Oskar Berggren]

Hi Gerke,

Thanks for your work on this. In my opinion it feels a bit late for
this in 3.3. I realise your patches for this has been available for
some time, but unfortunately there has not been time to review and
merge it. The 3.3.0 release is long overdue already and I feel
uncomfortable in bringing in this much code without doing a new
candidate release which would delay the final release further (and all
the fixes already in there).

We have not yet discussed the release schedule for coming releases,
but I myself think we should try to release more often - perhaps a
3.3.1 in a few months time.

/Oskar

2012/3/13 Gerke Geurts <gge...@gmail.com>:

[Gerke Geurts]

Hi Oscar,

 

I can understand the rationale and wisdom of postponing any merging until post 3.3. For ourselves the timing is not that important, as we run against a private NH build. However, it makes life a lot smoother when patches ultimately make it into the trunk.

 

Many thanks,

Gerke.