Closed BrentOzar closed 4 years ago
I'll try to push my version later today
I think I've managed to publish my changes. Tested on 2008 R2, 2017 and Azure, where it compiles ok. There are still things to fix, but I do not know how to.
OK, great. I verified that it compiles (but won't run) on Azure SQL DB and Hyperscale.
Just to set expectations though, for this to get merged in, sp_Blitz would actually need to run on Azure SQL DB. (I don't want to introduce a lot of changes only to get it to compile, but not run - changes = potential bugs, and there's a lot of code review that would need to be done here.)
If you want to keep working on getting it to run on Azure SQL DB, that's totally cool - when you've got it to run, and you've had at least a couple of people test it and verify that it runs, you can do a pull request from your repo up to the dev branch here. To do that, go to your branch with the fixes:
And click the "New pull request" button at the top. It should look like this - you're asking me to pull from issue_1970/HSP to the FRK's dev branch:
It should say "Able to merge. These branches can be automatically merged." If it doesn't, there was some kind of a breaking change, and we'll need to fix that before you can do a pull request. (Right now you're fine.)
However, just to be clear - it needs to run on Azure SQL DB before we can do a pull request into the dev branch. The dev branch is what other people work off of (like you) when they're getting ready to make changes, and we wouldn't want to introduce them to something that hasn't been tested yet.
Thanks sir!
I've found the first problem on Azure SQL DB with sp_blitz. I had run out of disk space the first time I ran Data Migration Assistant, and that must have left non-clustered indexes disabled, because sp_blitz reported lots of CheckId = 47 So I had to run this:
declare @sql nvarchar(max)='' select @sql += 'alter index ' + i.name + ' on ' + sc.name + '.' + t.name + ' Rebuild;' + char(10) FROM sys.indexes i INNER JOIN sys.tables t on t.object_id = i.object_id INNER JOIN sys.schemas sc on sc.schema_id = t.schema_id where i.is_disabled=1 exec sp_executesql @sql
fixed invalid cpu_count (thanks to Randi Vertongen)
Regarding: So gonna need to wrap that in dynamic SQL for it to work in both RDS and Azure SQL DB.
Instead of dynamic sql also a if-then-else of case-when-end construction could also be used. A lot of years ago I used this to differentiate between table names which were changed between version 2005 and 2008.
IF @@VERSION LIKE '%SQL SERVER 2008%'
BEGIN
SELECT suser_sname(ownersid), ownersid, name, description, *
FROM msdb.dbo.sysssispackages -- sql 2008
WHERE packagetype = 6 and ownersid <> 0x01
END
ELSE
IF @@VERSION NOT LIKE '%SQL SERVER 2000%'
BEGIN
SELECT suser_sname(ownersid), ownersid, name, description, *
FROM msdb.dbo.sysdtspackages90 -- sql 2005
WHERE packagetype = 6 and ownersid <> 0x01
END
Still a lot of work and almost duplicate SQL Queries which is not nice for maintenance.
Right, across thousands of lines of T-SQL, it's not realistic.
I'm going to go ahead and close this issue - I don't see us doing this anytime, ever.
Hello, I managed to make these scripts work on Azure SQL. I basically remove all related to HW since azure SQL is a PaaS and we don't have access to that part. I also updated some queries and removed references to master and msdb. This was a huge amount of change.
My next step is to split and add again all that was removed to make it works. In the near future is going to live as 2 separate script
for the long term, my idea is to encapsulate all that was removed in another SP. and add a flag based on the DB version and if is Azure SQL, don't install/use that SP
@felipeschneider88 OK, thanks, but before you do any more work, please read the contributing guide:
https://github.com/BrentOzarULTD/SQL-Server-First-Responder-Kit/blob/main/CONTRIBUTING.md
Before making changes, you'll want to start an issue here describing the changes you want to make so the community can have a discussion about the approach.
That refactoring sounds really ambitious. That's totally fine that you want to take it on, but I get really nervous about having that go into the First Responder Kit without having a quick proof of concept done first. The rest of us here have to support the changes, forever, and I want to make sure we're all on the same page about that new approach.
It does sound like you're already well on your way, so what you might want to do is host your own repo with the changes you've made. You're absolutely welcome to do that as long as you abide by the licensing.
Hello @BrentOzar, thanks for your response. Now is a WIP for my job but my idea is to share it with everyone. I have the code on my repository to avoid any possible issues. In my earlier comment, I was speaking out loud about the possible next steps. One way could be using a flag and encapsulating the removed code in another SP. I will we working with my version and testing for azure SQL
@felipeschneider88 Re: Hello, I managed to make these scripts work on Azure SQL. Do you have it on github. Can I use it ?
Version of the script SET @Version = '7.2'; SET @VersionDate = '20190128';
What is the current behavior?
These databases don't allow direct references to master, model, msdb, etc. We're referring to a bunch of system objects, so we would need to move all of these to dynamic SQL to even get sp_Blitz to COMPILE, let alone even run:
Note that we can't just remove the "master" part of "master.sys.all_objects" because we need to detect user objects in there, like in the Amazon RDS check:
AND EXISTS(SELECT * FROM master.sys.all_objects WHERE name IN ('rds_startup_tasks', 'rds_help_revlogin', 'rds_hexadecimal', 'rds_failover_tracking', 'rds_database_tracking', 'rds_track_change'))
This query was run in an Amazon RDS user database, and note that the second query doesn't return results:
So gonna need to wrap that in dynamic SQL for it to work in both RDS and Azure SQL DB.
This is going to be a ton of work. I'll leave it open in case someone from the community wants to tackle it, but I'm a hard no here. Just too much work.