October 23, 2002 at 1:25 pm
All comments welcome.
1. Is this coding good practice?
2. Is it good for performance?
3. Will there be an cached execution plan?
4. Will it recompile each time it is ran?
CREATE PROCEDURE SP_GMSGetDeclarationList (
@inp_strGMSPolicyNumberPolicyNumbervarchar(16)="~",
@inp_lngGMSPeriodTypeID int= -2147483647,
@inp_dtmGMSFromDateDateTime='1/2/1753',
@inp_dtmGMSToDateDateTime='1/2/1753',
@inp_lngGMSProcessedPeriodTypeIDint= -2147483647,
@inp_lngGMSDeclarationPeriodTypeIDint= -2147483647)
AS
--Declarations
DECLARE @SQL varchar(8000)
DECLARE @SQLWHERE varchar(8000)
---initialize values
SET @SQL = " "
SET @SQLWHERE = " "
-- SQL STATEMENT
SET @SQL = "SELECT "
Set @SQL=@SQL +"Exporter.ExporterNumber as lngGMSExporterExporterNumber, "
Set @SQL=@SQL +"Exporter.ExporterName as strGMSExporterExporterName, "
Set @SQL=@SQL +"Declaration.DeclarationDate as dtmGMSDeclarationDeclarationDate, "
Set @SQL=@SQL +"Declaration.ProcessedDate as dtmGMSDeclarationProcessedDate, "
Set @SQL=@SQL +"Declaration.CountryCode as strGMSDeclarationCountryCode, "
Set @SQL=@SQL +"Declaration.DeclarationCurrencyCode as strGMSDeclarationDeclarationCurrencyCode, "
Set @SQL=@SQL +"IsNULL(Declaration.InvoicedVolumeAmount,0) as curGMSDeclarationInvoicedVolumeAmount, "
Set @SQL=@SQL +"IsNULL(Declaration.PremiumCalculatedAmount,0) as curGMSDeclarationPremiumCalculatedAmount, "
Set @SQL=@SQL +"IsNULL(Declaration.PSTAmount,0) as curGMSDeclarationPSTAmount, "
Set @SQL=@SQL +"Declaration.ProvinceCode as strGMSDeclarationProvinceCode, "
Set @SQL=@SQL +"Declaration.RegionCode as strGMSDeclarationRegionCode, "
Set @SQL=@SQL +"CASE UPPER(RTrim(Declaration.AdjustmentCode)) "
Set @SQL=@SQL +" WHEN 'A' THEN CAST('1' AS bit) "
Set @SQL=@SQL +"ELSE CAST('0' As bit)"
Set @SQL=@SQL +"END as blnGMSDeclarationAdjustedIndicator "
Set @SQL=@SQL +"FROM "
Set @SQL=@SQL +"GMS_Declaration as Declaration "
Set @SQL=@SQL +"LEFT JOIN "
Set @SQL=@SQL +"GMS_PolicyNumber as PolicyNumber ON "
Set @SQL=@SQL +"Declaration.PolicyNumberID = PolicyNumber.PolicyNumberID "
Set @SQL=@SQL +"LEFT JOIN "
Set @SQL=@SQL +"GMS_Policy as Policy ON "
Set @SQL=@SQL +"PolicyNumber.PolicyNumberID = Policy.PolicyNumberID "
Set @SQL=@SQL +"LEFT JOIN "
Set @SQL=@SQL +"GMS_Exporter As Exporter ON "
Set @SQL=@SQL +"Policy.ExporterID = Exporter.ExporterID "
Set @SQL = @SQL + ' WHERE '
IF @inp_strGMSPolicyNumberPolicyNumber <> "~"
SET @SQLWHERE = @SQLWHERE+ " PolicyNumber.PolicyNumber = '" + @inp_strGMSPolicyNumberPolicyNumber + "'"
IF @inp_lngGMSPeriodTypeID = @inp_lngGMSProcessedPeriodTypeID
BEGIN
IF @inp_dtmGMSFromDate<>'1/2/1753'
IF Len(RTrim(@SQLWHERE)) > 0
SET @SQLWHERE = @SQLWHERE + " AND Declaration.ProcessedDate >= '" + CAST(@inp_dtmGMSFromDate as varchar(128)) + "'"
Else
SET @SQLWHERE = @SQLWHERE + " Declaration.ProcessedDate >= '" + CAST(@inp_dtmGMSFromDate as varchar(128)) + "'"
IF @inp_dtmGMSToDate<>'1/2/1753'
IF Len(RTrim(@SQLWHERE)) > 0
SET @SQLWHERE = @SQLWHERE + " AND Declaration.ProcessedDate <= '" + CAST(@inp_dtmGMSToDate as varchar(128)) + "'"
Else
SET @SQLWHERE = @SQLWHERE + " Declaration.ProcessedDate <= '" + CAST(@inp_dtmGMSToDate as varchar(128)) + "'"
END
ELSE
BEGIN
IF @inp_dtmGMSFromDate<>'1/2/1753'
IF Len(RTrim(@SQLWHERE)) > 0
SET @SQLWHERE = @SQLWHERE + " AND Declaration.DeclarationDate >= '" + CAST(@inp_dtmGMSFromDate as varchar(128)) + "'"
Else
SET @SQLWHERE = @SQLWHERE + " Declaration.DeclarationDate >= '" + CAST(@inp_dtmGMSFromDate as varchar(128)) + "'"
IF @inp_dtmGMSToDate<>'1/2/1753'
IF Len(RTrim(@SQLWHERE)) > 0
SET @SQLWHERE = @SQLWHERE + " AND Declaration.DeclarationDate <= '" + CAST(@inp_dtmGMSToDate as varchar(128)) + "'"
Else
SET @SQLWHERE = @SQLWHERE + " Declaration.DeclarationDate <= '" + CAST(@inp_dtmGMSToDate as varchar(128)) + "'"
END
-------------------------------------
EXEC(@SQL + @SQLWHERE)
RETURN
GO
October 23, 2002 at 2:31 pm
There are lots of discussions at the moment on dynamic SQL.
Comments remove most of the SET @SQL statements, bad for performance and readability just use CRs i.e
SET @SQL = "SELECT "
Exporter.ExporterNumber as lngGMSExporterExporterNumber,
Exporter.ExporterName as strGMSExporterExporterName,
Declaration.DeclarationDate as dtmGMSDeclarationDeclarationDate,
Declaration.ProcessedDate as dtmGMSDeclarationProcessedDate,
Declaration.CountryCode as strGMSDeclarationCountryCode,
Declaration.DeclarationCurrencyCode as strGMSDeclarationDeclarationCurrencyCode, ...
use shorter aliases, allows you to use one varchar(8000) and thus use sp_executesql.
Why do you have to alias every column?
Use sp_executesql and pass your variables in that way, no read to cache and results in proper plan caching.
Finally you only have 4 different permutations of SQL, Put this in stored procedures and decide which to call in another SP of in the front end.
Simon Sabin
Co-author of SQL Server 2000 XML Distilled
http://www.amazon.co.uk/exec/obidos/ASIN/1904347088
Simon Sabin
SQL Server MVP
http://sqlblogcasts.com/blogs/simons
October 23, 2002 at 2:46 pm
As far as Dynamic Sql goes, its beautiful. All explicit conversions, very modular for adding / removing columns, well formatted for easy reading.......
As far as good for performance, I assume your asking about dynamic vrs static sql (because of the other questions), this would be much better if parameterized using the sp_executesql, and even better using static. Its a tradeoff on maintenance among other things and there are several very good topics current that have this discussion going in full force.
Each Sql statement generated by this code will have a cached execution plan though it probably will not be re-used as it will most likely be flushed before the code generates the same SQL statement again. Parameterizing this, would create far fewer Sql statements (hopefully one) and allow for greater re-use.
And finally, Yes, it will recompile each time, at the point the exec statement runs, and the dynamic statement begins processing, as the code is currently written.
Best path of action would be to write your individual statements into procs, and use a pointer proc to choose which to run, passing in the params needed....
October 23, 2002 at 2:49 pm
I didn't realize someone else already told you how to deal with it while I was typing.....
October 23, 2002 at 2:51 pm
Agree strongly on the readability of TSQL.
One can spend a lot of time just dechipering code.
Suggest you look at the potential use of case then enable/disable a check inside one select.
Example And (Case When @a is Null then 'Condition eval to true/false) Else True 'Nocheck just return true' End)=True
October 23, 2002 at 3:41 pm
Do you really have double quotes around each part of the query you intend to build? I never use double qoutes with dynamic SQL as it can give problems sometimes. I always use single quotes (side by side when needed) with dynamic SQL.
Also it doesn't look like you have any returns or tabs in the SQL code you build. If you want to format your dynamically built code so that it is easier to read then you can add CHAR(10) before each line where you would like a return to appear and/or CHAR(9) wherever you want a tab. I often format my dynamic code this way for ease of reading the code and the SQL it builds:
DECLARE @SQL varchar(500)
SET @SQL = ''
SET @SQL = @SQL + 'SELECT Column1,'
+ CHAR(10) + CHAR(9) + 'Column2,'
+ CHAR(10) + CHAR(9) + 'Column3'
+ CHAR(10) + 'FROM Table1'
PRINT @SQL
If you choose to convert this to static SQL as recommended make sure you compare the speeds between the dynamic and static and review the execution plans. I have encountered cases where the static SQL queries will pull all the rows in a table before applying the where clause and the dynamic SQL will only pull the rows needed in the initial table scan or index seek. In such a case the dynamic SQL will perform better.
Robert W. Marda
SQL Programmer
bigdough.com
The world’s leading capital markets contact database and software platform.
Robert W. Marda
Billing and OSS Specialist - SQL Programmer
MCL Systems
Viewing 6 posts - 1 through 5 (of 5 total)
You must be logged in to reply to this topic. Login to reply