3

Background
My application is backed up by an SQL Server (2008 R2), and have quite a few SP, triggers etc..
My goal is to make sure upon program start that all of those objects are still valid.
For example, if I have a stored procedure A which calls stored procedure B, If someone changes the the name of B to C, I would like to get a notification when running my application in Debug environment.

What have I tried?
So, I figured using sp_refreshsqlmodule which according to the documentation returns 0 (success) or a nonzero number (failure):

DECLARE @RESULT int 
exec @RESULT  = sp_refreshsqlmodule N'A' --In this case A is the SP name
SELECT @@ERROR
SELECT @RESULT 

So I changed SP B name to C and ran the script. The results where:

  • @@ERROR was 0
  • @RESULT was 0
  • I got a message of:

The module 'A' depends on the missing object 'B'. The module will still be created; however, it cannot run successfully until the object exists.

My question:
Am I missing something here, shouldn't I get anon-zero number that indicates that something went wrong?

Avi Turner
  • 10,234
  • 7
  • 48
  • 75
  • Even though the message mentions issues, it also reads: "The module will still be created". That doesn't sound like a "failure" to me, so I can see no problem with `sp_refreshsqlmodule` returning 0 in this case. – Andriy M Feb 28 '14 at 11:53
  • @AndriyM I would expect it to return some non zero value that will indicate that not everything is 100% OK... – Avi Turner Feb 28 '14 at 14:18
  • Well, that would probably be helpful, but the manual clearly says that a nonzero results indicates a failure rather than "not everything is 100% OK". Apparently you and the manual have different views on the question of failure. :) – Andriy M Feb 28 '14 at 14:39
  • Seems related to the halting problem. Given the fact that you could be using dynamic sql in a sproc, I feel that this is not always a reasonable question. – Aron Apr 25 '14 at 10:54

6 Answers6

2

Assuming that all of your dependencies are at least schema qualified, it seems like you could use sys.sql_expression_dependencies. For instance, running this script:

create proc dbo.B
as
go
create proc dbo.A
as
exec dbo.B
go
select OBJECT_SCHEMA_NAME(referencing_id),OBJECT_NAME(referencing_id),
   referenced_schema_name,referenced_entity_name,referenced_id
from sys.sql_expression_dependencies
go
sp_rename 'dbo.B','C','OBJECT'
go
select OBJECT_SCHEMA_NAME(referencing_id),OBJECT_NAME(referencing_id),
   referenced_schema_name,referenced_entity_name,referenced_id
from sys.sql_expression_dependencies

The first query of sql_expression_dependencies shows the dependency as:

(No Column name) (No Column name) referenced_schema_name referenced_entity_name referenced_id
dbo              A                dbo                    B                      367340373

And after the rename, the second query reveals:

(No Column name) (No Column name) referenced_schema_name referenced_entity_name referenced_id
dbo              A                dbo                    B                      NULL

That is, the referenced_id is NULL.


So this query may find all of your broken stored procedures (or other objects that can contain references):

select OBJECT_SCHEMA_NAME(referencing_id),OBJECT_NAME(referencing_id)
from
    sys.sql_expression_dependencies
group by
    referencing_id
