Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sp_BlitzIndex -- code review to make sure we're joining on database ID #711

Closed
BlitzErik opened this issue Feb 14, 2017 · 3 comments
Closed
Assignees

Comments

@BlitzErik
Copy link
Contributor

Do you want to request a feature or report a bug?
Potential bug

What is the current behavior?
Since adding logic to check all databases, many joins now need to also join on database ID (don't get me started on schema ID -- most tables don't include that!)

When checking a single database, you get (okay, like 98%) correct results, but when checking all databases it's possible to get cross-database pollution when joins don't include database ID. For instance, some queries only join on object ID. This can overlap between databases. Boo and hiss.

What is the expected behavior?
Add safety joins on database ID to queries that carry it.

Which versions of SQL Server and which OS are affected by this issue? Did this work in previous versions of our procedures?
All.

@BlitzErik BlitzErik self-assigned this Feb 14, 2017
BrentOzar added a commit that referenced this issue Mar 2, 2017
Before, was producing duplicate results across databases. Closes #732.
Whispers sweet nothings to #711 but doesn’t fix it yet.
@BlitzErik
Copy link
Contributor Author

Doing a little recon on this one. Here are all the necessary tables to look at. Thankfully not many are missing schema name. We need schema names for cases where there are multiple schema in a database where tables have the same names. There was an open issue a while back to have this run against a single schema, which isn't happening here, but we'll be a little bit closer if it becomes a requested feature.

The easy part of this is changing the temp tables and adding the info to the initial inserts. The annoying part is going to be going through all the checks and adding additional joins.

#IndexSanity: [database_id]
Has schema name

#IndexPartitionSanity: [database_id]
No schema name

#IndexSanitySize: [database_id]
No schema name

#IndexColumns: [database_id]
No schema name

#MissingIndexes: [database_name]
Has schema name

#ForeignKeys: [database_name]
No schema name

#Statistics: database_name
Has schema name

#ComputedColumns: database_name
Has schema name

@BrentOzar
Copy link
Member

I salute you for doing this, sir.

@BlitzErik
Copy link
Contributor Author

Cool, so the update for this join now having database id and schema name in it makes for a lot less work in later checks where we join on these IDs.

image

BrentOzar added a commit that referenced this issue Apr 6, 2017
Some of the checks queries weren’t joining on database_id, so they
produced unpredictable results when objects in different databases had
the same object_id. Closes #711.
BlitzErik added a commit that referenced this issue Apr 6, 2017
#711 sp_BlitzIndex adding joins on database_id
BrentOzar added a commit that referenced this issue Apr 7, 2017
Erik’s fixes for #711 merged back into dev.
BrentOzar added a commit that referenced this issue Apr 7, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants