April 10, 2012 at 8:16 am
mtassin (4/10/2012)
Yes there's a CPU penalty to pay, but wouldn't it wind up with just three plans in the cache? The original one that builds the query plus one for branch one, one for branch two and the parameter passed in would just be peachy?
Yes, that would be three plans (well, in most cases), but the OP's original code may not result in just 3 plans.
There's nothing special about sp_executesql that prevents cache pollution, and there was nothing in your previous post explaining how to parameterise a call to sp_executesql.
Gail Shaw
Microsoft Certified Master: SQL Server, MVP, M.Sc (Comp Sci)
SQL In The Wild: Discussions on DB performance with occasional diversions into recoverability
April 10, 2012 at 8:17 am
GilaMonster (4/10/2012)
mtassin (4/10/2012)
Yes there's a CPU penalty to pay, but wouldn't it wind up with just three plans in the cache? The original one that builds the query plus one for branch one, one for branch two and the parameter passed in would just be peachy?Yes, that would be three plans (well, in most cases), but the OP's original code may not result in just 3 plans.
There's nothing special about sp_executesql that prevents cache pollution, and there was nothing in your previous post explaining how to parameterise a call to sp_executesql.
My first post pointed to your blog entry on catch-all's which explained it 🙂
April 10, 2012 at 9:05 am
First off, you are using sp_executesql, which is good, as I said. Because of this, you're not generating hundreds & thousands of variations on the query. However, we're still looking at a few issues. First, VARCHAR(20) is being used to compare to another string, fine, and a number, implicit conversion time, whichy may lead to issues. Not to mention, we don't have any checks in place to validate that we're dealing with a number in our VARCHAR(20) field, so you need to add some more code, further complicating an already complicated query.
As to cache, I modified the code slightly so I could use my test database:
CREATE PROCEDURE [dbo].[SearchRecords]
@searchQuery AS NVARCHAR(100),
@col AS VARCHAR(100)
AS
BEGIN
SET NOCOUNT ON;
DECLARE @sql NVARCHAR(1000)
DECLARE @value NVARCHAR(20)
SET @value = @searchQuery
IF (@col = 'PERSON_ID')
BEGIN
SET @sql = 'SELECT * FROM dbo.Movie WHERE MovieId = @value'
END
ELSE IF (@col = 'FIRST_NAME')
BEGIN
SET @sql = 'SELECT * FROM dbo.Movie WHERE MovieName = @value'
END
EXEC sp_executesql @sql,N'@value nvarchar(20)',@value=@value
END
GO
DBCC freeproccache();
GO
EXEC dbo.SearchRecords @searchQuery = N'42', -- nvarchar(100)
@col = 'PERSON_ID' -- varchar(100);
GO
EXEC dbo.SearchRecords @searchQuery = N'Serpico', -- nvarchar(100)
@col = 'FIRST_NAME' -- varchar(100);
GO
SELECTdeqp.query_plan,
deqs.execution_count,
deqs.query_hash,
deqs.query_plan_hash,
dest.text,
OBJECT_NAME(dest.object_id)
FROMsys.dm_exec_query_stats AS deqs
CROSS APPLY sys.dm_exec_sql_text(deqs.sql_handle) AS dest
CROSS APPLY sys.dm_exec_query_plan(deqs.plan_handle) AS deqp;
GO
You don't end up with three plans. You end up with two, one that's a clustered index seek and the other that's a clustered index scan (on my table anyway). Both plans are trivial since we kept the example nice & simple. But neither one is related back to the original stored procedure, the object_id is NULL. There is no plan for the procedure itself.
So, let's assume for a moment this is only dynamic sql in the system. You're right. Maintaining and troubleshooting this is easy. But, let's assume that this is our pattern. Instead of writing procedures for different types of queries (which Gail has already outlined as a best practice) we're instead going to combine them.... lots of them. And we do this not on one database, but on 5-10 on this server. Now, when you have thousands of queries completely divorced from their procedures, how easy is this to tune & maintain?
If you watch the extended events, you can see the plan_cache_miss for the procedure followed by a plan_cach_insert. But the insert is for a PREPARED statement, not a STORED PROCEDURE.
Again, I'm not against dynamic TSQL, but I absolutely am very much against suggesting using it outside of limited circumstances.
"The credit belongs to the man who is actually in the arena, whose face is marred by dust and sweat and blood"
- Theodore Roosevelt
Author of:
SQL Server Execution Plans
SQL Server Query Performance Tuning
April 10, 2012 at 9:25 am
Grant Fritchey (4/10/2012)
First off, you are using sp_executesql, which is good, as I said. Because of this, you're not generating hundreds & thousands of variations on the query. However, we're still looking at a few issues. First, VARCHAR(20) is being used to compare to another string, fine, and a number, implicit conversion time, whichy may lead to issues. Not to mention, we don't have any checks in place to validate that we're dealing with a number in our VARCHAR(20) field, so you need to add some more code, further complicating an already complicated query.
Not true... at least not from this example. The OP said that all 4 columns were nvarchar(20).
Grant Fritchey (4/10/2012)
As to cache, I modified the code slightly so I could use my test database:So, let's assume for a moment this is only dynamic sql in the system. You're right. Maintaining and troubleshooting this is easy. But, let's assume that this is our pattern. Instead of writing procedures for different types of queries (which Gail has already outlined as a best practice) we're instead going to combine them.... lots of them. And we do this not on one database, but on 5-10 on this server. Now, when you have thousands of queries completely divorced from their procedures, how easy is this to tune & maintain?
Ok but if we're building a Search SP like the OP wanted, this table has 4 columns, so we're talking about 4 stored procs and the program deciding which to call, which likely won't happen, he'll write a 5th stored proc that's 4 IF THEN ELSE's each one executing a lower level stored proc.
Is that really any easier to maintain?
If the database expands... and he's got 800 tables with no more than 4 columns each, that's 4000 stored procedures to maintain vs 800 that are a little bit harder. Just for the SELECT part of CRUD, no? There's still another 2400 Insert, Update, and Delete stored procs potentially with an 800 table database.
vs 1600. That's 3200 stored procedures vs 6400.
I love stored procedures... but waiting 10 minutes for SSMS to list them all for me sucks.
April 10, 2012 at 9:33 am
And for that matter, if it was this... we'd call it a catch-all query no?
ALTER PROCEDURE [dbo].[SearchRecords]
@PersonID NVARCHAR(20),
@FirstName NVARCHAR(20),
@MiddleName NVARCHAR(20),
@LastName NVARCHAR(20)
AS
BEGIN
SET NOCOUNT ON;
DECLARE @sql NVARCHAR(MAX)
SET @sql = N'SELECT * FROM dbo.student WHERE 1=1 '
IF @personID IS NOT NULL
SET @sql = @sql + N' AND [id] = @PersonID '
IF @FirstName IS NOT NULL
SET @sql = @sql + N' AND [fn] = @FirstName '
IF @MiddleName IS NOT NULL
SET @sql = @sql + N' AND [mi] = @MiddleName '
IF @LastName IS NOT NULL
SET @sql = @sql = N' AND [ln] = @LastName '
EXEC sp_executesql @sql,N'@PersonID NVARCHAR(20),@FirstName NVARCHAR(20),@MiddleName NVARCHAR(20),@LastName NVARCHAR(20)',
@PersonID=@PersonID,@FirstName=@FirstName,@MiddleName=@MiddleName,@LastName=@LastName
END
April 10, 2012 at 10:52 am
To Sir Grant Fritchey and Sir Mark Tassin:
You two have mentioned something about cache. I have certain things to know abut this cache and I hope it is okay to ask:
1. Is this a cache from my SQL Server or is it a cache on my laptop as general?
2. Every time a run a stored procedure, let's say 10 times of adding records, do I have a 10 copies of that stored procedure or just 1 copy?
3. If this cache is from my SQL Server, can I "clear" this so that every time I run my Management Studio I have a "fresh" or "empty" cache?
By the way guys/gals, your suggestions really helps me, and as of now...my stored procedure is A-OK now. My program in C# now can choose (by means of a drop down list box) if a user wants to search by id, first name, middle name, or last name.
But as all of you Sirs (and Mam/s) have said, I have many things to consider (CPU usage, execution time, what if the database expands?, how about much more complex search using LIKE?, is it maintainable?, and many many more), and I guess I will practice more and more in familiarizing myself in SQL Server.
Thanks guys, I owe you all a lot. 😉
Warm regards,
MarkSquall
________________________________
"Listen to advice and accept instruction, and in the end you will be wise." -Proverbs 19:20
April 10, 2012 at 11:01 am
marksquall (4/10/2012)
To Sir Grant Fritchey and Sir Mark Tassin:You two have mentioned something about cache. I have certain things to know abut this cache and I hope it is okay to ask:
1. Is this a cache from my SQL Server or is it a cache on my laptop as general?
2. Every time a run a stored procedure, let's say 10 times of adding records, do I have a 10 copies of that stored procedure or just 1 copy?
3. If this cache is from my SQL Server, can I "clear" this so that every time I run my Management Studio I have a "fresh" or "empty" cache?
1 & 3. We're talking about SQL Server's Cache, and you don't want to clear it, that's what makes SQL Server run fast.
What you want to do is not put junk in it. The two main caches that DBA's worry about are the Plan Cache and the Data Cache, both use RAM on the SQL server and can fight with each other over how much space each gets. Idealy you want as much Cache for data as you can get, but sql queries store their plans in the Procedure cache.
2. If you call the stored procedure via RPC you'll have a single plan in the Plan Cache. Unless it's well written dynamic like what I linked to or posted, in which case you'll have N plans in the Cache where N is the number of different combinations of where clauses your dynamic query generated.
If you run a query like
SELECT * FROM student WHERE [id] = '001'
That will store a plan in the cache. If later you run
SELECT * FROM student WHERE [id] = '002'
That will store a second plan in the cache because the constant changed.
Both the data and plan caches do slowly expire the data out of themselves based on a bunch of criteria.
If instead you wrote
SELECT * FROM student WHERE [id] = @variable
Then normally only one plan goes into the cache and the value in the variable is handled at execution time.
April 10, 2012 at 12:12 pm
marksquall (4/10/2012)
To Sir Grant Fritchey and Sir Mark Tassin:You two have mentioned something about cache. I have certain things to know abut this cache and I hope it is okay to ask:
1. Is this a cache from my SQL Server or is it a cache on my laptop as general?
There's a cache for the instance of SQL Server. It's multiple memory storage areas managed by SQL Server, for SQL Server. As a general rule of thumb, give it as much as you have available, it'll use it all. And yeah, the specific area we're talking about here is called the query cache or procedure cache or plan cache. There are other areas of memory management also referred to as the cache.
2. Every time a run a stored procedure, let's say 10 times of adding records, do I have a 10 copies of that stored procedure or just 1 copy?
Well, that's part of the discussion going on. If you're referring to a procedure call, not a prepared statement (as we're seeing with the sp_executesql), then a plan is created for the procedure and stored in cache. It's there because compiling an execution plan is actually quite expensive for SQL Server, so reusing the plans is a goal. If you refer to a procedure 10 times, then you'll have a single plan, and 10 executions of that plan (you validate this using the query I listed above that selects stuff from the DMO sys.dm_exec_query_stats in combination with other DMOs).
3. If this cache is from my SQL Server, can I "clear" this so that every time I run my Management Studio I have a "fresh" or "empty" cache?
No, don't do that. It causes a recompile of every query in the system.
By the way guys/gals, your suggestions really helps me, and as of now...my stored procedure is A-OK now. My program in C# now can choose (by means of a drop down list box) if a user wants to search by id, first name, middle name, or last name.
But as all of you Sirs (and Mam/s) have said, I have many things to consider (CPU usage, execution time, what if the database expands?, how about much more complex search using LIKE?, is it maintainable?, and many many more), and I guess I will practice more and more in familiarizing myself in SQL Server.
Thanks guys, I owe you all a lot. 😉
Warm regards,
MarkSquall
Tuning queries and writing queries is a huge topic. It's not one that I can easily summarize for you here. If you're just getting started, I strongly recommend getting a copy of Itzik Ben-Gan's book "TSQL Fundamentals." It's one of the best on the topic. If you want to learn about tuning, I'd suggest my book down below in my signatures.
"The credit belongs to the man who is actually in the arena, whose face is marred by dust and sweat and blood"
- Theodore Roosevelt
Author of:
SQL Server Execution Plans
SQL Server Query Performance Tuning
April 10, 2012 at 12:21 pm
mtassin (4/10/2012)
Not true... at least not from this example. The OP said that all 4 columns were nvarchar(20).
From the original bit of code I copied, it was a column named ID. You're right. My bad on assuming that it might have been an identity column and therefor an INT. I probably should have assumed a GUID, in which case the nvarchar(20) is inadequate. Are we really going to argue that using a "generic" data type is better than using a specific one? If so, have fun. I'm done on that account.
Ok but if we're building a Search SP like the OP wanted, this table has 4 columns, so we're talking about 4 stored procs and the program deciding which to call, which likely won't happen, he'll write a 5th stored proc that's 4 IF THEN ELSE's each one executing a lower level stored proc.
Is that really any easier to maintain?
If the database expands... and he's got 800 tables with no more than 4 columns each, that's 4000 stored procedures to maintain vs 800 that are a little bit harder. Just for the SELECT part of CRUD, no? There's still another 2400 Insert, Update, and Delete stored procs potentially with an 800 table database.
vs 1600.That's 3200 stored procedures vs 6400.I love stored procedures... but waiting 10 minutes for SSMS to list them all for me sucks.
As I demonstrated, the queries are divorced from the procedure. If you have 3 or four, or even 10 or 15 catch-all queries, maybe this isn't a big deal. No argument. But if you're arguing that waiting for 6400 procs to load in SSMS is preferable to 6400 queries in the system that I have to perform text searches to find the associated procedures in order to edit these queries, then no, I don't agree.
Let's just agree to disagree. As I've tried to point out, over & over, you will have to have the occasional dynamic, catch-all query. You will. Absolutely. It's going to happen. Up to there, we're in agreement. But, suggesting that is an acceptable pattern for a majority, or even all procedures, as you just did, no, I just can't support you there. So, on that point, we part ways. Have a good day.
"The credit belongs to the man who is actually in the arena, whose face is marred by dust and sweat and blood"
- Theodore Roosevelt
Author of:
SQL Server Execution Plans
SQL Server Query Performance Tuning
April 10, 2012 at 1:05 pm
They can get divorced from the procedure for other reasons.
For instance this stored procedure that's in my plan cache doesn't have a value for Object_Name(dest.objectid)
/****************************************** PROCEDURE: usp_ImplTemplateTaskDetail AUTHOR: mtassin DATE: 2009-10-05 PURPOSE: Return Template Task Details for a given Template Task MODIFICATION PMGS: 93264 PARAMETERS: @TemplateTaskID int Identifier for a given Template Task MODIFICATIONS: 2010-03-12 tcheung Altered to conform to duration vs time value 2010-08-13 mtassin Removed Source System ID/Description *******************************************/
CREATE PROCEDURE [usp_ImplTemplateTaskDetail] @TemplateTaskID INT
AS
SET NOCOUNT ON
SELECT
TemplateTaskID = a.prj_tpltsk_key,
TemplateID = a.prj_tpl_key,
TaskNumber = a.prj_tpltsk_step,
TemplateTaskDesc = a.prj_tpltsk_desc,
OwningRoleID = a.prj_role_key,
OwningRoleDesc = c.prj_role_desc,
Projectedduration = a.prj_tpltsk_total,
TaskMilestoneID = b.prj_milestone_key,
TaskMileStoneDesc = d.prj_milestone_desc,
TaskTypeID = a.prj_tsktype_key,
TaskTypeDesc = b.prj_tsktype_desc,
TargetDay = a.prj_tpltsk_finish,
TaskStartDays = a.prj_tpltsk_start,
InstrucDocPath = a.prj_tpltsk_instruc
FROM
iad_projmgt_project_template_tasks a
JOIN iad_projmgt_project_task_types b ON a.prj_tsktype_key = b.prj_tsktype_key
JOIN iad_projmgt_project_roles c ON a.prj_role_key = c.prj_role_key
JOIN iad_projmgt_project_milestones d ON b.prj_milestone_key = d.prj_milestone_key
WHERE
a.prj_tpltsk_key = @TemplateTaskID
ORDER BY
a.prj_tpltsk_step
No dynamic code here.
April 10, 2012 at 1:15 pm
And before you say it, I do realize that at least in the text of the query I have the name of the procedure (after all I did copy it out of there).
But with the catch-all queries, you also quickly learn which ones have which patterns... My Devs around here can look at the dynamic query generated and know which one it came from 🙂
do I have 1600 of them? Heaven's no... but I've probably got more than most people would be comfortable with. :w00t:
Viewing 11 posts - 16 through 25 (of 25 total)
You must be logged in to reply to this topic. Login to reply