April 2, 2014 at 7:04 am
I agree with what most of you have said, and running the code did not generate any errors. I guess the confusion comes from the statement "You're about to test updates and work on performance". This of course doesn't necessarily mean that you have enabled the execution plan, although I know it might be the first place I'd look.... So this was the trap in this question, you had to "assume" that by saying Hey! I'm working on performance, you had just turned on the execution plan.
Like many others, 4 answers right and the last one was wrong due to the fact that I had to guess it. Not making up excuses, just sayin' 😉
April 2, 2014 at 7:10 am
With respect - I know that preparation of a QoTD can be quite tricky and time consuming but this QoTD has the same problem as the last one; it's dealing with settings which are "not normal"
The default for that setting is false, but it in this case it was enabled
This information hits the nail - but it was in the answer not in the question 🙁
4 of 5 correct; last wrong because of missing information!
Microsoft Certified Master: SQL Server 2008
MVP - Data Platform (2013 - ...)
my blog: http://www.sqlmaster.de (german only!)
April 2, 2014 at 7:21 am
I'm using SQL 2012 Developer Edition for testing. Run this from the question text:
CREATE TABLE Questions
(
QuestionID INT
, QuestionTitle VARCHAR(100)
, datechanged DATETIME
, IsApproved BIT DEFAULT 0
)
go
CREATE TRIGGER updateQuestions ON dbo.Questions
FOR UPDATE
AS
UPDATE Q
SET Q.datechanged = GETUTCDATE()
FROM inserted i
INNER JOIN dbo.Questions Q
ON I.QuestionID = q.QuestionID
go
INSERT INTO Questions
( QuestionID
, QuestionTitle
, datechanged
)
VALUES
( 1
, 'Select me!'
, GETUTCDATE()
)
go
INSERT INTO Questions
( QuestionID
, QuestionTitle
, datechanged
)
VALUES
( 2
, 'Tables and Columns, Oh My'
, GETUTCDATE()
)
go
Then this as the secret sauce:
sp_configure 'disallow results from triggers', 1
reconfigure
And then Control-M to enable execution plans.
And then, the final update from the question text:
UPDATE Questions
SET IsApproved = 0
WHERE QuestionID IN ( 1, 2 )
Yields for me:
(2 row(s) affected)
(1 row(s) affected)
Msg 524, Level 16, State 1, Procedure updateQuestions, Line 4
A trigger returned a resultset and the server option 'disallow results from triggers' is true.
April 2, 2014 at 7:26 am
I tried to give all the clues I could without giving it away - tough to do that. Putting the easy answers in was designed to make it look like an easy question - it wasn't! I was trying to get you to keep saying "why" on the tough one, why would that happen?
I appreciate the comments, good and bad, and any ideas you have for making it better without making it easier!
April 2, 2014 at 7:49 am
I thought the question was a good one. However I too was trying to guess the 5th choice. I reread the question and it says
You're about to test updates and work on performance (you haven't added any indexes yet) when the gong rings for the stand up.
This suggested to me that performance testing had not yet begun. This would generally indicate you have not yet looked at the execution plan. I couldn't for the life of me figure out the three rows thing because the messages would have said 2 row(s) and 2 row(s). One for the update and the other for the trigger. Not sure how 3 came from that.
I think a simple text change to
You started looking at performance when the gong rings...
Still would have been somewhat ambiguous but would have offered a hint.
_______________________________________________________________
Need help? Help us help you.
Read the article at http://www.sqlservercentral.com/articles/Best+Practices/61537/ for best practices on asking questions.
Need to split a string? Try Jeff Modens splitter http://www.sqlservercentral.com/articles/Tally+Table/72993/.
Cross Tabs and Pivots, Part 1 – Converting Rows to Columns - http://www.sqlservercentral.com/articles/T-SQL/63681/
Cross Tabs and Pivots, Part 2 - Dynamic Cross Tabs - http://www.sqlservercentral.com/articles/Crosstab/65048/
Understanding and Using APPLY (Part 1) - http://www.sqlservercentral.com/articles/APPLY/69953/
Understanding and Using APPLY (Part 2) - http://www.sqlservercentral.com/articles/APPLY/69954/
April 2, 2014 at 7:53 am
By the way what is this Vanilla trigger?
April 2, 2014 at 7:59 am
It's unclear from the reqirements whether updating [IsApproved] column should set [datechanged], or if this should only apply to [QuestionTitle].
"Do not seek to follow in the footsteps of the wise. Instead, seek what they sought." - Matsuo Basho
April 2, 2014 at 8:19 am
Eric M Russell (4/2/2014)
It's unclear from the reqirements whether updating [IsApproved] column should set [datechanged], or if this should only apply to [QuestionTitle].
As the design stands at the moment, it would even trigger on change of QuestionID, which could make for some interesting results!
April 2, 2014 at 8:41 am
Given the requirements, I don't think there is anything functionally wrong with the table and trigger as is, except that the QuestionID needs to be indexed and the primary key.
Also, as a side note, SQL Server could really use a real date/time based alternative to TIMESTAMP datatype to prevent developers from feeling the need to do this type of thing with triggers. The current TIMESTAMP datatype is more of a unique row version stamp that is sequential but not date/time.
"Do not seek to follow in the footsteps of the wise. Instead, seek what they sought." - Matsuo Basho
April 2, 2014 at 11:32 am
I believe that there's an error in the logic of the trigger since there's no change in the data. If the information hasn't changed, then the datechanged value shouldn't change either.
April 2, 2014 at 11:35 am
Luis Cazares (4/2/2014)
I believe that there's an error in the logic of the trigger since there's no change in the data. If the information hasn't changed, then the datechanged value shouldn't change either.
Technically speaking the data did change. The columns were updated. The new values however do equal the old values. This one may be a case of semantics but the sql engine doesn't look at the existing values and not update a column just because it is the same value. It simply overwrites the values with the new ones, which in this case they are the same. 😉
_______________________________________________________________
Need help? Help us help you.
Read the article at http://www.sqlservercentral.com/articles/Best+Practices/61537/ for best practices on asking questions.
Need to split a string? Try Jeff Modens splitter http://www.sqlservercentral.com/articles/Tally+Table/72993/.
Cross Tabs and Pivots, Part 1 – Converting Rows to Columns - http://www.sqlservercentral.com/articles/T-SQL/63681/
Cross Tabs and Pivots, Part 2 - Dynamic Cross Tabs - http://www.sqlservercentral.com/articles/Crosstab/65048/
Understanding and Using APPLY (Part 1) - http://www.sqlservercentral.com/articles/APPLY/69953/
Understanding and Using APPLY (Part 2) - http://www.sqlservercentral.com/articles/APPLY/69954/
April 2, 2014 at 11:38 am
Eric M Russell (4/2/2014)
Given the requirements, I don't think there is anything functionally wrong with the table and trigger as is, except that the QuestionID needs to be indexed and the primary key.Also, as a side note, SQL Server could really use a real date/time based alternative to TIMESTAMP datatype to prevent developers from feeling the need to do this type of thing with triggers. The current TIMESTAMP datatype is more of a unique row version stamp that is sequential but not date/time.
The data definately changed, but it's not clear which columns are information. If business doesn't like how it works, then this one's a change request, not a bug given the requirements such as they are.
"Do not seek to follow in the footsteps of the wise. Instead, seek what they sought." - Matsuo Basho
April 2, 2014 at 11:43 am
Sean Lange (4/2/2014)
Luis Cazares (4/2/2014)
I believe that there's an error in the logic of the trigger since there's no change in the data. If the information hasn't changed, then the datechanged value shouldn't change either.Technically speaking the data did change. The columns were updated. The new values however do equal the old values. This one may be a case of semantics but the sql engine doesn't look at the existing values and not update a column just because it is the same value. It simply overwrites the values with the new ones, which in this case they are the same. 😉
That's why I avoided the word update. The column was updated and the data was replaced with the same values. To me, there's no change in the data even if there's a change underneath. It's just an issue on what values do we feel are relevant, last update or last change.
April 2, 2014 at 11:59 am
It's just an issue on what values do we feel are relevant, last update or last change.
That is pretty much what I said but yours is much easier to read and understand. 😀
_______________________________________________________________
Need help? Help us help you.
Read the article at http://www.sqlservercentral.com/articles/Best+Practices/61537/ for best practices on asking questions.
Need to split a string? Try Jeff Modens splitter http://www.sqlservercentral.com/articles/Tally+Table/72993/.
Cross Tabs and Pivots, Part 1 – Converting Rows to Columns - http://www.sqlservercentral.com/articles/T-SQL/63681/
Cross Tabs and Pivots, Part 2 - Dynamic Cross Tabs - http://www.sqlservercentral.com/articles/Crosstab/65048/
Understanding and Using APPLY (Part 1) - http://www.sqlservercentral.com/articles/APPLY/69953/
Understanding and Using APPLY (Part 2) - http://www.sqlservercentral.com/articles/APPLY/69954/
April 2, 2014 at 2:18 pm
I like the recent series of questions with some backstory. Not the strongest stories, but enough to add a nice touch to the QotD.
I do not, however, like this question. It builds on assumptions that are simply too far-fetched and unrealistic. And parts of the "correct" answer foster bad practice. Let's go over the answer options to see where I believe Andy went wrong.
The table does not have a primary key - obvious, too obvious. No idea why Andy added this optoin to the question. The 11% who (currently) did not select this option were probably either overlooking the "choose 5" remark, or just clicking some random options because they just want to read the answer and discussion and don't care about the points.
There are 2 rows in the table - again, very obvious. No INSERT trigger, no constraints, and no errors in the INSERT statements.
The query plan for the update shows a table scan - well, duh! The table is an unindexed heap; table scan is the only available option, period. I am not sure if this is supposed to refer to the update in the query window, the update in the trigger, or both, but it's true anyway. And not, as the answer suggests, because "there only two rows in the table and the table has no indexes" - but just because the table has no indexes. Add twenty billion rows, and you'll still get a scan for this query.
The trigger does not contain a logic error - this is where it gets tricky. The UPDATE ... FROM used in the trigger is a highly dangerous construction that, if I had my way, would have been deprecated a long time ago already. It is only reliable if the updated table is joined to the other table(s) on a condition that is guaranteed never to return more than a single row. In this case, that guarantee can only be given by a primary key constraint, unique constraint, or unique index on the QuestionID column. There is no such guarantee in this case, so it is possible that a single row will be updated multiple times in the trigger, if it joins to multiple rows in inserted.
However, in this specific case I will not call it a logic error - the SET clause only uses a system function, not a column from inserted, so no matter which order the optimizer picks, the result will still be the same. Just in a highly inefficient way. Any junior developer presenting this trigger code to me would be sent back to his desk immediately with the task of rewriting this code to use WHERE EXISTS - which does not have any of these issues and is hence far more efficient.
A resultset is being returned when the update runs - now this is where I really got angry. So I am supposed to assume that I would start my query tuning by just enabling the actual execution plan option? Even before I add the first index? Please don't tell me that there are people who tune this way. Good tuning does NOT start with the execution plan. It starts with using yoour brains to think about indexes. The next step is to do actual performance measurements, not by looking at execution plans but by using either SET STATISTICS IO and SET STATISTICS TIME, or by looking at actual elapsed time on a high enough umber of executions.
You only look at execution plans when you have a very complex query and are worried that you have made the optimzers' task too hard, or when you have seen a demonstrable performance issue with a query and need to figure out what the heck is causing it.
But one assumption is not enough - no, to arrive at the "right" answer, I had to make a second assumption as well: "The default for that setting is false, but it in this case it was enabled". Why would I have changed that setting? Why would anyone ever change that setting? Oh, I see - because, apparently, "it's never a good idea to return results from a trigger". Never? Really? I'd say that it's never a good idea to use the word never in any statement about SQL Server. And definitely not in this case. What if a query becomes a performance problem and you need to see the execution plan of a query in the trigger? (Sure, there are ways - but they are much harder, so why not follow the easy route?) What if a trigger doesn't yield the expected results and you cannot find the root cause? The best (or rather: least sucky) way to debug a trigger is to insert a few PRINT or SELECT statements in it and then trigger its execution in a query window. And whay if you run into one of the rare but real business cases where you actually need some results to be returned from a trigger?
If you don't want resultsets from your trigger, then simply don't use SELECT or PRINT statements in it. But don't disable a completely fine option.
With these assumptions, I could have explained the "total of three rows" statement. But there are other, less far-fetched assumptions that can also explain this - like maybe my default schema is not dbo, so the trigger is referencing another table; or a coworker played a prank on me while I was away from my keyboard; or one of the other people at the meeting was so excited that he immediately added his question to the table. Or (the one I went with, as being the least far-fetched) Andy made a mistake while typing the question and intended to write "four" - because both the update in the query window and the update in the trigger will return a "2 row(s) returned" message. (Well, unless of course you have set your SSMS to default to use SET NOCOUNT ON in every query window, like I normally do).
The update can be fixed by adding SET NOCOUNT ON - that depends on what you perceive to be the problem. Assuming that Andy meant "four" instead of "three", this will definitely fix that "problem". Plus, it is a well-documented best practice to start every trigger with SET NOCOUNT ON - those extra "nn row(s) returned" messages place unneeded extra burden on the network, slow down the performance, and worst of all can cause errors in some front-ends.
Out of the five options I picked, four matched Andy's. Three of them with no debate at all. One rather questionable - I believe that the "logic error" is a position that can be defended and I would never accept this trigger in my production system for that reason.
On the one where my answer differes from Andy's - after reading his explanation, I am even more convinced then before that my choice is less wrong than his. Not correct, as it does not exactly fir the question description, but at least it requires assumptions that are not so extremely far-fetched as the assumptions Andy implies.
I understand that, after running into this, Andy wanted to submit this as a QotD. But he somehow forgot to check exectly how unrealistic the assumptions are that someone seeing only this question would have to make.
Andy, I have always respected you a lot. Seeing the enormous amount of high-quality questions you contributed recently, and the extra effort you went through to add a light back-story has raised that respect even more. This question was an unfortunate slip. No worries, I am already looking forward to your next question, and I am 100% sure that it will be a good one again.
Viewing 15 posts - 16 through 30 (of 58 total)
You must be logged in to reply to this topic. Login to reply