Skip to content

Fixes with prepare_sql #710

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Oct 10, 2017
Merged

Fixes with prepare_sql #710

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6655c2e
NH-4091 - Cease specifying parameters size for avoiding out of memory
fredericDelaporte Oct 9, 2017
6cfb2a2
NH-4090 - Set time parameter size for Sql Server
fredericDelaporte Oct 9, 2017
5c01806
Restrict a test to Sql Server
fredericDelaporte Oct 9, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 8 additions & 5 deletions src/NHibernate.Test/Async/NHSpecificTest/SqlConverterAndMultiQuery/Fixture.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@


using NHibernate.Cfg;
using NHibernate.Dialect;
using NHibernate.Driver;
using NHibernate.Engine;
using NUnit.Framework;

namespace NHibernate.Test.NHSpecificTest.SqlConverterAndMultiQuery
Expand All @@ -25,11 +26,13 @@ protected override void Configure(Configuration configuration)
configuration.DataBaseIntegration(x => x.ExceptionConverter<SqlConverter>());
}

protected override bool AppliesTo(Dialect.Dialect dialect)
protected override bool AppliesTo(ISessionFactoryImplementor factory)
{
// MsSqlCe throws InvalidOperationException instead of a DbException for these tests, preventing
// the test SqlConverter to do its job.
return !(Dialect is MsSqlCeDialect);
// Test current implementation allows to test mmostly SQL Server. Other databases
// tend to (validly) send InvalidOperationException during prepare phase due to the closed
// connection, which get not converted. For testing other case, maybe a failure caused by a
// schema mismatch (like done in transaction tests) would be better.
return factory.ConnectionProvider.Driver is SqlClientDriver;
}

[Test]
Expand Down
13 changes: 8 additions & 5 deletions src/NHibernate.Test/NHSpecificTest/SqlConverterAndMultiQuery/Fixture.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using NHibernate.Cfg;
using NHibernate.Dialect;
using NHibernate.Driver;
using NHibernate.Engine;
using NUnit.Framework;

namespace NHibernate.Test.NHSpecificTest.SqlConverterAndMultiQuery
Expand All @@ -14,11 +15,13 @@ protected override void Configure(Configuration configuration)
configuration.DataBaseIntegration(x => x.ExceptionConverter<SqlConverter>());
}

protected override bool AppliesTo(Dialect.Dialect dialect)
protected override bool AppliesTo(ISessionFactoryImplementor factory)
{
// MsSqlCe throws InvalidOperationException instead of a DbException for these tests, preventing
// the test SqlConverter to do its job.
return !(Dialect is MsSqlCeDialect);
// Test current implementation allows to test mmostly SQL Server. Other databases
// tend to (validly) send InvalidOperationException during prepare phase due to the closed
// connection, which get not converted. For testing other case, maybe a failure caused by a
// schema mismatch (like done in transaction tests) would be better.
return factory.ConnectionProvider.Driver is SqlClientDriver;
}

[Test]
Expand Down
4 changes: 3 additions & 1 deletion src/NHibernate/Driver/Sql2008ClientDriver.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,19 +2,21 @@
using System.Data;
using System.Data.Common;
using System.Data.SqlClient;
using System.Linq;

namespace NHibernate.Driver
{
public class Sql2008ClientDriver : SqlClientDriver
{
const byte MaxTime = 5;

protected override void InitializeParameter(DbParameter dbParam, string name, SqlTypes.SqlType sqlType)
{
base.InitializeParameter(dbParam, name, sqlType);
switch (sqlType.DbType)
{
case DbType.Time:
((SqlParameter) dbParam).SqlDbType = SqlDbType.Time;
dbParam.Size = MaxTime;
break;
case DbType.Date:
((SqlParameter) dbParam).SqlDbType = SqlDbType.Date;
Expand Down
1 change: 0 additions & 1 deletion src/NHibernate/Driver/SqlClientDriver.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,6 @@ protected override void InitializeParameter(DbParameter dbParam, string name, Sq
SetVariableLengthParameterSize(dbParam, sqlType);
}

// Used from SqlServerCeDriver as well
public static void SetVariableLengthParameterSize(DbParameter dbParam, SqlType sqlType)
{
SetDefaultParameterSize(dbParam, sqlType);
Expand Down
8 changes: 0 additions & 8 deletions src/NHibernate/Driver/SqlServerCeDriver.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@
using System.Data.Common;
using System.Reflection;
using NHibernate.SqlTypes;
using NHibernate.Util;
using Environment = NHibernate.Cfg.Environment;

namespace NHibernate.Driver
{
Expand All @@ -25,13 +23,11 @@ public SqlServerCeDriver()
{
}

private bool prepareSql;
private PropertyInfo dbParamSqlDbTypeProperty;

public override void Configure(IDictionary<string, string> settings)
{
base.Configure(settings);
prepareSql = PropertiesHelper.GetBoolean(Environment.PrepareSql, settings, false);

using (var cmd = CreateCommand())
{
Expand Down Expand Up @@ -102,10 +98,6 @@ protected override void InitializeParameter(DbParameter dbParam, string name, Sq
base.InitializeParameter(dbParam, name, AdjustSqlType(sqlType));

AdjustDbParamTypeForLargeObjects(dbParam, sqlType);
if (prepareSql)
{
SqlClientDriver.SetVariableLengthParameterSize(dbParam, sqlType);
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think of this removal? It works but is in contradiction with documentation:

Before you call Prepare, specify the data type of each parameter in the statement to be prepared. For each parameter that has a variable-length data type, you must set the Size property to the maximum size needed. Prepare returns an error if these conditions are not met.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is strange that it works.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I dislike it quite. But that is the simplest change to get it work... Otherwise I could move the code from SqlClientDriver into Sql CE driver and adjust it to its needs, meaning removing 2008 stuff and not specifying blob sizes, but letting all the other been set, since it will be more compliant with doc and should work too.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think that SqlServer CE requires having all parameter sizes to be set when Prepare is used.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So we just keep that change as is. This PR could then be merged after dropping the first commit.

}

private static SqlType AdjustSqlType(SqlType sqlType)
Expand Down