Easy SQL mistakes: Accidental Correlated Subquery

Just a fun note, because it was a moment of pure puzzlement for me. I’m writing a new SP for Minion Backup (you knew it had to be Minion related, didn’t you?) and I just got the oddest error. Here’s how it went.

First, I got a list of backups from the log:

SELECT ExecutionDateTime
    , Status
    , PctComplete
    , DBName
    , DBType
    , BackupType
    , BackupStartDateTime
    , BackupEndDateTime
 FROM Minion.BackupLogDetails;

But I wanted to limit the resulting list based on an existing temporary table, so I joined them, adding simple table aliases as I went (“D” and “B”):

SELECT D.ExecutionDateTime
    , D.Status
    , D.PctComplete
    , D.DBName
    , D.DBType
    , D.BackupType
    , D.BackupStartDateTime
    , D.BackupEndDateTime
 FROM Minion.BackupLogDetails as D
 JOIN #Backups as B ON D.DBName = B.DBName;

But the important thing was, I wanted this new result set to be of backups PRIOR TO the most recent backup set. That’s easy, I’ll just get the max date from BackupLogDetails, and pull a less than:

SELECT D.ExecutionDateTime
    , D.Status
    , D.PctComplete
    , D.DBName
    , D.DBType
    , D.BackupType
    , D.BackupStartDateTime
    , D.BackupEndDateTime
 FROM Minion.BackupLogDetails as D
 JOIN #Backups as B ON D.DBName = B.DBName
 WHERE D.ExecutionDateTime < ( SELECT   MAX(D.ExecutionDateTime)
    FROM Minion.BackupLogDetails

Aaaaand then I get the weird error:

Msg 147, Level 15, State 1, Line 50
An aggregate may not appear in the WHERE clause unless it is in a subquery contained in a HAVING clause or a select list, and the column being aggregated is an outer reference.

What the what?? I literally JUST ran a query exactly like this, but without the join. I haven’t mixed aggregate and non-aggregate columns in the query without a GROUP BY…the only aggregate is in the subquery, and it’s all by its little lonesome!

Well, you likely read the blog title, so it’s no mystery to you: by adding that table alias, “D.”, to the subquery, I accidentally made this a correlated subquery…the thing was now trying to pull MAX(ExecutionDateTime) from that outer table, instead of the inner table.  That’s not at ALL what I wanted to do!

The fix here is easy – just take out that table alias from the subquery:

  SELECT D.ExecutionDateTime
    , D.STATUS
    , D.PctComplete
    , D.DBName
    , D.DBType
    , D.BackupType
    , D.BackupStartDateTime
    , D.BackupEndDateTime
 FROM Minion.BackupLogDetails as D
 JOIN #Backups as B
 ON D.DBName = B.DBName
 WHERE D.ExecutionDateTime < ( SELECT MAX(ExecutionDateTime)
      FROM Minion.BackupLogDetails );

If you wanted to be really really pedantic, you could of course add a new table alias to the subquery table – say, “sub” – and then select MAX(sub.ExecutionDateTime), etc etc. It’s all up to you. Just, you know, don’t accidentally correlate when you didn’t meant to!

Happy days,
Jen McCown

XP_CmdShell isn’t Evil

Bonus summary: xp_cmdshell is limited to admins, unless you specifically grant permissions to users. And if you’re an admin, you have the power to turn on and use xp_cmdshell anyway.  Xp_cmdshell is not a security hole.

This is a reprint of Sean McCown’s original post on DBARant. You know, reprinted with permission and all that.

I’ve been hearing it more and more the past year.
“XP_cmdshell should always be turned off.”
“Whatever you do, don’t turn on XP_cmdshell!”
“We can’t do that, it requires XP_cmdshell!”
“You’ll fail your audit if XP_cmdshell is turned on.”
And all the other variations.

And I suppose I’ve been hearing it more and more lately because Minion Reindex requires it and Minion Backup will require it even more so.

However, I’ll tell you I’m getting pretty tired of hearing it so true to my blog I’m going to rant.
XP_cmdshell has been around forever. And way back in the day, like 15-20yrs ago, it was installed wide open to the public. This is where the problem started. This was back in the day when SQL’s GUI allowed way too many people who had no idea what they were doing to create and manage DBs. That ease of use was a huge part of SQL Server taking hold in the industry. However, with the product being that easy to use, a lot of these untrained DBAs had no idea XP_cmdshell was even there, so their instance was completely vulnerable and they didn’t even know it. Honestly, this was Microsoft’s fault. They should never have packaged up something that dangerous completely open to the public. But you know what, back then they were also installing sa with a NULL password by default too. And Oracle had their scott\tiger username\password combo, so MS wasn’t the only one doing dumb security back then.

However, now XP_cmdshell comes turned off and when you enable it, it’s not open to public anymore. So seriously, what are you still afraid of? I understand that you used to be scared of it because there was no way to lock it down back then. In fact, Microsoft didn’t provide a way to lockdown XP_cmdshell until somewhere in the neighborhood of version 4.2. So back when it was open to public I can see how writing a DENY statement would be really taxing to you as a DBA.
But these days you don’t have any excuses. You have to go out of your way to open it up to public. XP_cmdshell is still really useful and I’m personally able to create many excellent solutions using it… things that would be much more difficult otherwise. And do you know what I tell people who tell me how dangerous it is? I ask them why they don’t lock it down.

Think about it… there are many dangerous features in SQL. And they’re all kept in check by controlling permissions to them. You don’t see anyone screaming that those other features should be allowed on the box because they just say, we use it but we keep its usage controlled pretty tightly. So why doesn’t that apply to XP_cmdshell? Do you think that SQL all of a sudden forgets how to deny execute perms when that gets called? Do you think that SQL honors all security except that one? Do you think XP_cmdshell is powerful enough to override SQL security and just do what it wants anyway?
Of course not. So what are you afraid of?

The truth is that XP_cmdshell can do a lot and in the wrong hands it can make a royal mess of things. Then again so can DELETE and UPDATE. So can SHUTDOWN. So can CLR. So can DROP DATABASE. So can Dynamic SQL. And you don’t see anyone saying that all of those should never be allowed on any server for any reason. And I would honestly venture to say that Dynamic SQL has been the cause of far more security breaches than XP_cmdshell ever has. I don’t have any numbers to back me up, but I bet if you look at the number of security issues caused by XP_cmdshell, they’re far out-weighed by other features.

And it’s not like people have to way to get that functionality just because XP_cmdshell is disabled. There are still cmdline job steps and cmdline SSIS tasks. And of course, you’ve got CLR. All of which can be just as dangerous as XP_cmdshell yet they run on systems all the time. And I know what you’re thinking… “But Sean, we control those through permissions so they can’t do anything really bad.” Yeah, so you’re making my point for me. But do you think that if an SSIS guy wanted to do something bad to your box that he couldn’t find a way if he weren’t locked down? Of course he could.

The cool thing about the cmdline task in Agent jobs is that they can be run via proxy. You can setup a proxy user to run that step under so that its Windows perms are limited and it can’t run haywire. You wanna hear a secret? There’s a built-in proxy mechanism for XP_cmdshell too. I could tell you how to do it, but DatabaseJournal has already done such a fine job. So here’s the link to setting up the cmdshell credential.

I don’t want you to just turn on XP_cmdshell on all of your systems for no reason. But I don’t want you to completely rule it out as a solution just because you’re afraid of it. Tell your Windows admins who are afraid of it to mind their own business and stick to what they know. You’re a DBA and it’s time for you to take back your SQL instances. Lock them down. Don’t be afraid to use cool functionality because so many people refused to read the documentation 20yrs ago. You know better now. So go out there and do the right thing. Lockdown XP_cmdshell, but use it.

Announcement: Backup(n.) vs Back Up(v.)

Let it be known that the word “backup” is a noun (it refers to a thing), and “back up” is a verb (it refers to an action.

  • I’m going to back up the database.
  • It will produce a backup.
  • I’ll save that backup until we back up the database three more times.

This is how the terms are used throughout Books Online. I don’t really care if you use it wrong. I just thought you should know.

He was head of Books Online for half of forever. So, yeah.

Real news, real tech.