February 14, 2022 at 2:10 pm
Needs some help with suggestions to help in the speed of this SP. When I break down different parts seems to run fast but the group by
section is where I need some help\suggestions. The initial select pulls back about 7500 records. The views also retrieve data quick.
The logging is overkill...
Thanks..
[Get_Container_Quality_Summary]
-- Input parameters.
@Quality_Container_ID AS nvarchar(MAX) = NULL
AS
BEGIN
-- SET NOCOUNT ON added to prevent extra result sets from
-- interfering with SELECT statements.
SET NOCOUNT ON
-- Begin procedure.
EXECUTE Write_Error_Log 'INFORMATION','Beginning procedure (Get_Container_Quality_Summary)','Get_Container_Quality_Summary'-- Revision 2 - Added (Begin)
-- Internal variables.
EXECUTE Write_Error_Log 'INFORMATION','Declaring internal variables','Get_Container_Quality_Summary'
DECLARE @Error AS int = 0
DECLARE @Error_Log_Desc AS nvarchar(MAX)
-- Check for required inputs.
IF ISNULL(@Quality_Container_ID,'') = ''
BEGIN
-- Log error.
SET @Error = 1
EXECUTE Write_Error_Log 'ERROR','Required inputs not specified','Get_Container_Quality_Summary',@Error
-- Exit procedure.
GOTO Log_Result_Before_Exit
END-- Revision 2 - Added (End)
-- Create temp table to insert quality data only for the specific container.
EXECUTE Write_Error_Log 'INFORMATION','Creating #Quality temp table','Get_Container_Quality_Summary'-- Revision 2 - Added
BEGIN TRY-- Revision 2 - Added
CREATE TABLE #Quality(
[Quality_Id] [int] NOT NULL,
[Quality_Test_ID] [nvarchar](50) NOT NULL,
[Quality_Container_ID] [nvarchar](20) NOT NULL,
[Result] [nvarchar](50) NULL)
END TRY-- Revision 2 - Added
-- Log errors.
BEGIN CATCH-- Revision 2 - Added (Begin)
EXECUTE Write_Error_Log 'ERROR','Cannot create #Quality temp table','Get_Container_Quality_Summary'
SET @Error = 2
-- Exit procedure.
GOTO Log_Result_Before_Exit
END CATCH-- Revision 2 - Added (End)
-- Insert current data into the temp table from the Quality table.
EXECUTE Write_Error_Log 'INFORMATION','Inserting quality records into #Quality temp table','Get_Container_Quality_Summary'-- Revision 2 - Added
BEGIN TRY-- Revision 2 - Added
INSERT INTO #Quality (Quality_id,Quality_Test_ID,Quality_Container_ID,Result)
SELECT DISTINCT Quality_id,Quality_Test_ID,Quality_Container_ID,Result
FROM Quality (NOLOCK)
--WHERE Quality_Container_ID = @Quality_Container_ID AND ISNULL(@Quality_Container_ID,'') <> ''-- Revision 2 - Removed
WHERE Quality_Container_ID COLLATE DATABASE_DEFAULT = @Quality_Container_ID COLLATE DATABASE_DEFAULT-- Revision 2 - Added
END TRY-- Revision 2 - Added
-- Log errors.
BEGIN CATCH-- Revision 2 - Added (Begin)
EXECUTE Write_Error_Log 'ERROR','Cannot insert quality records into #Quality temp table','Get_Container_Quality_Summary'
SET @Error = 3
-- Exit procedure.
GOTO Log_Result_Before_Exit
END CATCH-- Revision 2 - Added (End)
-- Insert history data into the temp table from the Quality_History table.
EXECUTE Write_Error_Log 'INFORMATION','Inserting quality history records into #Quality temp table','Get_Container_Quality_Summary'-- Revision 2 - Added
BEGIN TRY-- Revision 2 - Added
INSERT INTO #Quality (Quality_id,Quality_Test_ID,Quality_Container_ID,Result)
SELECT DISTINCT Quality_id,Quality_Test_ID,Quality_Container_ID,Result
FROM Quality_History (NOLOCK)
--WHERE Quality_Container_ID = @Quality_Container_ID AND ISNULL(@Quality_Container_ID,'') <> ''-- Revision 2 - Removed
WHERE Quality_Container_ID COLLATE DATABASE_DEFAULT = @Quality_Container_ID COLLATE DATABASE_DEFAULT-- Revision 2 - Added
END TRY-- Revision 2 - Added
-- Log errors.
BEGIN CATCH-- Revision 2 - Added (Begin)
EXECUTE Write_Error_Log 'ERROR','Cannot insert quality history records into #Quality temp table','Get_Container_Quality_Summary'
SET @Error = 4
-- Exit procedure.
GOTO Log_Result_Before_Exit
END CATCH-- Revision 2 - Added (End)
-- Calculate quality summary data for the specified container using the temp table.
EXECUTE Write_Error_Log 'INFORMATION','Selecting records from #Quality temp table','Get_Container_Quality_Summary'-- Revision 2 - Added
BEGIN TRY-- Revision 2 - Added
SELECT DISTINCT
Q.Quality_Test_ID AS Test,
--COUNT(QD.Quality_ID) AS Test_Count,-- Revision 1 - Removed
COUNT(QD.Quality_ID)/ISNULL(NULLIF(COUNT(DISTINCT QD.Value_Count),0),1) AS Test_Count,-- Revision 1 - Added
SUM(CAST(ISNULL(QD.Value,0) AS float))/ISNULL(NULLIF(COUNT(DISTINCT QD.Value_Count),0),1) AS Test_Total,
CAST(SUM(CAST(ISNULL(QD.Value,0) AS float))/ISNULL(NULLIF(COUNT(QD.Quality_ID),0),1) AS decimal(38,5)) AS Test_Avg,
--COUNT(QD1.Quality_ID) AS Test_Count,-- Revision 1 - Removed
COUNT(QD1.Quality_ID)/ISNULL(NULLIF(COUNT(DISTINCT QD1.Value_Count),0),1) AS Pass_Count,-- Revision 1 - Added
SUM(CAST(ISNULL(QD1.Value,0) AS float))/ISNULL(NULLIF(COUNT(DISTINCT QD1.Value_Count),0),1) AS Pass_Total,
CAST(SUM(CAST(ISNULL(QD1.Value,0) AS float))/ISNULL(NULLIF(COUNT(QD1.Quality_ID),0),1) AS decimal(38,5)) AS Pass_Avg,
--COUNT(QD2.Quality_ID) AS Test_Count,-- Revision 1 - Removed
COUNT(QD2.Quality_ID)/ISNULL(NULLIF(COUNT(DISTINCT QD2.Value_Count),0),1) AS Warning_Count,-- Revision 1 - Added
SUM(CAST(ISNULL(QD2.Value,0) AS float))/ISNULL(NULLIF(COUNT(DISTINCT QD2.Value_Count),0),1) AS Warning_Total,
CAST(SUM(CAST(ISNULL(QD2.Value,0) AS float))/ISNULL(NULLIF(COUNT(QD2.Quality_ID),0),1) AS decimal(38,5)) AS Warning_Avg,
--COUNT(QD3.Quality_ID) AS Test_Count,-- Revision 1 - Removed
COUNT(QD3.Quality_ID)/ISNULL(NULLIF(COUNT(DISTINCT QD3.Value_Count),0),1) AS Fail_Count,-- Revision 1 - Added
SUM(CAST(ISNULL(QD3.Value,0) AS float))/ISNULL(NULLIF(COUNT(DISTINCT QD3.Value_Count),0),1) AS Fail_Total,
CAST(SUM(CAST(ISNULL(QD3.Value,0) AS float))/ISNULL(NULLIF(COUNT(QD3.Quality_ID),0),1) AS decimal(38,5)) AS Fail_Avg,
Q.Quality_Container_ID AS Test_Container
INTO #Quality_Summary-- Revision 2 - Added
--FROM #Quality AS Q INNER JOIN Quality_Test AS QT ON (Q.Quality_Test_ID = QT.Test_ID) INNER JOIN-- Revision 2 - Removed
FROM #Quality AS Q INNER JOIN Quality_Test AS QT ON (Q.Quality_Test_ID COLLATE DATABASE_DEFAULT = QT.Test_ID COLLATE DATABASE_DEFAULT) INNER JOIN-- Revision 2 - Added
Quality_Detail_View AS QD ON (Q.Quality_ID = QD.Quality_ID) LEFT OUTER JOIN
#Quality AS Q1 ON (Q.Quality_ID = Q1.Quality_ID AND Q1.Result = 'PASS') LEFT OUTER JOIN
Quality_Detail_View AS QD1 ON Q1.Quality_ID = QD1.Quality_ID LEFT OUTER JOIN
#Quality AS Q2 ON (Q.Quality_ID = Q2.Quality_ID AND Q2.Result = 'WARNING') LEFT OUTER JOIN
Quality_Detail_View AS QD2 ON Q2.Quality_ID = QD2.Quality_ID LEFT OUTER JOIN
#Quality AS Q3 ON (Q.Quality_ID = Q3.Quality_ID AND Q3.Result = 'FAIL') LEFT OUTER JOIN
Quality_Detail_View AS QD3 ON (Q3.Quality_ID = QD3.Quality_ID)
WHERE (QT.Test_Type = N'Numeric') AND
(QT.Test_Name NOT LIKE '%Total%') AND
(Q.Quality_ID NOT IN (SELECT Q.Quality_ID
FROM #Quality Q INNER JOIN Quality_Attribute_View QA ON (Q.Quality_Id = QA.Quality_ID)
WHERE (Attribute = 'Comment') AND (Attribute_Value LIKE 'In%Status%')))
GROUP BY Q.Quality_Container_ID,Q.Quality_Test_ID
ORDER BY Q.Quality_Container_ID,Q.Quality_Test_ID
END TRY-- Revision 2 - Added
-- Log errors.
BEGIN CATCH-- Revision 2 - Added (Begin)
EXECUTE Write_Error_Log 'ERROR','Cannot select records from #Quality temp table','Get_Container_Quality_Summary'
SET @Error = 5
-- Exit procedure.
GOTO Log_Result_Before_Exit
END CATCH-- Revision 2 - Added (End)
-- Drop temp table.
EXECUTE Write_Error_Log 'INFORMATION','Dropping #Quality temp table','Get_Container_Quality_Summary'-- Revision 2 - Added
BEGIN TRY-- Revision 2 - Added
DROP TABLE #Quality
END TRY-- Revision 2 - Added
-- Log errors.
BEGIN CATCH-- Revision 2 - Added (Begin)
EXECUTE Write_Error_Log 'ERROR','Cannot drop #Quality temp table','Get_Container_Quality_Summary'
SET @Error = 6
-- Exit procedure.
GOTO Log_Result_Before_Exit
END CATCH-- Revision 2 - Added (End)
Log_Result_Before_Exit:-- Revision 2 - Added (Begin)
-- Log result and error code.
SET @Error_Log_Desc = 'Returning procedure ERROR CODE (' + CAST(@Error AS nvarchar(MAX)) + ')'
IF @Error = 0
BEGIN
EXECUTE Write_Error_Log 'INFORMATION',@Error_Log_Desc,'Get_Container_Quality_Summary'
END
ELSE
BEGIN
EXECUTE Write_Error_Log 'ERROR',@Error_Log_Desc,'Get_Container_Quality_Summary',@Error
END
Exit_Procedure:
-- End procedure.
EXECUTE Write_Error_Log 'INFORMATION','Ending procedure (Get_Container_Quality_Summary)','Get_Container_Quality_Summary'
-- Display result and return error code.
SELECT * FROM #Quality_Summary
RETURN @Error-- Revision 2 - Added (End)
END
February 14, 2022 at 5:55 pm
You could start by creating the temp table with COLLATE DATABASE_DEFAULT and getting rid of all the COLLATE DATABASE_DEFAULT in your joins. All the casting is not going to help the query plan.
CREATE TABLE #Quality
(
[Quality_Id] [int] NOT NULL PRIMARY KEY,
[Quality_Test_ID] [nvarchar](50) COLLATE DATABASE_DEFAULT NOT NULL,
[Quality_Container_ID] [nvarchar](20) COLLATE DATABASE_DEFAULT NOT NULL,
[Result] [nvarchar](50) COLLATE DATABASE_DEFAULT NULL
);
February 14, 2022 at 6:51 pm
Try this as a start... How to Post Performance Problems , which might also give you a hint on how to start troubleshooting performance issues as well as what to post.
Second, look at the stuff in the SELECT list and FROM clause... basically, it's all a repeat of the same thing with a different qualifier. That's a pretty good indication that you could seriously simplify using pre-aggregation and a CROSSTAB, which will also seriously "DRY" out your code ("DRY" = Don't Repeat Yourself).
Here's an article to get you started on those concepts. Cross Tabs and Pivots, Part 1 – Converting Rows to Columns
You also need to have a serious look at the view you're using because that may also be a major issue.
--Jeff Moden
Change is inevitable... Change for the better is not.
February 14, 2022 at 7:04 pm
Big improvement, do you see anything else that could be tweaked?
Many thanks for suggestion.
February 14, 2022 at 7:45 pm
Big improvement, do you see anything else that could be tweaked?
Many thanks for suggestion.
Two way street, first... what does the code look like now that you've improved it?
[EDIT] Either that or you're talking about the collation fix. In that case, see my previous post. It's a "target rich environment".
--Jeff Moden
Change is inevitable... Change for the better is not.
February 14, 2022 at 8:01 pm
Yes collation fix helped...
"Second, look at the stuff in the SELECT list and FROM clause... basically, it's all a repeat of the same thing with a different qualifier. That's a pretty good indication that you could seriously simplify using pre-aggregation and a CROSSTAB, which will also seriously "DRY" out your code ("DRY" = Don't Repeat Yourself)."
basically, it's all a repeat of the same thing with a different qualifier ??
Thanks,
February 14, 2022 at 8:24 pm
Would help greatly to see the DDL for the tables, including all indexes.
The query could also be rewritten to help performance, but without sample data there's no way to test the rewrite so it's not possible to do now.
SQL DBA,SQL Server MVP(07, 08, 09) A socialist is someone who will give you the shirt off *someone else's* back.
February 14, 2022 at 8:44 pm
After following Ken and Jeff's advice first maybe have a look at the error handling and flow of control. You've got TRY/CATCH all over the place and GOTO and it's difficult to follow. Error logging 'INFORMATION' such as the creation and dropping of temp tables is a waste imo. Who reads that now? Nobody I'm sure. Also, none of the error logging actually interrupts the flow of control which makes it doubly hard to understand. A good rule of thumb is to make every effort to only use TRY/CATCH once per proc imo. Why? Because you can THROW any number of errors and it interrupts the flow of control in an intuitive way. Also, the right and proper column type for error messages is NVARCHAR(2048) because that's the type returned by the ERROR_MESSAGE() function as well as the FORMATMESSAGE() function (the purpose of which is to create custom error messages in a C# like way). Lastly, performance-wise, if you're returning 7500 rows to a temp table then there probably needs to be (an) index(es) of some sort.
Maybe something very roughly like this
create or alter proc [Get_Container_Quality_Summary]
@Quality_Container_ID AS nvarchar(MAX) = NULL
AS
SET NOCOUNT ON
DECLARE @Error AS int = 0
DECLARE @Error_Log_Desc AS nvarchar(2048)
--DECLARE @Error_Log_Desc AS nvarchar(MAX)
BEGIN TRY-- Revision 2 - Added
CREATE TABLE #Quality(
[Quality_Id] [int] NOT NULL,
[Quality_Test_ID] [nvarchar](50) NOT NULL,
[Quality_Container_ID] [nvarchar](20) NOT NULL,
[Result] [nvarchar](50) NULL);
-- Insert current data into the temp table from the Quality table.
INSERT INTO #Quality (Quality_id,Quality_Test_ID,Quality_Container_ID,Result)
SELECT DISTINCT Quality_id,Quality_Test_ID,Quality_Container_ID,Result
FROM Quality (NOLOCK)
--WHERE Quality_Container_ID = @Quality_Container_ID AND ISNULL(@Quality_Container_ID,'') <> ''-- Revision 2 - Removed
WHERE Quality_Container_ID COLLATE DATABASE_DEFAULT = @Quality_Container_ID COLLATE DATABASE_DEFAULT;
if @@rowcount=0
begin
select @Error_Log_Desc=formatmessage(N'No rows inserted into #Quality table from Quality with @Quality_Container_ID=%s', @Quality_Container_ID);
throw 50001, @Error_Log_Desc, 1
end
/* equiv to @Error=3 */
INSERT INTO #Quality (Quality_id,Quality_Test_ID,Quality_Container_ID,Result)
SELECT DISTINCT Quality_id,Quality_Test_ID,Quality_Container_ID,Result
FROM Quality_History (NOLOCK)
--WHERE Quality_Container_ID = @Quality_Container_ID AND ISNULL(@Quality_Container_ID,'') <> ''-- Revision 2 - Removed
WHERE Quality_Container_ID COLLATE DATABASE_DEFAULT = @Quality_Container_ID COLLATE DATABASE_DEFAULT; -- Revision 2 - Added
if @@rowcount=0
begin
select @Error_Log_Desc=formatmessage(N'No rows inserted into #Quality table from Quality_History with @Quality_Container_ID=%s', @Quality_Container_ID);
throw 50002, @Error_Log_Desc, 1
end
/* equiv to @Error=4 */
-- Calculate quality summary data for the specified container using the temp table.
SELECT DISTINCT
Q.Quality_Test_ID AS Test,
--COUNT(QD.Quality_ID) AS Test_Count,-- Revision 1 - Removed
COUNT(QD.Quality_ID)/ISNULL(NULLIF(COUNT(DISTINCT QD.Value_Count),0),1) AS Test_Count,-- Revision 1 - Added
SUM(CAST(ISNULL(QD.Value,0) AS float))/ISNULL(NULLIF(COUNT(DISTINCT QD.Value_Count),0),1) AS Test_Total,
CAST(SUM(CAST(ISNULL(QD.Value,0) AS float))/ISNULL(NULLIF(COUNT(QD.Quality_ID),0),1) AS decimal(38,5)) AS Test_Avg,
--COUNT(QD1.Quality_ID) AS Test_Count,-- Revision 1 - Removed
COUNT(QD1.Quality_ID)/ISNULL(NULLIF(COUNT(DISTINCT QD1.Value_Count),0),1) AS Pass_Count,-- Revision 1 - Added
SUM(CAST(ISNULL(QD1.Value,0) AS float))/ISNULL(NULLIF(COUNT(DISTINCT QD1.Value_Count),0),1) AS Pass_Total,
CAST(SUM(CAST(ISNULL(QD1.Value,0) AS float))/ISNULL(NULLIF(COUNT(QD1.Quality_ID),0),1) AS decimal(38,5)) AS Pass_Avg,
--COUNT(QD2.Quality_ID) AS Test_Count,-- Revision 1 - Removed
COUNT(QD2.Quality_ID)/ISNULL(NULLIF(COUNT(DISTINCT QD2.Value_Count),0),1) AS Warning_Count,-- Revision 1 - Added
SUM(CAST(ISNULL(QD2.Value,0) AS float))/ISNULL(NULLIF(COUNT(DISTINCT QD2.Value_Count),0),1) AS Warning_Total,
CAST(SUM(CAST(ISNULL(QD2.Value,0) AS float))/ISNULL(NULLIF(COUNT(QD2.Quality_ID),0),1) AS decimal(38,5)) AS Warning_Avg,
--COUNT(QD3.Quality_ID) AS Test_Count,-- Revision 1 - Removed
COUNT(QD3.Quality_ID)/ISNULL(NULLIF(COUNT(DISTINCT QD3.Value_Count),0),1) AS Fail_Count,-- Revision 1 - Added
SUM(CAST(ISNULL(QD3.Value,0) AS float))/ISNULL(NULLIF(COUNT(DISTINCT QD3.Value_Count),0),1) AS Fail_Total,
CAST(SUM(CAST(ISNULL(QD3.Value,0) AS float))/ISNULL(NULLIF(COUNT(QD3.Quality_ID),0),1) AS decimal(38,5)) AS Fail_Avg,
Q.Quality_Container_ID AS Test_Container
INTO #Quality_Summary-- Revision 2 - Added
--FROM #Quality AS Q INNER JOIN Quality_Test AS QT ON (Q.Quality_Test_ID = QT.Test_ID) INNER JOIN-- Revision 2 - Removed
FROM #Quality AS Q INNER JOIN Quality_Test AS QT ON (Q.Quality_Test_ID COLLATE DATABASE_DEFAULT = QT.Test_ID COLLATE DATABASE_DEFAULT) INNER JOIN-- Revision 2 - Added
Quality_Detail_View AS QD ON (Q.Quality_ID = QD.Quality_ID) LEFT OUTER JOIN
#Quality AS Q1 ON (Q.Quality_ID = Q1.Quality_ID AND Q1.Result = 'PASS') LEFT OUTER JOIN
Quality_Detail_View AS QD1 ON Q1.Quality_ID = QD1.Quality_ID LEFT OUTER JOIN
#Quality AS Q2 ON (Q.Quality_ID = Q2.Quality_ID AND Q2.Result = 'WARNING') LEFT OUTER JOIN
Quality_Detail_View AS QD2 ON Q2.Quality_ID = QD2.Quality_ID LEFT OUTER JOIN
#Quality AS Q3 ON (Q.Quality_ID = Q3.Quality_ID AND Q3.Result = 'FAIL') LEFT OUTER JOIN
Quality_Detail_View AS QD3 ON (Q3.Quality_ID = QD3.Quality_ID)
WHERE (QT.Test_Type = N'Numeric') AND
(QT.Test_Name NOT LIKE '%Total%') AND
(Q.Quality_ID NOT IN (SELECT Q.Quality_ID
FROM #Quality Q INNER JOIN Quality_Attribute_View QA ON (Q.Quality_Id = QA.Quality_ID)
WHERE (Attribute = 'Comment') AND (Attribute_Value LIKE 'In%Status%')))
GROUP BY Q.Quality_Container_ID,Q.Quality_Test_ID
ORDER BY Q.Quality_Container_ID,Q.Quality_Test_ID
if @@rowcount=0
throw 50003, N'No rows inserted into #Quality_Summary', 1
SELECT * FROM #Quality_Summary
select @error=0;
RETURN @Error; -- Revision 2 - Added (End)
END TRY
BEGIN CATCH
declare @proc_name sysname;
select @Error_Log_Desc=error_message();
select @proc_name=error_procedure()
EXECUTE Write_Error_Log 'ERROR', @Error_Log_Desc, @proc_name;
END CATCH
go
Aus dem Paradies, das Cantor uns geschaffen, soll uns niemand vertreiben können
February 14, 2022 at 9:47 pm
Yes collation fix helped...
"Second, look at the stuff in the SELECT list and FROM clause... basically, it's all a repeat of the same thing with a different qualifier. That's a pretty good indication that you could seriously simplify using pre-aggregation and a CROSSTAB, which will also seriously "DRY" out your code ("DRY" = Don't Repeat Yourself)."
basically, it's all a repeat of the same thing with a different qualifier ??
Thanks,
Look at the code in the SELECT and FROM... Each column in the SELECT is created using mostly the same code. Look at the joins in the FROM clause... except for the alias names and the Qx.Result ='something", most of the joins are virtually identical. You could return all that as a single per-aggregated table and then do a CROSSTAB to "pivot" it without all the fan-fare you have going on in the SELECT list.
--Jeff Moden
Change is inevitable... Change for the better is not.
February 14, 2022 at 11:10 pm
Do you mean where it's getting data from quality and quality_history ... the history part was added but currently that table is empty..
Could you provide the example crosstab..
Thx.
Viewing 10 posts - 1 through 9 (of 9 total)
You must be logged in to reply to this topic. Login to reply