THE SQL Server Blog Spot on the Web

Welcome to SQLblog.com - The SQL Server blog spot on the web Sign in | |
in Search

Jamie Thomson

This is the blog of Jamie Thomson, a freelance data mangler in London

The perils of double-dash comments [T-SQL]

I was checking my Twitter feed on my way in to work this morning and was alerted to an interesting blog post by Valentino Vranken that highlights a problem regarding the OLE DB Source in SSIS. In short, using double-dash comments in SQL statements within the OLE DB Source can cause unexpected results. It really is quite an important read if you’re developing SSIS packages so head over to SSIS OLE DB Source, Parameters And Comments: A Dangerous Mix! and be educated. Note that the problem is solved in SSIS2012 and Valentino explains exactly why.

If reading Valentino’s post has switched your brain into “learn mode” perhaps also check out my post SSIS: SELECT *... or select from a dropdown in an OLE DB Source component? which highlights another issue to be aware of when using the OLE DB Source.

As I was reading Valentino’s post I was reminded of a slidedeck by Chris Adkin entitled T-SQL Coding Guidelines where he recommends never using double-dash comments:

image

That’s good advice!

@Jamiet

Published Thursday, December 06, 2012 10:21 AM by jamiet

Comment Notification

If you would like to receive an email when updates are made to this post, please register here

Subscribe to this post's comments using RSS

Comments

 

Koen Verbeeck said:

I always use double dash, so I guess it's time to start changing my habits :)

December 6, 2012 4:34 AM
 

Dave Ballantyne said:

Maybe i should look at my TSQL Smells project taking apart a dtsx package also as it can already detect these in .sql scripts :)

http://tsqlsmells.codeplex.com/

December 6, 2012 5:11 AM
 

Mikael Eriksson said:

Unless you need to comment out some XML code that uses /* in the XPath expression.

<pre>

select @XML.value('.', 'int')

from @XML.nodes('/root/*[@ID = 1]') as T(N)

</pre>

Then you need to use the double dashes.

December 6, 2012 5:49 AM
 

Adam Machanic said:

Sorry, but this is totally off the mark. Inline comments are easier to use, maintain, and even to read for small comments. Plus they don't create a comment block, and that's a plus when you need to later comment out larger chunks of code.

Don't abandon tools due to perceived weakness. Just as you wouldn't abandon a hammer due to its lack of utility as a screwdriver. Figure out, instead, the appropriate time and place for everything.

December 6, 2012 7:22 AM
 

jamiet said:

Adam,

Each to their own. If there are good arguments for and against then I hope people have the good sense to take both arguments on board before deciding what they want to do.

JT

December 6, 2012 7:59 AM
 

Mike Woodburne said:

Adam and Mikael both have very good points - the double dash comment has a purpose and it shouldn't be discarded as being dangerous, especially because of a product bug.

Also, unless I am missing some clever trolling or delicious irony, Chris Adkin's slide not only offers up a poor reason for not using the double dash (any good statement can become useless because of an accidental backspace), but it also contains a spelling error when countering the argument for good code review (an error which would have been caught during a review).

December 6, 2012 8:43 AM
 

jamiet said:

Hi Mike,

Thanks for the comment.

No clever trolling or irony intended. Yes, Chris' slide contained a spelling error but I wasn't going to not post it because of that. Hopefully Chris will come here and reply to your belief that he offered a poor reason for not using double dash.

Regards

Jamie

December 6, 2012 9:30 AM
 

Chris Adkin said:

Mike,

As per Jamie's comment, "Each to their own". However, I do know for a fact that there have been instances where code has gone bang in production because of lines being back spaced onto preceding lines containing '--' comments by mistake. Also, and you can put this down to my own personal preference, I think /* */ comment blocks are more readable. As for your comment about trolling, I'm not sure where this comes from. Another point I would make, and again this is regarding readability, and again, before anyone shoots me down in flames you can attribute this to my own personal preference, is that I often see people use '--' style cooments where by you have to scroll a considerable distance to the right of the screen to see what the comment text actually is. In my humble opinion, the use of /* */ style comments is more conducive to readable code in this specific case. Thanks for the point about the typo, I hadn't noticed this and I will correct it. I would suspect that the spirit of Jamie's post was intended to reflect the fact that '--' can be dangerous and was not intended to out right castigate anyone that uses these. Conversely I have seen people comment out whole blocks of code with /* */ which is also bad, such code, again IMHO, should never make it into production. I never suspected that comment styles would illicit such strong opinions, temp tables versus table variables perhaps . . .

Chris

December 6, 2012 10:33 AM
 

Cliff said:

I prefer using a stored procedure as the source if there is anything more than a simple select. If it is a simple select with maybe a few joins, I will place comments in the annotation next to the OLE SRC if needed.

