November 22, 2011 at 8:45 am
Your fr_reverse_charIndex function can be rewritten as:
alter function [dbo].[fn_reverse_charIndex]
/*********************************************************/
/* CREATE fn_reverse_charIndex */
/* ---------------- COMMENTS ------------------------ */
/* Give the last occurrence of the character passed in */
/* ---------------- HISTORY LOG ------------------------ */
/*********************************************************/
( @char char(1), @expression varchar(255))
returns int
as
begin
ifcharindex(@char,reverse(@expression)) = 0 return 0
return len(@expression) - charindex(@char,reverse(@expression)) + 1
end
November 22, 2011 at 9:10 am
no recursion (L to R no precedence either) - needs a tally table - uses the split like logic:
-- select dbo.fnSimpleMath( '3.5 * 2.2' ) , dbo.fnSimpleMath( '3 + 8' ) , dbo.fnSimpleMath( '5' ), dbo.fnSimpleMath( '3 / 4 + 3.2 ' ) , dbo.fnSimpleMath( '1/0' )
alter function dbo.fnSimpleMath(@string varchar(max))
returns numeric(18,6)
as begin
declare @math as table ( [sign] varchar(1), [orderby] int, addby numeric(18,6), subby numeric(18,6), multby numeric(18,6), divby numeric(18,6))
insert into @math values
('+',2,1.0,0,0,0)
,('-',2,0,1.0,0,0)
,('*',1,0,0,1.0,0)
,('/',1,0,0,0,1.0)
declare @StringSep varchar(max) = replace(replace(replace(replace(@String, '+','|'), '-','|'), '/','|'), '*','|')+ '|'
declare @result numeric(18,6) = 0
select
@result = ((@result + data )* isnull(math.addby,1.0) )
+ ((@result - data )* isnull(math.subby,0) )
+ (@result * data * isnull(math.multby ,0) )
+ isnull((@result / nullif(data,0) * isnull(math.divby ,0) ),0)
from
(select
convert(numeric(18,6), SUBSTRING(@String, Numbers.N+1 ,CHARINDEX('|', @StringSep , Numbers.N+1) - Numbers.N-1)) as data
,SUBSTRING(@String, Numbers.N, 1) as symbol
FROM dbo.Tally Numbers WITH (NOLOCK)
WHERE Numbers.N <= LEN(@String)
AND SUBSTRING('|'+@StringSep, Numbers.N+1, 1)='|'
) T
left outer join @math math
on math.sign = symbol
return @result
end
I really like the xml method stated above but am unable to get it to work dynamically without an error like The argument 1 of the XML data type method "query" must be a string literal.
declare @string varchar(max) = '3.5 * 2.2'
select convert(numeric(18,6),CONVERT(varchar(max), CONVERT(xml,'').query(@string)))
November 22, 2011 at 11:37 am
I think there are several lessons that can be learned from this article, but the one that some of the responders seem to be ignoring is that most projects are built around specs and sometimes you can't change them.
For this exercise, the specs were:
1. The string to be evaluated is a column in a table
2. The only operations we need to handle are -, +, *, / and %
3. Operations are processed from left to right, with no other precedence to be presumed.
Once this has been established, there is nothing to be gained by saying that the specs are wrong or that's not how it's done in the real world. We also can't say Ben's solution is wrong since it apparently works for him. Let's face it. In the "real" world, this is an extremely non-standard way to store data. Odds are that no one outside of Ben will ever come across this problem. And it's probably a one-time thing for Ben. But, we will each come across our own unique data problems where the techniques discussed here may be of use.
What we, as readers/kibitzers, should be doing is three-fold...
First we look at Ben's solution to see if he's used any techniques that we've not used before. If so, we want to file these away in case they might be useful for us in the future.
The second thing we should do is present alternatives, so that Ben and other readers can learn something new, while staying within the specified parameters.
The third thing that I like to consider is not that the specs are wrong, but what if they change in the future? What are the likely changes that would affect the code? Is it worth coding now to allow for these possible changes? Is your code understandable enough that someone else could make the change?
The answer to these largely involves knowing your customer base to determine what is likely, deciding how difficult it would be to implement any change.
For instance,
What if they add another operator? This is probably a simple change for one or two new operators.
What if they add an operator that is more than one character? This is a little more involved, since the initial coding probably assumed each operator was a single character.
What if they do want to implement operator precedence or include parentheses? This would be a big change and likely involve a rewrite. So, this is a possibility that should be immediately addressed and since it wasn't, we assume it's beyond the scope of this exercise.
Bottom line, I thank Ben for taking the time to write the article and I hope he (and perhaps others) find some value in my reponses.
November 22, 2011 at 11:49 am
Kevin,
I appreciate your post.
I hope the other posters will understand, I don't have issue with people commenting on how to improve code. As long as they understand the requirements. It needs to be a function, it is very simple math expression, etc. So when someone suggests that I use exec, or I am not follow mathmatical precedence, it is not really helpful, etc. I suppose I really should have stated more strongly in the article that I did not include precedence and other such things.
I wish that when people post their comments, that they do it with a touch of humility instead of pridefulness and an air of "I am going to teach somebody something.".
I think we need more humility in the DBA community.
Ben
November 22, 2011 at 4:54 pm
If you wanted too, you can make this follow the order of operations (except for parentheses) with a few small changes.
Where you currently have it decide which the last operator is by separately comparing each of the four operators, instead have it always pick addition and subtraction first, then the others. Your existing code to go from left to right will handle the rest.
--figure out which operation is last
if @addIdx > @firstIdx
begin
set @firstIdx = @addIdx
set @precedent = 1
end
if @subIdx > @firstIdx
begin
set @firstIdx = @subIdx
set @precedent = 1
end
--Add restriction that addition/subtraction must be found first (and so done last)
if @multIdx > @firstIdx AND @precedent = 0
begin
set @firstIdx = @multIdx
end
if @divIdx > @firstIdx AND @precedent = 0
begin
set @firstIdx = @divIdx
end
if @modIdx > @firstIdx AND @precedent = 0
begin
set @firstIdx = @modidx
end
Then in each of your cases, be sure to run the function on both halves
select @nopr2 = dbo.fn_simplemath(@opr2)
select @nopr1 = dbo.fn_simplemath(@opr1)
November 22, 2011 at 5:01 pm
I'll start with, I meant no disrespect with my post. (Like leading with, "No offense, but..." which usually means just the opposite. Please take my comments with a grain of salt.)
However, I did not take from Ben's post that the specifications excluded order of operations for the mathematical formula. He was asked to process a dynamic simple math expression in a SELECT statement, restricted to addition, subtraction, multiplication and division.
Ben actually states:
I will pause here and acknowledge that I could have set a precedence to process certain operators first. Or I could have used brackets to control which operations should go together. I decided I didn’t really want to go there. Instead I decided I just needed a charindex that would give me the last occurrence of the operator.
I would suggest, however, that precedence is not an optional component and that without properly including it, you risk incorporating a rather large logical flaw in whatever system this finds as its home. As it's not for my customer, I clearly have no dog in the fight and am not in a position to properly evaluate the impact. Maybe it is truly not a big deal.
I do think the reverse charindex function would be useful and the approach taken was novel. Ultimately, we are also dealing with a posting complied by a very bright guy that was sent out via email subscription to a large group of very bright guys. Eliciting commentary and constructive criticism is part of the reason to post something like this. Take, for example, @negak's modification to the work Ben did to setup order of operations.
Happy coding and Ben - thanks for being willing to post and bear the communities commentary (good and bad).
-wbw
November 22, 2011 at 5:50 pm
Actually, Kevin was right. The existing spec only needed to solve one math operation. That is the column would contain either + or - or X or /
So I actually did more than needed by causing the function to process multiple operators from left to right. It is my understanding that there will never be more than one math operation in a column for my customer.
Like I stated before, I appreciate the posts that give useful ways to improve the code. I agree that the people reading this article are intelligent. I guess what bothers me sometimes in when intelligent people are motivated by trying to show others how smart they are. Presenting an improvement to the code speaks for itself. Degrading comments are not necessary.
Thanks for your post.
Ben
November 22, 2011 at 5:56 pm
Different scenario then. As I'd said in the prior post, I did not get that as part of the specs.
-wbw
November 22, 2011 at 6:16 pm
So - more for fun than for any other reason...
Here's a recursive function for Simple Math Eval supporting the four operators. (I know, I know - have a problem, use recursion and now you have two problems... plus the call depth limitation in SQL could become an issue on very involved expressions...)
CREATE FUNCTION [dbo].[fn_SimpleMathEval]
(
@expression VARCHAR(255)
)
RETURNS NUMERIC(18, 6)
AS
BEGIN
DECLARE @result NUMERIC(18, 6)
IF LEN(ISNULL(@expression, '')) = 0
SET @result = NULL
ELSE
BEGIN
SET @result = CASE WHEN CHARINDEX('+', @expression) <> 0
THEN dbo.fn_SimpleMathEval(LTRIM(RTRIM(LEFT(@expression,
CHARINDEX('+',
@expression) - 1))))
+ dbo.fn_SimpleMathEval(LTRIM(RTRIM(RIGHT(@expression,
LEN(@expression)
- CHARINDEX('+',
@expression)))))
WHEN CHARINDEX('-', @expression) <> 0
THEN dbo.fn_SimpleMathEval(LTRIM(RTRIM(LEFT(@expression,
CHARINDEX('-',
@expression) - 1))))
- dbo.fn_SimpleMathEval(LTRIM(RTRIM(RIGHT(@expression,
LEN(@expression)
- CHARINDEX('-',
@expression)))))
WHEN CHARINDEX('*', @expression) <> 0
THEN dbo.fn_SimpleMathEval(LTRIM(RTRIM(LEFT(@expression,
CHARINDEX('*',
@expression) - 1))))
* dbo.fn_SimpleMathEval(LTRIM(RTRIM(RIGHT(@expression,
LEN(@expression)
- CHARINDEX('*',
@expression)))))
WHEN CHARINDEX('/', @expression) <> 0
THEN dbo.fn_SimpleMathEval(LTRIM(RTRIM(LEFT(@expression,
CHARINDEX('/',
@expression) - 1))))
/ dbo.fn_SimpleMathEval(LTRIM(RTRIM(RIGHT(@expression,
LEN(@expression)
- CHARINDEX('/',
@expression)))))
WHEN ISNUMERIC(@expression) = 1
THEN CONVERT(NUMERIC(18, 6), @expression)
ELSE NULL
END
END
RETURN @result
END
-wbw
November 23, 2011 at 7:02 am
Although your procedure solves the expression correctly from left to right, basic math rules require that we use an order of precedence which you excluded, but probably shouldn't have. In high school algebra, we learn the pnemonic PEMDAS (Parentheses, Exponentiation, Multiplication, Division, Addition, Subtraction) for the order of precedence. These rules must be followed.
November 23, 2011 at 9:23 am
If the specification is for a single operation with an integer on either side ...
CREATE FUNCTION [SimpleMath](@expression VARCHAR(max))
RETURNS INT
AS
BEGIN
DECLARE
@position INT,
@operator CHAR(1),
@left INT,
@right INT,
@result INT
SELECT
@position = PATINDEX('%[*/+-]%', @expression),
@operator = SUBSTRING(@expression, @position, 1),
@left = CONVERT(INT, LTRIM(RTRIM(LEFT(@expression, @position - 1)))),
@right = CONVERT(INT, LTRIM(RTRIM(RIGHT(@expression, LEN(@expression) - (@position + 1))))),
@result = CASE @operator
WHEN '*' THEN @left * @right
WHEN '/' THEN @left / @right
WHEN '+' THEN @left + @right
WHEN '-' THEN @left - @right
END
RETURN @result
END
GO
;WITH [data] ([expression]) AS (
SELECT '1 + 5' UNION ALL
SELECT '10 * 4' UNION ALL
SELECT '15 / 3' UNION ALL
SELECT '100 - 25'
)
SELECT
[expression],
[result] = [dbo].[SimpleMath]([expression])
FROM [data]
Viewing 11 posts - 31 through 40 (of 40 total)
You must be logged in to reply to this topic. Login to reply