Get-DbaDbStoredProcedure: Moved Test-Bound -ParameterName ExcludeSystemSp outside of nested loop

[ReeceGoding]

Type of Change

• Bug fix (non-breaking change, fixes Get-DbaDbStoredProcedure seems to hang after first database #9418 )
• New feature (non-breaking change, adds functionality, fixes # )
• Breaking change (affects multiple commands or functionality, fixes # )
• Ran manual Pester test and has passed (.\tests\manual.pester.ps1)
• Adding code coverage to existing functionality
• Pester test is included
• If new file reference added for test, has is been added to github.com/dataplat/appveyor-lab ?
• Unit test is included
• Documentation
• Build system

Purpose

As I have complained about in #9418 , I have some concerns about the performance of Get-DbaDbStoredProcedure. This is my best guess at what will fix it. Running Test-Bound (which is itself a loop) inside of a nested loop despite its value being set from the moment you call the function seems like something that will negatively impact performance, so this fixes that. Running it inside the loops means running it a great deal of times. Over 1,000 per database by my count.

Approach

Aside from what I've said above, all that I can mention is to stress that this is a guess. It is an edit made in GitHub's GUI after a late-night eureka moment. I don't have any skill in measuring PowerShell performance.

Commands to test

Get-DbaDbStoredProcedure

[Imran-imtiaz48]

Review:

Overall Feedback:

The refactoring improves the code's readability and efficiency. The check for the bound parameter ExcludeSystemSp has been moved outside the loop, which is a great optimization.

Positives:

1. Optimization:

   • Moving the $ExcludeSystemSpIsBound check outside the loop avoids redundant checks, enhancing performance.

2. Code Readability:

   • The code is now more readable with the reduced nesting and fewer inline checks.

3. Consistency:

   • The consistency in coding style and indentation has been maintained, making the script easy to follow.

Areas for Improvement:

1. Error Handling:

   • Consider adding error handling for the Get-DbaDatabase function to manage scenarios where the database retrieval fails.

2. Comments and Documentation:

   • Adding comments to explain the purpose of key sections and the reason behind moving the parameter check outside the loop would benefit future maintainers.

Suggestions:

• Logging:
  Including more detailed logging or messages for each major step can help in troubleshooting and understanding the script flow, especially in larger environments.

• Parameter Validation:
  Adding parameter validation at the beginning of the function to ensure all necessary parameters are provided and valid.

Example:

Here's an example of adding error handling and comments:

try {
    $InputObject = Get-DbaDatabase -SqlInstance $SqlInstance -SqlCredential $SqlCredential -Database $Database -ExcludeDatabase $ExcludeDatabase
} catch {
    Write-Message -Level Error -Message "Failed to retrieve databases. Please check the SQL instance and credentials."
    return
}

$ExcludeSystemSpIsBound = Test-Bound -ParameterName ExcludeSystemSp

foreach ($db in $InputObject) {
    if (!$db.IsAccessible) {
        Write-Message -Level Warning -Message "Database $db is not accessible. Skipping."
        continue
    }
    
    # Iterate through stored procedures
    foreach ($proc in $procs) {
        if ($ExcludeSystemSpIsBound -and $proc.IsSystemObject) {
            continue
        }
        # Additional processing here
    }
}

---

Overall, the refactoring is a positive change that optimizes the script. Implementing the suggested improvements would further enhance the robustness and maintainability of the code.

[potatoqualitee]

sweet, hopefully this works @ReeceGoding. If not, it may be SMO. Sprocs have always been slow. Maybe this will help if yours is not effective enough.

https://stackoverflow.com/questions/401740/is-there-anyway-to-speed-up-sql-server-management-objects-traversal-of-a-existin