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

Update SSDB.non-service_accounts.sql #6

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

nextaxtion
Copy link

No description provided.

Copy link

@Imran-imtiaz48 Imran-imtiaz48 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This SQL script efficiently audits database ownership, user roles, agent jobs, and report subscriptions by leveraging conditional filters, joins, and aggregations. It ensures that only valid accounts are included by excluding service accounts and filtering based on a dynamic login name parameter, $(login_name).
Feedback:

  1. Code Structure & Readability:
    o The structure is clean, and logical grouping of sections like "check databases users," "check agent jobs," and "check report subscriptions" makes it easy to follow.
    o Using meaningful aliases (e.g., db, sl, dp, dm) keeps the code compact, but a comment for each section would improve readability further, especially for junior DBAs.
  2. Efficiency & Performance:
    o Great job using JOIN clauses to retrieve associated data across various system tables. These joins are essential for accurate reporting and improve performance compared to subqueries in most cases.
    o The use of GROUP BY and ORDER BY is efficient, helping to ensure organized output. Ensure #service_accounts has necessary indexes if it’s frequently used for exclusions to avoid scan operations.
  3. Parameterization:
    o Leveraging $(login_name) as a filter parameter offers flexibility, making this script versatile for different scenarios.
    o Consider adding a DEFAULT value if this parameter isn’t defined to handle cases where no specific login is targeted.
  4. Error Handling & Edge Cases:
    o The script’s handling of NULL values, especially in sections where dp.[sid] is checked, helps avoid errors related to missing or NULL principals.
    o Be cautious with LIKE ''##%''' in role checks. This is generally effective but could benefit from a comment or explanation, especially if it targets a specific naming convention.
  5. Collation Sensitivity:
    o Good handling of collation differences with the COLLATE clause on ou.[UserName]. It ensures consistent results when comparing string data across different collation settings, an essential practice for multi-collation environments.
  6. Suggestions:
    o Consider wrapping the script in a stored procedure or encapsulating each section as functions or views if this will be a reusable audit tool.
    o Adding comments for complex joins or conditional logic would aid future maintainers in understanding the intent of each section more quickly.
    Overall:
    This is a well-crafted and thorough script that captures essential audit data across multiple dimensions. With some additional commenting and minor adjustments, it could serve as a robust solution for ongoing database and user audits.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants