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

New Check - Indexed view without a clustered index #302

Open
michalporeba opened this issue Feb 23, 2018 · 10 comments

Comments

Projects
None yet
5 participants
@michalporeba
Copy link
Collaborator

commented Feb 23, 2018

I don't want to get into the devops process as it is a process in itself that takes time, but what happens is I have a number of views with clustered indexes on them, then somebody alters one but forgets that there was an index, which now needs to be recreated. I will need to create such check for myself, so I thought I will put it out there to see if there would be any interest in it and how I could make it useful not only for myself.

Obviously there is no clear way to know if a view was supposed to have an index on it or not so I propose looking at the definition for a specific text. I would look for 'with schemabinding' as I use it only for indexes on which I want to build clustered indexes. But equally there could be a convention to add a comment in the definition /* INDEXED */ for example and any view with that in the definition would be expected to have at least a clustered index.

@SQLDBAWithABeard

This comment has been minimized.

Copy link
Collaborator

commented Feb 26, 2018

I like this, needs a little discussion about the best way forward @jpomfret do you have any thoughts on the best way forward?

@jpomfret

This comment has been minimized.

Copy link
Collaborator

commented Feb 27, 2018

I think the only way to find views that should have indexes on would be a specific tag as a comment, you could have 'with schemabinding' on a view that isn't necessarily supposed to have an index. @SQLDBAWithABeard how do you feel about tags like that in comments? The keyword could be specified in the config file, we'd want to make sure it was something pretty specific to avoid finding other objects accidentally, perhaps dbcIndexedView.

@SQLDBAWithABeard

This comment has been minimized.

Copy link
Collaborator

commented Feb 27, 2018

That works for me. It would need a new config item following the naming convention and referenced in the test and left null by default. I would also think that this test should have a skip config set to true by default

But its an excellent approach

@michalporeba

This comment has been minimized.

Copy link
Collaborator Author

commented Feb 28, 2018

I wonder how many people use with schema binding on views. I was thinking if it was configurable we could have a specific comment like /* DbcCheckIndexedView */ but if it was configurable, in my case I could just change it to 'with schemabinding` and use that without changing the existing objects.

Why would we have tests which are set to skip by default? Just out of curiosity, realy.

@jpomfret

This comment has been minimized.

Copy link
Collaborator

commented Mar 1, 2018

Since the comment would need to be added to view source code for the check to work it would out of the box it wouldn't do anything, that would be my guess.

@jpomfret

This comment has been minimized.

Copy link
Collaborator

commented Mar 1, 2018

@SQLDBAWithABeard do you think we need to add a new check file for this and other object level tests? Objects.Tests.Ps1?

@michalporeba

This comment has been minimized.

Copy link
Collaborator Author

commented Mar 1, 2018

@jpomfret I like the idea of a new tests group / tag / file. But perhaps call it Schema.Tests.ps1? With that not included in other tests automatically we don't need to have any new mechanism, configuration, to make the execution not happen by default.

I'd probably do that for performance reasons too. Schema checks will be more expensive than configuration checks and probably could be run on a different schedule. I suggested checking for schema changes (#303) which is a test that would go to the same Schema.Tests.ps1 file, and perhaps looking for duplicate or unusued indexes would go there too.

I'd prefer to go first, by default, to look for with schemabinding. That would allow people like myself who use schema bidingin on views only for clustered indexes, to just run the -Check schema and get the results. For people who use it for other purposes it would probably offer an easier way to explore and find the test. I'd imagine they run the same test and get an error message that an index might be missing on a view with a link to a wiki page (or help command) explaining this particular test and that it can either be disabled in configuration or it can be configured to use an alternative tag which then have to be added to the schema definition.

@SQLDBAWithABeard

This comment has been minimized.

Copy link
Collaborator

commented Mar 2, 2018

I very much want to keep confusion down and keep the number of test files small.
Is there a good reason why this cannot be in the database.tests.ps1 file?

@jpomfret

This comment has been minimized.

Copy link
Collaborator

commented Mar 2, 2018

No good reason, just wasn't sure on if it made sense to break it out.

@wsmelton

This comment has been minimized.

Copy link
Member

commented Mar 3, 2018

Why would you be looking or parsing the definition of an view to see if it contained specific text? You can hit Get-DbaDatabaseView and check IsSchemaBound

image

USE [dbatoolsci_lildb]
GO

/****** Object:  View [dbo].[View_1]    Script Date: 2018-03-02 10:03:23 PM ******/
SET ANSI_NULLS ON
GO

SET QUOTED_IDENTIFIER ON
GO

ALTER VIEW [dbo].[View_1]
with schemabinding
AS
SELECT        id, name
FROM            dbo.Example
GO
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.