October 18, 2011 at 8:35 am
I've created a simple TVF (based on Jeff Moden's posts) to shred delimited strings to their constituent parts. (Background: this is to deal with SSRS multi-value parameters being unable to deal with multiple integer values for a parameter, but that's not really needed for this problem.)
Goal: take a CSV string like '12,333,543' and return one row for each element in the string.
Problem:
If I run code interactively, I correctly get 3 rows returned for the following input string: '65,66,117'. I get one row each for 65, 66, and 117. That's good.
If I create a TVF using the same code and input string, I get 5 rows returned. I get 2 add'l rows, each with a blank result.
I've been through this code many times, and I cannot see the flaw. Can anyone see what I'm missing here?
Thanks,
Rich
USE dev;
GO
--1.Interactive version: correctly returns 3 rows
DECLARE @DelimitedList NVARCHAR(4000), @delim CHAR(1);
SET @DelimitedList = N'65,66,117';
SET @delim = ',';
--Put @delim fore n aft to deal with single elements:
SET @DelimitedList = @delim + @DelimitedList + @delim;
WITH cteNumbers As
(
SELECT 1 Num
UNION ALL
SELECT Num + 1 FROM cteNumbers WHERE Num < 100
)
SELECT
SUBSTRING(@DelimitedList, Num+1, CHARINDEX(@delim, @DelimitedList, Num+1) - Num - 1) As Element
FROM cteNumbers
WHERE Num < LEN(@DelimitedList) AND--Don't include the last comma
SUBSTRING(@DelimitedList,Num,1) = @delim;
--2. TVF: Incorrectly returns an additional 2 rows (5 total).
--Create the TVF, using identical code as above....
GO
IF OBJECT_ID('udfShredList', 'TF') IS NOT NULL DROP FUNCTION udfShredList;
GO
CREATE FUNCTION udfShredList(@DelimitedList NVARCHAR(4000), @delim CHAR(1)= ',')
RETURNS @Parameters TABLE (Element NVARCHAR(1000))AS
BEGIN
SET @DelimitedList = @delim + @DelimitedList + @delim;
WITH cteNumbers As
(
SELECT 1 Num
UNION ALL
SELECT Num + 1 FROM cteNumbers WHERE Num < 100
)
INSERT INTO @Parameters
(Element)
SELECT
SUBSTRING(@DelimitedList, Num+1, CHARINDEX(@delim, @DelimitedList, Num+1) - Num - 1) As Element
FROM cteNumbers
WHERE Num < LEN(@DelimitedList) AND--Don't include the last comma
SUBSTRING(@DelimitedList,Num,1) = @delim;
RETURN
END
GO
--... and test it using the same inputs as above
DECLARE @DelimitedList NVARCHAR(4000), @delim CHAR(1);
SET @DelimitedList = N'65,66,117';
SET @delim = ',';
--Put @delim fore n aft to deal with single elements:
SET @DelimitedList = @delim + @DelimitedList + @delim;
SELECT *
FROM udfShredList(@DelimitedList, @delim);
October 18, 2011 at 8:41 am
rmechaber (10/18/2011)
I've created a simple TVF (based on Jeff Moden's posts) to shred delimited strings to their constituent parts. (Background: this is to deal with SSRS multi-value parameters being unable to deal with multiple integer values for a parameter, but that's not really needed for this problem.)Goal: take a CSV string like '12,333,543' and return one row for each element in the string.
Problem:
If I run code interactively, I correctly get 3 rows returned for the following input string: '65,66,117'. I get one row each for 65, 66, and 117. That's good.
If I create a TVF using the same code and input string, I get 5 rows returned. I get 2 add'l rows, each with a blank result.
I've been through this code many times, and I cannot see the flaw. Can anyone see what I'm missing here?
Thanks,
Rich
USE dev;GO
--1.Interactive version: correctly returns 3 rows
DECLARE @DelimitedList NVARCHAR(4000), @delim CHAR(1);
SET @DelimitedList = N'65,66,117';
SET @delim = ',';
--Put @delim fore n aft to deal with single elements:
SET @DelimitedList = @delim + @DelimitedList + @delim;
WITH cteNumbers As
(
SELECT 1 Num
UNION ALL
SELECT Num + 1 FROM cteNumbers WHERE Num < 100
)
SELECT
SUBSTRING(@DelimitedList, Num+1, CHARINDEX(@delim, @DelimitedList, Num+1) - Num - 1) As Element
FROM cteNumbers
WHERE Num < LEN(@DelimitedList) AND--Don't include the last comma
SUBSTRING(@DelimitedList,Num,1) = @delim;
--2. TVF: Incorrectly returns an additional 2 rows (5 total).
--Create the TVF, using identical code as above....
GO
IF OBJECT_ID('udfShredList', 'TF') IS NOT NULL DROP FUNCTION udfShredList;
GO
CREATE FUNCTION udfShredList(@DelimitedList NVARCHAR(4000), @delim CHAR(1)= ',')
RETURNS @Parameters TABLE (Element NVARCHAR(1000))AS
BEGIN
SET @DelimitedList = @delim + @DelimitedList + @delim;
WITH cteNumbers As
(
SELECT 1 Num
UNION ALL
SELECT Num + 1 FROM cteNumbers WHERE Num < 100
)
INSERT INTO @Parameters
(Element)
SELECT
SUBSTRING(@DelimitedList, Num+1, CHARINDEX(@delim, @DelimitedList, Num+1) - Num - 1) As Element
FROM cteNumbers
WHERE Num < LEN(@DelimitedList) AND--Don't include the last comma
SUBSTRING(@DelimitedList,Num,1) = @delim;
RETURN
END
GO
--... and test it using the same inputs as above
DECLARE @DelimitedList NVARCHAR(4000), @delim CHAR(1);
SET @DelimitedList = N'65,66,117';
SET @delim = ',';
--Put @delim fore n aft to deal with single elements:
SET @DelimitedList = @delim + @DelimitedList + @delim;
SELECT *
FROM udfShredList(@DelimitedList, @delim);
Your first set statement in the function is causing the problems:
SET @DelimitedList = @delim + @DelimitedList + @delim;
comment that out and the function should work fine.
October 18, 2011 at 9:43 am
Put @delim fore n aft to deal with single elements:
SET @DelimitedList = @delim + @DelimitedList + @delim;
Either outside the function or within it, not both, which creates a new list element fore and aft
For fast, accurate and documented assistance in answering your questions, please read this article.
Understanding and using APPLY, (I) and (II) Paul White
Hidden RBAR: Triangular Joins / The "Numbers" or "Tally" Table: What it is and how it replaces a loop Jeff Moden
October 18, 2011 at 9:44 am
bkubicek (10/18/2011)
Your first set statement in the function is causing the problems:SET @DelimitedList = @delim + @DelimitedList + @delim;
comment that out and the function should work fine.
Not quite, but I did repeat that line outside the TVF, which resulted in double-modification of the original string! :crying:
For some reason, I couldn't see the double entry until you pointed out that line to me. Thanks very much for helping me find the problem!
Rich
October 18, 2011 at 9:46 am
Chris, thanks to you too, you nailed it. Dunno why I couldn't see that before....
Rich
October 18, 2011 at 10:18 am
We've all been there, Rich 😉
For fast, accurate and documented assistance in answering your questions, please read this article.
Understanding and using APPLY, (I) and (II) Paul White
Hidden RBAR: Triangular Joins / The "Numbers" or "Tally" Table: What it is and how it replaces a loop Jeff Moden
October 18, 2011 at 10:22 pm
[font="Arial Black"]Danger!!!! DO NOT USE RECURSIVE CTES FOR COUNTING!!!! PERIOD!!!![/font]
See the following article for why...
http://www.sqlservercentral.com/articles/T-SQL/74118/
[font="Arial Black"]Danger!!!! DO NOT USE ANY FORM OF CONCATENATION WHEN SHREDDING CSVS!!!! PERIOD!!!![/font]
See the following article for why...
http://www.sqlservercentral.com/articles/Tally+Table/72993/
[font="Arial Black"]Danger!!!! DO NOT JEFF MODEN'S NAME IF YOU DO USE ONE OF THE ABOVE!!!! PERIOD!!!![/font]
See the Jeff Moden book on "High Velocity Porkchop Launchers" for the reasons why. :-P;-)
--Jeff Moden
Change is inevitable... Change for the better is not.
October 19, 2011 at 6:09 am
Geez, Jeff: this was example code for posting to SSC only, and not the full text of what I was actually using.
In particular, the recursive CTE was junk code to post so that I would have complete code for others to test, without assuming the existence of a tally table on their servers. You know, "post complete code" and all that?
I fear no pork chop.
Rich
October 19, 2011 at 7:48 am
Jeff, if you were anyone else I'd just shrug off your emphatic reply as overreaction and leave it at that.
You have been a great help to me and literally thousands of others, so I'm taking time to reply more fully.
First, I don't mean (and didn't say) that my code was taken from yours, only that it was based on yours. By stating this, I didn't mean to appropriate your name, only to give credit where credit was due for the algorithm used to parse strings. I believe you've called it the inch-worm technique. I didn't want to take credit for being clever enough to have devised the CHARINDEX bit. Fair enough?
Second, the recursive CTE for a small tally table bit: this was only for posting to the SSC forum, and in NO way is production or even development code for me. I did it as a fast way to meet SSC's stated goal for members to post complete code that others can quickly use to test. That's it. I didn't recommend it for others to use, and I certainly didn't mean to suggest this was code you had written.
Finally, I don't get why you needed to reply in inch-high letters. My problem was solved by others and had nothing to do with any of your complaints about my posting: I was blind not to see that I'd double-coded something both in the TVF and in the query, but that's what sometimes happens. At least, that happens to me sometimes. Maybe if I wrote my T-SQL in inch-high letters, I wouldn't miss these errors? 😉
I understand that errors and "recommendations" posted here often get repeated elsewhere, and that you are rightly eager to prevent bad code from propagating in your good name. I think if you re-read what I posted, I neither said "this is Jeff's code, please use it" nor did I say "this is production code and an excellent approach to the problem." If others want to take what I posted and (mis)use it without further consideration, I can't help that. This is a forum exchange, not a peer-reviewed article.
Tough to be publicly excoriated by a god, but that's my brouhaha for today I guess.
Back on my head,
Rich
November 24, 2011 at 8:45 am
Heh... Sorry for the inch high letters and the late reply, Rich. I'm just getting caught up on some old posts.
I apparently took what you said the wrong way but that, in my own feeble mind, is a part of the reason for my outburst. If I took it wrong, others may, as well.
Anyway, thank you for taking the time to write the feedback you did. It really shows that you have your mind in the right place and the good person that you really are. Thanks, Rich.
--Jeff Moden
Change is inevitable... Change for the better is not.
November 24, 2011 at 9:34 pm
Thanks for the kind reply and apology Jeff, I appreciate it.
I'm glad I took the time earlier to explain why I didn't think I'd misappropriated or misrepresented your code. It's important to me not to claim credit for work others create, which is really the only reason I even mentioned your name in my original posting. I'll certainly try to re-read my posts to avoid this kind of misunderstanding again.
Guess I can remove my Binford 2000, laser-enhanced, Pork-Chop-Repelling armor now? 😀
Yours,
Rich
P.S. Your initial reply referenced a 'Jeff Moden book on "High Velocity Porkchop Launchers"', yet I saw no link to this Amazon bestseller. Care to explain where we can find this?
November 26, 2011 at 8:36 am
rmechaber (11/24/2011)
P.S. Your initial reply referenced a 'Jeff Moden book on "High Velocity Porkchop Launchers"', yet I saw no link to this Amazon bestseller. Care to explain where we can find this?
BWAA-HAAA!!! It's a "work in progress"... as you've just seen, one of my launchers fired when it shouldn't have. 😀
--Jeff Moden
Change is inevitable... Change for the better is not.
Viewing 12 posts - 1 through 11 (of 11 total)
You must be logged in to reply to this topic. Login to reply