Add support for single-argument truncate to dialects that do not support it natively #1597
Conversation
* Support single parameter truncate, like round. * Add PostgreSQL truncate * Fix Sybase Anywhere round which was not supporting single parameter calls either.
| RegisterFunction("round", new RoundFunction()); | ||
| RegisterFunction("round", new RoundFunction(false)); | ||
| RegisterFunction("truncate", new RoundFunction(true)); | ||
| RegisterFunction("trunc", new RoundFunction(true)); |
There was a problem hiding this comment.
As written here, I consider supporting dialect specific names (trunc here) as a bad practice. But since there is some disagreement about this, I will not fight against it and so I am still adding it here.
| @@ -140,12 +140,13 @@ protected virtual void RegisterMathFunctions() | |||
| RegisterFunction("radians", new StandardSQLFunction("radians", NHibernateUtil.Double)); | |||
| RegisterFunction("rand", new StandardSQLFunction("rand", NHibernateUtil.Double)); | |||
| RegisterFunction("remainder", new StandardSQLFunction("remainder")); | |||
| RegisterFunction("round", new StandardSQLFunction("round")); | |||
| RegisterFunction("round", new RoundEmulatingSingleParameterFunction("round")); | |||
There was a problem hiding this comment.
Tested with an Anywhere 17, using this custom driver.
| RegisterFunction("sign", new StandardSQLFunction("sign", NHibernateUtil.Int32)); | ||
| RegisterFunction("sin", new StandardSQLFunction("sin", NHibernateUtil.Double)); | ||
| RegisterFunction("sqrt", new StandardSQLFunction("sqrt", NHibernateUtil.Double)); | ||
| RegisterFunction("tan", new StandardSQLFunction("tan", NHibernateUtil.Double)); | ||
| RegisterFunction("truncate", new StandardSQLFunction("truncate")); |
There was a problem hiding this comment.
Removed, it was an invalid registration.
| @@ -343,8 +344,6 @@ protected virtual void RegisterMiscellaneousFunctions() | |||
| RegisterFunction("transactsql", new StandardSQLFunction("transactsql", NHibernateUtil.String)); | |||
| RegisterFunction("varexists", new StandardSQLFunction("varexists", NHibernateUtil.Int32)); | |||
| RegisterFunction("watcomsql", new StandardSQLFunction("watcomsql", NHibernateUtil.String)); | |||
| RegisterFunction("truncnum", new StandardSafeSQLFunction("truncnum", 2)); | |||
| RegisterFunction("truncate", new StandardSafeSQLFunction("truncnum", 2)); | |||
There was a problem hiding this comment.
Moved to RegisterMathFunctions (and adjusted).
|
I would not say that this is a bugfix. It's rather an improvement. |
| /// Constructs a <c>RoundEmulatingSingleParameterFunction</c>. | ||
| /// </summary> | ||
| /// <param name="name">The SQL name of the round function to call.</param> | ||
| public RoundEmulatingSingleParameterFunction(string name) |
There was a problem hiding this comment.
Class is public, so you need to add a default constructor.
There was a problem hiding this comment.
For avoiding a breaking change? This class has not yet been released. It was not existing in 5.0.3.
| @@ -24,9 +35,9 @@ public class RoundEmulatingSingleParameterFunction : ISQLFunction | |||
|
|
|||
| public SqlString Render(IList args, ISessionFactoryImplementor factory) | |||
| { | |||
There was a problem hiding this comment.
I think it would be much lightweight if we rewrite this function explicitly not using the template functions
There was a problem hiding this comment.
Either I do not get what you are thinking about, or it will be slightly more CPU lightweight but with a lot more code, rewriting a SqlString generation just for this case. For the later I do not think it is worth it.
var buf = new SqlStringBuilder();
buf.Add(_name)
.Add("(");
for (var i = 0; i < args.Count; i++)
{
var arg = args[i];
if (arg is Parameter || arg is SqlString)
{
buf.AddObject(arg);
}
else
{
buf.Add(arg.ToString());
}
if (i < (args.Count - 1)) buf.Add(", ");
}
if (args.Count == 1)
buf.Add(", 0");
return buf.Add(")").ToSqlString();And this is an almost duplicate of StandardSQLFunction.
There was a problem hiding this comment.
Why would you need a loop here? You have 2 arguments.
if (args.Count != 1 && args.Count != 2) throw new ... ;
return new SqlString(
_name,
"(",
args[0],
", ",
args.Count > 1 ? args[1] : "0",
")"
)
There was a problem hiding this comment.
This would be breaking for dialects supporting more than two arguments (SQL-Server round case).
|
Sent you a PR fredericDelaporte#3 with another approach. |
hazzik
changed the title
hazzik
changed the title
This is a follow up to #1533, which had made some minimal adjustments on truncate for supporting its specific needs. Its subject was not truncate, so complete fix had to be done in another PR.