-
Notifications
You must be signed in to change notification settings - Fork 760
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
Update IndexOptimize.sql #848
Open
michele-tahay-kohera
wants to merge
2
commits into
olahallengren:main
Choose a base branch
from
michele-tahay-kohera:patch-1
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Update IndexOptimize.sql #848
michele-tahay-kohera
wants to merge
2
commits into
olahallengren:main
from
michele-tahay-kohera:patch-1
+3
−1
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Included lock timeout setting on fetching details about indexes/statistics. Even with the transaction level to readuncommitted, I had an issue on my environment that locked this query for 15 hours because of another process taking that long (an issue on its own :) ).
variable @CurrentCommand should be initialized
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Query Optimization and Index Selection Logic
Strengths:
- Comprehensive Query Construction:
o The query dynamically builds a @CurrentCommand string that adapts based on the provided conditions. This approach adds flexibility to execute specific commands such as setting lock timeouts or transaction isolation levels.
o The SELECT statement includes a broad range of index and table-related properties, making it a powerful tool for index diagnostics and optimization. - Well-Defined Conditional Logic:
o The use of conditions ensures that the logic only executes when specific parameters (@ActionsPreferred or @UpdateStatistics) are present or within a time limit. This ensures resource-efficient execution and aligns well with good database practices. - Attention to Detail:
o Properties like IsMemoryOptimized, AllowPageLocks, and ResumableIndexOperation show that you’re considering modern SQL Server features and edge cases.
Suggestions for Improvement: - Code Readability:
o Consider splitting long lines of code for better readability. For example, break the SET @CurrentCommand assignments into multiple lines using string concatenation.
o Adding comments about what each major section of the code does will make it easier for collaborators to follow. - Dynamic SQL Risks:
o Ensure that any dynamic SQL generated by @CurrentCommand is thoroughly validated to mitigate the risk of SQL injection or unintended consequences. - Error Handling:
o Adding error-handling mechanisms (e.g., TRY...CATCH blocks) can improve the robustness of the script. - Logging and Debugging:
o Incorporate logging for debugging purposes. Logging the generated @CurrentCommand could be useful for troubleshooting. - Performance Optimization:
o Since this query accesses system views and metadata, ensure that it performs well under heavy loads. Test it in a production-like environment if possible.
Additional Notes:
• The inclusion of index-specific columns like IndexType, IsColumnStore, and StatisticsID indicates that this script could be useful for tasks such as performance tuning or index management.
• If this script will be frequently reused, consider encapsulating it in a stored procedure or a reusable function to enhance modularity.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Strengths:
- Use of Conditional Execution: The inclusion of conditions such as checking for @ActionsPreferred or @UpdateStatistics ensures that the script only executes under specific scenarios, which is a good practice to avoid unnecessary processing.
- Dynamic Command Building: The use of dynamic command building with @CurrentCommand allows flexibility in setting up configurations like LOCK_TIMEOUT and TRANSACTION ISOLATION LEVEL. This modularity enhances the script’s adaptability.
- Comprehensive Column Selection: The SELECT statement includes a detailed list of columns, which demonstrates thoroughness in extracting metadata or performance-related details about database objects.
Suggestions for Improvement: - Code Readability:
o Consider breaking the dynamic command into smaller, clearly labeled sections. This will make the script easier to read and maintain. For example, you could create separate variables for each command part.
o Add comments to explain the purpose of each block of code. - Lock Timeout Logic:
o Validate that the multiplication by 1000 (to convert seconds to milliseconds) aligns with the intended behavior for all possible @LockTimeout values. It might help to add a check to handle edge cases like NULL or negative values. - Use of READ UNCOMMITTED Isolation Level:
o While the READ UNCOMMITTED isolation level is efficient, it can lead to dirty reads. Confirm that this behavior is acceptable for the intended use case and document the reason for this choice in a comment. - Error Handling:
o The script currently lacks error handling. Consider wrapping the dynamic SQL execution in a TRY...CATCH block to log or handle potential issues during execution. - Parameter Validation:
o Add input validation at the beginning of the script to ensure that parameters like @ActionsPreferred, @UpdateStatistics, @Timelimit, and @starttime are not NULL or out of expected ranges unless intended. - Formatting the SELECT Statement:
o For better readability, align the selected columns vertically in a structured format.
Example:
o SELECT SchemaID,
o SchemaName,
o ObjectID,
o ObjectName,
o ObjectType,
o ...
Additional Enhancements:
• If this script is intended for long-term use or frequent modification, consider parameterizing it further and documenting its usage in an external file or header comment block.
• Integrate logging mechanisms to track the execution of the script for auditing purposes.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Included lock timeout setting on fetching details about indexes/statistics. Even with the transaction level to readuncommitted, I had an issue on my environment that locked this query for 15 hours because of another process taking that long (an issue on its own :) ).