having SUM(CASE WHEN referenced_id IS NULL THEN 1 ELSE 0 END) > 0
Damien_The_Unbeliever
  • 234,701
  • 27
  • 340
  • 448
  • Thanks for your reply. The only problem with your suggestion is that I have many procedures, some of them do not depend on any other procedures. This will result that both procedures with broken dependencies and procedures with no dependencies will have a `referenced_id = NULL`. I do not want to go over them one by one manually in order to check which are broken and which are not... – Avi Turner Apr 24 '14 at 08:34
  • @AviTurner - there's one row in this view for each dependency that an object has. So the row I'm referring to is the row for the dependency from `dbo.A` to an object called `dbo.B`. In the second select, this reference is broken. There are no rows in this view for the `dbo.B` stored procedure that has no dependencies. – Damien_The_Unbeliever Apr 24 '14 at 08:42
  • I will check your solution more thoroughly when I get the chance. When From a first glance, when I run it, it seems that it gives me many results that seems to be a false positive. – Avi Turner Apr 24 '14 at 08:56
  • Well, is my first assumption, in my first sentence, valid? Are your dependencies schema qualified? Because if you just have `exec B`, there's no way to know until runtime whether a valid `B` will exist. – Damien_The_Unbeliever Apr 24 '14 at 09:03
  • Well it is not... Sorry for not noticing earlier. although I think I'm using only the default schema. Would this suffice? In the script you published I get only `dbo` for `OBJECT_SCHEMA_NAME(referencing_id)` and `dbo` or `NULL` for `referenced_schema_name`. – Avi Turner Apr 24 '14 at 09:40
  • @AviTurner - those ones you're finding where the `referenced_schema_name` is `NULL` might already be considered to be broken - the system will decide at runtime which procedure to execute, and it may not be the one that you're expecting, if someone adds a new `B` procedure in another schema and that's their default schema. Whether you want to go through all of your existing procedures and fix them is a question of time that only you can decide. – Damien_The_Unbeliever Apr 24 '14 at 09:56
  • I see your point as far as it goes for the validity check. However, I am the only one that handles the DB so I know as a fact that there isn't and most probably won't be another schema so I am not worried about calling the wrong procedure. Unfortunately, at this point, going over all procedures and fixing them is out of the question... If I'll have the time to spare in the future I might go for it... – Avi Turner Apr 24 '14 at 10:16
  • 2
    @AviTurner: Until then consider them as broken. :) It is really a good idea to [specify the schema always](https://sqlblog.org/2019/09/12/bad-habits-to-kick-avoiding-the-schema-prefix). – Andriy M Apr 25 '14 at 17:21
  • This is the answer that gave me the most insight and understanding about what I should do. It didn't really solve my issue, but I now realize that this is due to bad habits of the past. I will accept this answer, and the bounty is yours as well... – Avi Turner Apr 30 '14 at 08:24
2

You can try this. It may not be 100% for schema (it has owner name below) as it was based more when I worked with SQL Server 2000, but I tested it 2008, it basically runs alter statement on all the procs, functions, views. Comment out the PRINT @objName + ' seems valid.' to see only invalid procs, functions, views... Feel free to edit any parts you want!

DECLARE @objId INT
DECLARE @objName NVARCHAR(max)
DECLARE @owner NVARCHAR(255)
DECLARE @Def nvarchar(max)

DECLARE checker CURSOR FAST_FORWARD FOR
    SELECT
        id, name, USER_NAME(o.uid) owner 
    FROM sysobjects o 
    WHERE   o.type IN ('P', 'TR', 'V', 'TF', 'FN', 'IF')
            AND o.name <> 'RecompileSQLCode'

OPEN checker
FETCH FROM checker INTO @objId, @objName, @owner

WHILE @@FETCH_STATUS=0
BEGIN
    SELECT @Def = definition      
    FROM sys.sql_modules 
    WHERE object_id = @objId


       --print @objName
       --print @def
       SET @def = REPLACE(@def, 'create procedure','alter procedure')
       SET @def = REPLACE(@def, 'create PROC','alter PROC')
       SET @def = REPLACE(@def, 'create trigger','alter trigger')
       SET @def = REPLACE(@def, 'create function','alter function')
       SET @def = REPLACE(@def, 'create view','alter view')
    BEGIN TRANSACTION
        BEGIN TRY
            EXEC sp_executesql @def
            PRINT @objName + ' seems valid.'
        END TRY
        BEGIN CATCH
                PRINT 'Error: ' + @objName + ' : ' + CONVERT(nvarchar(10), ERROR_NUMBER()) + ' ' + ERROR_MESSAGE()
        END CATCH
    ROLLBACK

    FETCH NEXT FROM checker INTO @objId, @objName, @owner
END

CLOSE checker
DEALLOCATE checker
Jim
  • 2,034
  • 1
  • 22
  • 43
MacWise
  • 520
  • 3
  • 13
  • Let me know if you were able to test it. I would be interested to get your feedback on it. Thanks! – MacWise Apr 30 '14 at 05:03
  • Update: first of all, I must say, this is a nice piece of code... I slightly changed it (see the edit to the post). I am still getting some errors. I will check if they are real issues or a false positive (The errors are: `Error: xyz: 2714 There is already an object named 'xyz' in the database.`). I will update you with the results. – Avi Turner Apr 30 '14 at 06:16
  • Thanks for the feedback, Ah, I probably should have put a REPLACE for each type, like 'create trigger','alter trigger', and the functions as well probably for each type in the IN clause – MacWise Apr 30 '14 at 06:26
  • OK, I have checked your solution and It turned out the although we have renamed `B` To `C`, the `definition` value in table `sys.sql_modules` for `C` is still `CREATE PROC B`. So this will fail you test when you try to alter SP C... Thanks though, that was a good effort. – Avi Turner Apr 30 '14 at 08:21
1

Here is a procedure that will script all of the stored procedures on your server as CREATE procedures with a Suffix to the name. The script created a corresponding DROP PROCEDURE for the 'TEMP/Test' procedure.

This will not confirm if a stored proc is referencing an invalid table name, as the normal create of a stored proc does not validate for this.

BEGIN TRAN

--Creating temp able with copy of all procedures
 DECLARE @tTempProcedures TABLE
 (
    ProcedureName NVARCHAR(MAX),
    OriginalProcCreateSQL NVARCHAR(MAX),
    CreateNewProcSQL NVARCHAR(MAX),
    DropTestProcedureSQL NVARCHAR(MAX),
    AllInOneSQL NVARCHAR(MAX)
 )

 INSERT INTO @tTempProcedures
 SELECT 
         procedures.name                                AS  ProcedureName         
        ,syscomments.Text                               AS  OriginalProcCreateSQL
        ,REPLACE(syscomments.Text
                ,procedures.name
                ,procedures.name + '_TEST_CREATE')
            + ' GO'                                     AS  CreateNewProcSQL
        ,'DROP PROCEDURE ' 
            + procedures.name 
            + '_TEST_CREATE'                         AS  DropTestProcedureSQL

    ,'EXEC sp_executesql ' +''''''+
        REPLACE(
            REPLACE(syscomments.Text
                    ,procedures.name
                    ,procedures.name + '_TEST_CREATE')
                ,''''
                ,'''''')

        +''''''
        +  CHAR(10) + CHAR(13) 

        +  CHAR(10) + CHAR(13) 
        + 'EXEC sp_executesql ' +''''''+ 'DROP PROCEDURE ' 
        +  procedures.name 
        +  '_TEST_CREATE' +''''''
        +  CHAR(10) + CHAR(13)                  
                                                AS  AllInOneSQL
FROM
    syscomments

    Inner Join sys.procedures
    ON syscomments.id = procedures.OBJECT_ID






DECLARE cur CURSOR FOR 
SELECT AllInOneSQL FROM @tTempProcedures 

OPEN cur
DECLARE @AllInOneSQL NVARCHAR(MAX)
FETCH NEXT FROM cur INTO @AllInOneSQL

WHILE (@@FETCH_STATUS = 0)
BEGIN
    PRINT(@AllInOneSQL)
    EXEC sp_executesql @AllInOneSQL

    FETCH NEXT FROM cur INTO @AllInOneSQL
END

CLOSE cur
DEALLOCATE cur


ROLLBACK

WARNING: Please be careful using any DROP PROCEDURE Statements.

NOTE: You can also use : "SET NOEXEC ON" and then execute the procedure. If the procedure is invalid you will get errors. If the procedure is valid no records will be updated after setting "SET NOEXEC ON". This is however difficult to automate as you need to call the proc with valid parameters.

Avi Turner
  • 10,234
  • 7
  • 48
  • 75
Richard Vivian
  • 1,700
  • 1
  • 14
  • 19
  • Thanks for your input. At first - your solution looked promising, I Modified `AllInOneSQL` to wrap the `create` and `drop` statements with `EXEC sp_executesql` instead of the `GO`s (`EXEC sp_executesql` cannot execute a statement with `GO`s). But then I encountered an issue of procedures that had an Apostrophe (`'`) commented out which broke my scripts. I have edited your script so you can see the issue. Fell free to role it back If I did missed something in your intention. – Avi Turner Apr 27 '14 at 07:05
  • I like the cursor and approach. Makes the script more useful. I have also considered adding better error handling into the process. In further testing I also noted that it does not detect where the proc references columns and tables that dont exist. I think this is a limitation in the validation process. How I am planning on handling this at our site is adding a logging function in all procs to a central error logging DB. That will show me when and where an invalid proc was called. Not as pro-active as I would like though. – Richard Vivian Apr 29 '14 at 07:04
0

Strange indeed, I've tried myself and result of sp_refreshsqlmodule is not consistent. Even strange is a transaction remains open in case an error appears, that's I've added ROLLBACK TRAN. Here it is an alternative:

DECLARE
    @is_refresh_ok AS BIT = 0
    , @error_message VARCHAR(MAX)

BEGIN TRY       
    EXEC sp_refreshsqlmodule '<SP name here>'       
    SET @is_refresh_ok = 1
END TRY
BEGIN CATCH
    SET @error_message = ERROR_MESSAGE()

    IF @@TRANCOUNT > 0
    ROLLBACK TRAN
END CATCH   

SELECT @is_refresh_ok, @error_message

In case you need, here it is a script that automatically refresh all stored procedures and functions in a database.

bjnr
  • 3,353
  • 1
  • 18
  • 32
  • 1
    Thanks for your answer. This is what my script looks like more or less. The reason I posted the question was that it was nor giving the expected result... – Avi Turner Feb 27 '14 at 21:34
0

The text of the SP is parsed when the CREATE PROCEDURE statement is executed but external name resolution is deferred until run-time. This allows for, say, circular dependencies between objects and avoids the necessity of having release scripts structured just so. Here's a technet link on the subject. I could see how sp_refreshsqlmodule could re-parse the text of the SP and extract its meta data successfully, reporting 0, but still not bind it to the dependent objects. This is another SO question dealing with the topic.

I've had some success with SQL parsers (SO questions here and here) in other situations. You may be able to capture the EXEC statements and list associated SP name.

Community
  • 1
  • 1
Michael Green
  • 1,397
  • 1
  • 17
  • 25
0
SELECT
  OBJECT_NAME(referencing_id) AS [this sproc or view...],
  referenced_entity_name AS [... depends on this missing entity name]
FROM sys.sql_expression_dependencies
WHERE is_ambiguous = 0
  AND OBJECT_ID(referenced_entity_name) IS NULL
  AND referenced_entity_name NOT IN
    (SELECT Name FROM sys.types WHERE is_user_defined = 1)
ORDER BY OBJECT_NAME(referencing_id), referenced_entity_name

I hope this helps. -Thomas

RosSQL
  • 325
  • 1
  • 5