Add locate support for SQLite #2392
Conversation
fredericDelaporte
changed the title
| { | ||
| // Since v5.3 | ||
| [Obsolete("Use GetReturnType method instead.")] | ||
| public IType ReturnType(IType columnType, IMapping mapping) |
There was a problem hiding this comment.
Nit (not required to change): it seems better to me to explicitly implement obsolete interface members. So when the member is dropped from the interface, its implementation cannot be forgotten. And it would also hide the obsolete member when manipulating a reference typed as the class.
There was a problem hiding this comment.
Ok. Noted for future... But I just copy/pasted it from some other LocateFunction for other dialect.
There was a problem hiding this comment.
Which was likely already released, and so had to avoid breaking changes. This one is not released, so the nit.
| { | ||
| #pragma warning disable 618 | ||
| return ReturnType(argumentTypes.FirstOrDefault(), mapping); | ||
| #pragma warning restore 618 |
There was a problem hiding this comment.
Nit continuing: this pattern is used for existing externally visible types, with an overridable obsolete member. In this case, an external derived class overriding the obsolete member would be broken without that pattern.
For a new class, there is no need for it. We could put the implementation directly here, and make the obsolete member call the non-obsolete one instead.
| var text = args[1]; | ||
| var value = args[0]; | ||
| var startIndex = args[2]; | ||
| //ifnull( |
There was a problem hiding this comment.
Other nit: I tend to favor the use of functions which are in the SQL Standard, which is the case of coalesce, supported by SQLite, and which can replace ifNull without changing anything more that the function name. I do not know with certainty if ifNull is in the standard, but I think it is not.
(I have not found any reference about it being in the standard or not. It is supported by MySql, SQLite, Big Query, but not by SQL-Server (which has isNull) nor Oracle (nvl), while they all support coalesce.)
There was a problem hiding this comment.
This is a dialect specific function implementation, so using non-standard function should be fine here.
SQLite can support locate as
instrfunction since Release 3.7.15 on 2012-12-12.locate(text, value, startIndex)is emulated via combination ofsubstrandinstr