I also agree that "someone might hit backspace and hose code" is not, IMO, a good reason to not use the double dash. If the developer is testing code, and code is being locked down, then this will be caught long before it gets to production. In terms of portability, I believe there are some SQL engines that do not support /* */ commenting. Of course, I could be wrong on that last point.

All that being said, I use both styles. If it's a simple comment, I will use the double dash. If it's going to need a paragraph then I will use the comment block style.

December 6, 2012 12:25 PM
 

Valentino Vranken said:

On the comment-style usage topic, I'm using both all the time. As Adam said, they both have their good uses!

Having said that, the bug has been filed: https://connect.microsoft.com/SQLServer/feedback/details/773689/ssis-ole-db-source-incorrectly-returns-zero-records-in-combination-with-parameter-and-comment

December 6, 2012 12:27 PM
 

Marcus van Raalte said:

I just read this advice and this is my first time reading a SQLblog.com entry that i said NO!

Thanks you Adam Machanic for your comment

The problem with /**/ style comments, that means we must be careful with them also, are 2 fold

1) If there is a big block of comments, finding the start and end of a block can be a challenge

2) You cannot have 1 set embedded in another, this limitation can result in difficult find compile SQL errors.

My default advice to juniors, or non-SQL developers is to use the double dash by default. The reason is that they are "in your face" at the begining of each line of SQL code, easy and obvious.

Typing errors can cause all sorts of problems and the fear of them should not be the basis for defining coding best practise when using the alternatives has bigger issues to worry about.

December 6, 2012 8:55 PM
 

mst said:

Well...

[irony]I think the solution, then, is NO comments.[/irony] Because in SSMS for SQL2005, we encountered this:

I tried to modifiy a USP; SSMS pops up a dialog:

Script failed for StoredProcedure 'dbo.uspUhoh'. (Microsoft.SqlServer.Smo)

Additional information:

Syntax error in TextHeader of StoredProcedure 'uspUhoh'. (Microsoft.SqlServer.Smo)

We were in the middle of a server issue-- and when I hit THIS I was concerned we may have had a db problem. I punted to the developer to do some checking.

His conclusion was that it was caused by nested comments before the ALTER PROCEDURE statement-- here was his sample:

SET ANSI_NULLS ON

GO

SET QUOTED_IDENTIFIER ON

GO

/*

So it is okay to have one level of comments...

It is not okay /* to have more than one embedded */ C-Style /* comments like these */

Though, strangly one set of embedded C-Style comments is, apparently, okay. Two are not.

-- These style are okay to embed

*/

ALTER PROCEDURE dbo.uspUhoh

AS

BEGIN

-- your code

END

December 7, 2012 1:09 PM
 

Cliff said:

I don't think a valid solution to not using the comment block properly is to just not put in any comments. That's like refusing to walk because you tripped once.

December 7, 2012 3:55 PM
 

Adam Machanic said:

Cliff,

Pretty sure mst was making some attempt at humor.

By the way, to those who believe in banning inline comments due to some tiny potential for error: this same argument would have to apply to any language that supports such comments. So what you're really implying is that virtually every language ever created got this wrong. Sorry, but I'll have to take the side of the language designers in that particular battle.

By the way, this reminds me of a manager I once had who banned us from using the ternary operator in C++ programs, because once he had accidentally misread one and introduced an error in some program. There is such a thing as acceptable risk, and both inline comments and ternary operators have very low, very reasonable thresholds.

--Adam

December 7, 2012 4:13 PM
 

CodePro said:

Actually, my goal is always to make my code as self-documenting as possible, Zen-like. When I insert a comment, I am acknowledging failure.

December 10, 2012 1:09 PM
 

Adam Machanic said:

CodePro: I've heard that lots and lots of times. And it doesn't work. Use comments, please.

December 10, 2012 8:14 PM
 

Larry Leonard said:

CodePro - Code cannot be made self-documenting.  Code always answers the question "How?", but can never the question "Why?".  This is a problem, as anyone can (eventually) understand *how* a piece of code works; understanding *why* the code is doing it requires comments.

In other, more Zen-like words, just because the cat has her kittens in the oven doesn't make them biscuits.  Code is code: it cannot be made more than it is.

December 12, 2012 10:42 AM
 

JoejITSu said:

use 4 dashes (double click the comment button in SSMS, then an inadvertent delete still leaves you with a spare dash...ROFL at the logic here, why not just never drive because an inadvertent slip might put you in the guardrail...

Test your code before you commit it, so users don't catch your silliness...THAT is the hard\fast rule...

August 29, 2013 12:08 PM

Leave a Comment

(required) 
(required) 
Submit

This Blog

Syndication

Powered by Community Server (Commercial Edition), by Telligent Systems
  Privacy Statement