May 3, 2013 at 5:04 pm
My biggest concern is the conversion to FLOAT to do the sum. FLOAT only has a precision of 15 significant digits. If you go any higher than that, you will start to get siginficant rounding errors without any warning.
Use the appropriate datatype for these types of things. For most sums, FLOAT isn't it.
--Jeff Moden
Change is inevitable... Change for the better is not.
May 5, 2013 at 7:16 pm
Jeff Moden (5/3/2013)
My biggest concern is the conversion to FLOAT to do the sum. FLOAT only has a precision of 15 significant digits. If you go any higher than that, you will start to get siginficant rounding errors without any warning.Use the appropriate datatype for these types of things. For most sums, FLOAT isn't it.
Is it?
Really?
How about this one:
SELECT ROUND(SUM(CONVERT(BIGINT, FPSTemp.amount))/100,2)
?
Would make a good QoD: How many errors can you find in this piece of code?
:hehe:
_____________
Code for TallyGenerator
May 5, 2013 at 7:38 pm
niladri.primalink,
this one does not look right:
(SELECT TOP 1 ISNULL(status,0)status FROM submission WHERE fileid=file_id AND file_type=1)sub_status
TOP 1 with no ORDER BY means "random".
If there are 2 or more statuses for any particular file_id the result might be different every single time it's executed.
What do you need from this query?
_____________
Code for TallyGenerator
May 6, 2013 at 12:05 am
Hi niladri.primalink,
Using correlated subqueries (especially so many of them) is as bad as having an open bottle of rum on the front seat of your car or carrying a gun openly on a street.
Lack of enforcement from the police (probably because they can't read SQL very well) does not make it less criminal.
😀
I've tried to fix you query by moving subqueries to derived tables.
Some might suggest CTE - it's up to you which style to choose.
CTE might be the perfect choice for selecting "submission status" - if you can tell which one you need to select for each particular file_id.
I did not have table definitions and test data to test the script, so please do this part yourself - there might be some errors.
But anyway this might give you an idea how it should be done:
SELECT fb.{/*list pf columns here*/}
ISNULL(UL.USER_NAME,'')created_by,
ISNULL (ft.totcreditamount, FTE.totcreditamount) totcreditamount,
ISNULL (ft.totdebitamount, FTE.totdebitamount) totdebitamount,
FROM filebasic fb
LEFT JOIN userlogin UL ON UL.user_id=fb.createdby
LEFT JOIN (
SELECT fileid, MAX(status))status
FROM submission
WHERE file_type=1
GROUP BY fileid )sub_status ON fileid = file_id
LEFT JOIN (
SELECT fileid,
CONVERT (DECIMAL(13,2), SUM(CONVERT(FLOAT, CASE WHEN transactioncode IN ('99', 'Z4', 'Z5') THEN amount ELSE 0.00 END) )/100.00 ) totcreditamount,
CONVERT (DECIMAL(13,2), SUM(CONVERT(FLOAT, CASE WHEN transactioncode IN ('01', '17', '18', '19') THEN amount ELSE 0.00 END) )/100.00 ) totdebitamount
FROM filetransaction
WHERE iscontra=0 AND ISNUMERIC(amount + '.0e0') = 1
GROUP BY fileid
) ft ON ft.fileid=fb.file_id
LEFT JOIN (
SELECT FPSTemp.fileid,
CONVERT(DECIMAL(13,2), SUM(CASE WHEN mt.transaction_code IN ('99', 'Z4', 'Z5') THEN FPSTemp.amount ELSE 0 END )/100.00 ) totcreditamount,
CONVERT(DECIMAL(13,2), SUM(CASE WHEN mt.transaction_code IN ('01', '17', '18', '19') THEN FPSTemp.amount ELSE 0 END )/100.00 ) totdebitamount
FROM filetransaction_excel FPSTemp
INNER JOIN m_transactiontype mt ON mt.name = FPSTemp.transaction_code
WHERE ISNUMERIC(FPSTemp.amount + '.0e0') = 1 AND FPSTemp.amount IS NOT NULL
GROUP BY FPSTemp.fileid
) FTE ON FTE.fileid= fb.file_id
WHERE company_id=@company_id
AND status=0 AND createdby = @INT_USERID
ORDER BY file_id DESC
I have doubts about use of table m_transactiontype in the last derived table.
It seems that "mt.transaction_code" must be replaced with "FPSTemp.transaction_code" and the INNER JOIN removed completely, as columns from the table are not used neither for SELECT not for WHERE clauses.
_____________
Code for TallyGenerator
May 6, 2013 at 6:23 am
This is from my Common TSQL Mistakes SQL Saturday session, which I have presented over 50 times now at various venues. Hopefully you can adapt it to your needs (if you haven't already found another solution like simply removing the unwanted data or adding a new column that is just numeric, etc.
KEY TAKEAWAYS
1) Remember that the optimizer can do almost anything it wants with your query as long as algebraically and logically it gives you the same output/effect. I note this may not be the effect you desire! 🙂
2) Learn about and use the power of the CASE keyword!
3) Try to not architect multi-variant columns! 😉
Use tempdb
set nocount on
go
IF OBJECT_ID(N'Accounts', N'U') IS NOT NULL
DROP TABLE dbo.Accounts;
CREATE TABLE dbo.Accounts (
account_nbr INT NOT NULL PRIMARY KEY,
account_type VARCHAR(20) NOT NULL
CHECK (account_type IN ('Personal', 'Business Basic', 'Business Plus')),
account_reference VARCHAR(30) NOT NULL);
INSERT dbo.Accounts VALUES(1, 'Personal', 'abc');
INSERT dbo.Accounts VALUES(2, 'Business Basic', '101');
INSERT dbo.Accounts VALUES(3, 'Personal', 'def');
INSERT dbo.Accounts VALUES(4, 'Business Plus', '5');
SELECT account_nbr, account_type, account_reference
FROM dbo.Accounts;
SELECT account_nbr, account_reference AS account_ref_nbr
FROM dbo.Accounts
WHERE account_type LIKE 'Business%'
AND CAST(account_reference AS INT) > 20;
Error:
Conversion failed when converting the varchar value 'abc' to data type int.
SELECT account_nbr, account_ref_nbr
FROM (SELECT account_nbr,
CAST(account_reference AS INT) AS account_ref_nbr
FROM dbo.Accounts
WHERE account_type LIKE 'Business%') AS A
WHERE account_ref_nbr > 20;
Error:
Conversion failed when converting the varchar value 'abc' to data type int.
SELECT account_nbr, account_reference AS account_ref_nbr
FROM dbo.Accounts
WHERE account_type LIKE 'Business%'
AND CASE WHEN account_reference NOT LIKE '%[^0-9]%'
THEN CAST(account_reference AS INT)
END > 20;
SELECT account_nbr, SUM(CAST(account_reference AS INT)) AS account_total
FROM dbo.Accounts
WHERE account_type LIKE 'Business%'
AND CASE WHEN account_reference NOT LIKE '%[^0-9]%'
THEN CAST(account_reference AS INT)
END > 20
GROUP BY account_nbr
DROP TABLE dbo.Accounts;
Best,
Kevin G. Boles
SQL Server Consultant
SQL MVP 2007-2012
TheSQLGuru on googles mail service
May 6, 2013 at 9:12 am
Sergiy (5/5/2013)
Jeff Moden (5/3/2013)
My biggest concern is the conversion to FLOAT to do the sum. FLOAT only has a precision of 15 significant digits. If you go any higher than that, you will start to get siginficant rounding errors without any warning.Use the appropriate datatype for these types of things. For most sums, FLOAT isn't it.
Is it?
Really?
Understood and appreciated. Thanks, Sergiy.
But, the OP had this...
select sum(CONVERT(float, ft.amount))
I could be wrong but if the precision goes past 15 digits, won't you get the same kind of rounding "errors" that people get when they use STR simply because of the implicit conversion to float?
http://www.sqlservercentral.com/articles/T-SQL/71565/
--Jeff Moden
Change is inevitable... Change for the better is not.
May 6, 2013 at 2:16 pm
Jeff Moden (5/6/2013)
But, the OP had this...select sum(CONVERT(float, ft.amount))
I could be wrong but if the precision goes past 15 digits, won't you get the same kind of rounding "errors" that people get when they use STR simply because of the implicit conversion to float?
Final desired precision is DECIMAL(13,2).
On average you lose 1 precise digit onm every 2 math operations.
Number of operations foe any particular calculation does not exceed 3 (or 4 if I missed something).
So 15 digits precision provided by FLOAT is absolutely sufficient in this particular case.
Converting "amount" to BIGINT, having heaps of uncontrolled implicit conversions (including use of 100.00 which may bring to REAL level of precision when used in divisions), using different types for values in the same returned column - those are much more serious reasons for concerns, IMHO.
🙂
_____________
Code for TallyGenerator
Viewing 7 posts - 16 through 21 (of 21 total)
You must be logged in to reply to this topic. Login to reply