January 31, 2014 at 7:44 pm
Hi,
I have a function that accespts a string and a delimeter returns the results in a temp table. I am using the funtion for one of the columns in my view that needs be to split and display the column into different columns. The view takes for ever to run and finally it doesn't split and doesn't display in the column.
Function:
-----------------------------------
ALTER FUNCTION [dbo].[func_Split]
(
@DelimitedString varchar(8000),
@Delimiter varchar(100)
)
RETURNS @tblArray TABLE
(
DimensionID int IDENTITY(1,1), -- Array index
Dimension varchar(1000) -- Array Dimension contents
)
AS
BEGIN -- Local Variable Declarations
DECLARE @index smallint,
@Start smallint,
@DelSize smallint
SET @DelSize = LEN(@Delimiter) -- Loop through source string and add elements to destination table array
WHILE LEN(@DelimitedString) > 0
BEGIN
SET @index = CHARINDEX(@Delimiter, @DelimitedString)
IF @index = 0
BEGIN
INSERT INTO
@tblArray
(Dimension)
VALUES
(LTRIM(RTRIM(@DelimitedString)))
BREAK
END
ELSE
BEGIN
INSERT INTO
@tblArray
(Dimension)
VALUES
(LTRIM(RTRIM(SUBSTRING(@DelimitedString, 1,@Index - 1))))
SET @Start = @index + @DelSize
SET @DelimitedString = SUBSTRING(@DelimitedString, @Start , LEN(@DelimitedString) - @Start + 1)
END
END
RETURN
END
------------------------------------
View:
------------------------------------
CREATE VIEW [dbo].[ACLEDGERACCOUNTSANDFINANCIALDIMENSIONS]
AS
SELECT T1.TEXT AS TEXT,T1.LEDGERACCOUNT AS LEDGERACCOUNT,
T1.TRANSACTIONCURRENCYAMOUNT AS TRANSACTIONCURRENCYAMOUNT,
T1.ACCOUNTINGCURRENCYAMOUNT AS ACCOUNTINGCURRENCYAMOUNT,
T1.RECID AS RECID,T2.ACCOUNTINGDATE AS ACCOUNTINGDATE,
T2.SUBLEDGERVOUCHER AS SUBLEDGERVOUCHER,T2.JOURNALNUMBER AS JOURNALNUMBER,
T4.MAINACCOUNTID AS MAINACCOUNTID,T4.NAME AS ACCOUNTNAME,
(CAST ((SELECT DIMENSION FROM Dynamicsax.DBO.FUNC_SPLIT(T1.LEDGERACCOUNT, '-')
WHERE DIMENSIONID=4) AS NVARCHAR(100))) AS AREAOFLAW
FROM GENERALJOURNALACCOUNTENTRY T1 CROSS JOIN GENERALJOURNALENTRY T2 CROSS JOIN
DIMENSIONATTRIBUTEVALUECOMBINATION T3
LEFT OUTER JOIN MAINACCOUNT T4 ON (T3.MAINACCOUNT=T4.RECID)
WHERE (T1.GENERALJOURNALENTRY=T2.RECID) AND (T1.LEDGERDIMENSION=T3.RECID)
GO
------------------------------------
-----------------------------------
Here is the sample that works just fine and returns the temp table
Declare @string varchar(max);
set @string = '90032-GA-005-0511-001A';
select * from dbo.func_split(@string,'-');
-----------------------------------
Not sure what I am missing in the above view why it doesn't split the string. Can some one please help me what I am missing/doing wrong in the above view.
Thanks in advance.
Dev
January 31, 2014 at 9:25 pm
The function needs to be in the FROM clause of the overall query for the view in a CROSS JOIN or as a CROSS APPLY.
You should also pick a function that doesn't use a While Loop, Recursive CTE, XML, or Cursor if you want performance. Hopefully, you don't actually have the need for more than a 1 character delimiter nor need to split a string of more than 8K. If you do, use the CLR cited in the article. Please see the following article.
http://www.sqlservercentral.com/articles/Tally+Table/72993/
--Jeff Moden
Change is inevitable... Change for the better is not.
February 1, 2014 at 12:45 am
i can vouch for the performance of Jeff's DelimitedSplit8K function. I know the article contains performance benchmarking that looks too good to be true, but it is true. I have this function in a shared database of functions used by others in production and the performance is truly awesome.
As a word of caution, once you move from a function using a WHILE look to a set-based approach, it will change your expectations of good performance. 😉
February 1, 2014 at 8:12 am
You're too kind, Ed. :blush: Thanks for the kudo.
--Jeff Moden
Change is inevitable... Change for the better is not.
February 1, 2014 at 1:18 pm
Hi Jeff,
Thank you for your reply. I am not that expert in t-sql and more of a front-end guy. Can you please help me modify my query based on your inputs? That would be very helpful.
Thank you again!
Dev
February 1, 2014 at 2:53 pm
Sure but the price you have to pay is to listen to a lecture first. 😉 Don't be offended by any of this. Rather, think of it as a bit of "tough love advice" to a fellow programmer just getting started in the art of writing T-SQL that will keep your butt out of sling.
The first letter of "SQL" stands for "Structured". Your code is mostly unreadable simply because you haven't structured it. Not everything should be in all upper case and certainly not in all lower case. The structure that I've applied is...
All SQL Keywords are in upper case.
All schema names and aliases are in lower case.
Just about everything else is in mixed case where the first letter of each word is capitalized.
ALWAYS use the 2 part naming convention. Never rely on 1 part naming and, if you need 3 or 4 part naming, create a synonym so that you can stick with 2 part naming. That way, if the database or server location changes, you only need to change synonyms instead of having to find all code that needs to be changed.
None of the columns in the SELECT list changed names so there's no need to clutter your code with a column alias for each.
Learn how to do joins properly. You had 3 CROSS JOINED tables and then used non-ANSI joins in your WHERE clause to turn them into INNER JOINs (JOIN in the code that follows). That's not only confusing as hell, but it makes your code even more unreadable/easy to understand. With the understanding that I don't have your data to test with, it appears that the CROSS JOIN you had between t2 and t3 was NOT converted to an INNER JOIN by any of your criteria (you couldn't tell because the code is so hard to read) and that spawned a Cartesian Product between t2 and t3, which takes forever to resolve and returns many undesirable false rows.
It takes virtually no time to copy and paste a header in at the beginning of the code and scribble in some simple facts. Be kind to the next person that has to use your code and, remember, that next person might be YOU or, worse yet, the guy who writes your evals for raises or continued employment.
My rule of thumb is that EVERY Insert, Update, Delete, and Select has a business reason for being in the code. A short comment on every one of those statements, even on sub-queries, will make troubleshooting and maintenance of code a thousand times easier that having to read all of the code, no matter how short it is, to figure out what it does. Don't use comments like "UPDATE the Customer" table. Anyone can see that in the code. Instead, explain WHY the Customer table is being updated. Something like "Give all qualifying customers a calculated discount" goes a long way in reducing troubleshooting/maintenance times and makes it much less likely that someone will change the wrong section of code.
With all that in mind, here's the way I would have written the code. And, yes, you need to get a copy of the dbo.DelimitedSplit8k function as previously recommended if you're truly concerned about performance, scalability, and resource usage. Again, I don't have your data to test with so I make no guarantee that it's 100% correct but it should get you much closer.
CREATE VIEW dbo.ACLedgerAccountsAndFinancialDimensions
/**********************************************************************************************
Purpose:
put a description of what this view does here.
Revision History:
Rev 00 - 01 Feb 2014 - put your name here
- Initial creation and unit test.
**********************************************************************************************/
AS
SELECT t1.[Text]
,t1.LedgerAccount
,t1.TransactionCurrencyAmount
,t1.AccountingCurrencyAmount
,t1.RecID
,t2.AccountingDATE
,t2.SubLedgerVoucher
,t2.JournalNUMBER
,t4.MainAccountID
,t4.Name
,t5.AreaOfLaw
FROM dbo.GeneralJournalAccountEntry t1
JOIN dbo.GeneralJournalEntry t2 ON t2.RecID = t1.GeneralJournalEntry
JOIN dbo. DimensionAttributeValueCombination t3 ON t3.RecID = t1.LedgerDimension
LEFT JOIN dbo.MainAccount t4 ON t4.RecID = t3.MainAccount
CROSS APPLY ( --==== Finds the 4th item in the LedgerAccount number for "AreaOfLaw"
SELECT Item
FROM dbo.DelimitedSplit8K(t1.LedgerAccount,'-')
WHERE ItemNumber = 4
) t5 (AreaOfLaw)
;
As a bit of a sidebar, the "AreaOfLaw" column is a calculated column. If someone uses this view in a SELECT that has criteria against that column, the entire view will need to materialize first and that will cause HUGE performance problems down the ways. This is part of the reason why you shouldn't use Views for this type of code encapsulation. No one looks at this type of stuff until it does become a huge performance problem. And we all know exactly when that will happen... when you can least afford it to.
--Jeff Moden
Change is inevitable... Change for the better is not.
February 4, 2014 at 2:36 pm
Hi Jeff,
Thank you for taking time to help me with some basic stuff. I do need to get my head around with more T-SQL coding. We use ERP system and the views that were devloped in the ERP gets synchronized and it doesn't do good formatting in the SQL Server. Your DelimitedSplit8K funcion was very helpful and returns the data (around 2 million records) relatively way quickly. I modified my View using this method and this is how it looks and seems working as expected. I also tried using the CROSS APPLY as you suggested but the below version returns rows relatively quicker. Please do reply if this can be written even better. Thank you again!
CREATE VIEW dbo.LedgerAccountsAndFinancialDimensions
/**********************************************************************************************
Purpose:
put a description of what this view does here.
Revision History:
Rev 00 - 01 Feb 2014 - put your name here
- Initial creation and unit test.
**********************************************************************************************/
AS
SELECT t1.[Text]
,t1.LedgerAccount
,t1.TransactionCurrencyAmount
,t1.AccountingCurrencyAmount
,t1.RecID
,t2.AccountingDATE
,t2.SubLedgerVoucher
,t2.JournalNUMBER
,t4.MainAccountID
,t4.Name
,t5.AreaOfLaw
,(CAST ((SELECT Item FROM dbo.DelimitedSplit8K(t1.LEDGERACCOUNT, '-') WHERE ItemNumber=4) AS NVARCHAR(100))) AS AREAOFLAW
,(CAST ((SELECT Item FROM dbo.DelimitedSplit8K(t1.LEDGERACCOUNT, '-') WHERE ItemNumber=2) AS NVARCHAR(10))) AS DIVISION
,(CAST ((SELECT Item FROM dbo.DelimitedSplit8K(t1.LEDGERACCOUNT, '-') WHERE ItemNumber=3) AS NVARCHAR(10))) AS Department
,(CAST ((SELECT Item FROM dbo.DelimitedSplit8K(t1.LEDGERACCOUNT, '-') WHERE ItemNumber=5) AS NVARCHAR(10)))+'-'+
(CAST ((SELECT Item FROM dbo.DelimitedSplit8K(t1.LEDGERACCOUNT, '-') WHERE ItemNumber=6) AS NVARCHAR(10))) AS Project
FROM dbo.GeneralJournalAccountEntry t1
JOIN dbo.GeneralJournalEntry t2 ON t2.RecID = t1.GeneralJournalEntry
JOIN dbo. DimensionAttributeValueCombination t3 ON t3.RecID = t1.LedgerDimension
LEFT JOIN dbo.MainAccount t4 ON t4.RecID = t3.MainAccount
;
February 5, 2014 at 10:55 am
movvap (2/4/2014)
Hi Jeff,Thank you for taking time to help me with some basic stuff. I do need to get my head around with more T-SQL coding. We use ERP system and the views that were devloped in the ERP gets synchronized and it doesn't do good formatting in the SQL Server. Your DelimitedSplit8K funcion was very helpful and returns the data (around 2 million records) relatively way quickly. I modified my View using this method and this is how it looks and seems working as expected. I also tried using the CROSS APPLY as you suggested but the below version returns rows relatively quicker. Please do reply if this can be written even better. Thank you again!
CREATE VIEW dbo.LedgerAccountsAndFinancialDimensions
/**********************************************************************************************
Purpose:
put a description of what this view does here.
Revision History:
Rev 00 - 01 Feb 2014 - put your name here
- Initial creation and unit test.
**********************************************************************************************/
AS
SELECT t1.[Text]
,t1.LedgerAccount
,t1.TransactionCurrencyAmount
,t1.AccountingCurrencyAmount
,t1.RecID
,t2.AccountingDATE
,t2.SubLedgerVoucher
,t2.JournalNUMBER
,t4.MainAccountID
,t4.Name
,t5.AreaOfLaw
,(CAST ((SELECT Item FROM dbo.DelimitedSplit8K(t1.LEDGERACCOUNT, '-') WHERE ItemNumber=4) AS NVARCHAR(100))) AS AREAOFLAW
,(CAST ((SELECT Item FROM dbo.DelimitedSplit8K(t1.LEDGERACCOUNT, '-') WHERE ItemNumber=2) AS NVARCHAR(10))) AS DIVISION
,(CAST ((SELECT Item FROM dbo.DelimitedSplit8K(t1.LEDGERACCOUNT, '-') WHERE ItemNumber=3) AS NVARCHAR(10))) AS Department
,(CAST ((SELECT Item FROM dbo.DelimitedSplit8K(t1.LEDGERACCOUNT, '-') WHERE ItemNumber=5) AS NVARCHAR(10)))+'-'+
(CAST ((SELECT Item FROM dbo.DelimitedSplit8K(t1.LEDGERACCOUNT, '-') WHERE ItemNumber=6) AS NVARCHAR(10))) AS Project
FROM dbo.GeneralJournalAccountEntry t1
JOIN dbo.GeneralJournalEntry t2 ON t2.RecID = t1.GeneralJournalEntry
JOIN dbo. DimensionAttributeValueCombination t3 ON t3.RecID = t1.LedgerDimension
LEFT JOIN dbo.MainAccount t4 ON t4.RecID = t3.MainAccount
;
Interesting on the performance thing. I'm going to have to do some testing in that area. Thanks a heap for the feedback.
--Jeff Moden
Change is inevitable... Change for the better is not.
February 5, 2014 at 12:01 pm
I have nothing to test, but a single call to the function might work better.
SELECT t1.[Text]
,t1.LedgerAccount
,t1.TransactionCurrencyAmount
,t1.AccountingCurrencyAmount
,t1.RecID
,t2.AccountingDATE
,t2.SubLedgerVoucher
,t2.JournalNUMBER
,t4.MainAccountID
,t4.Name
,t5.AreaOfLaw
,(CAST t5.AREAOFLAW AS NVARCHAR(100)) AS AREAOFLAW
,(CAST t5.DIVISION AS NVARCHAR(10)) AS DIVISION
,(CAST t5.Department AS NVARCHAR(10)) AS Department
,(CAST t5.Project AS NVARCHAR(20)) AS Project
FROM dbo.GeneralJournalAccountEntry t1
JOIN dbo.GeneralJournalEntry t2 ON t2.RecID = t1.GeneralJournalEntry
JOIN dbo. DimensionAttributeValueCombination t3 ON t3.RecID = t1.LedgerDimension
LEFT JOIN dbo.MainAccount t4 ON t4.RecID = t3.MainAccount
CROSS APPLY( SELECT MAX( CASE WHEN ItemNumber = 2 THEN Item END) DIVISION,
MAX( CASE WHEN ItemNumber = 3 THEN Item END) Department,
MAX( CASE WHEN ItemNumber = 4 THEN Item END) AREAOFLAW,
MAX( CASE WHEN ItemNumber = 5 THEN Item END) +
MAX( CASE WHEN ItemNumber = 6 THEN Item END) Project
FROM dbo.DelimitedSplit8K(t1.LEDGERACCOUNT, '-')
WHERE ItemNumber BETWEEN 2 AND 6) t5
Viewing 9 posts - 1 through 8 (of 8 total)
You must be logged in to reply to this topic. Login to reply