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

sp_helpme - add incl col #267

Merged
merged 34 commits into from
Jan 24, 2023
Merged

sp_helpme - add incl col #267

merged 34 commits into from
Jan 24, 2023

Conversation

lowlydba
Copy link
Owner

@lowlydba lowlydba commented Jan 8, 2023

Add included columns to the standard output from sp_helpindex.

Description

I decided to keep in place the original sp_helpindex since this is a small addition and there are some very specific ways that sp_helpindex works. With a focus on keeping the "drop in replacement" functionality I think it is better to keep the overall behavior, error reporting, etc. as original as possible/reasonable.

Unrelated:

  • Remove Appveyor CI
  • Enhance unit tests
  • Update copyright year on all procs
  • Clarify README support section
  • Fix SSL issues

Issue: Fixes #264

How Has This Been Tested?

MARs are hard to test, so this was tested manually.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)

Checklist:

  • I have updated the Version number of any scripts modified.
  • I have updated the documentation accordingly. - See lowlydba/dba-multitool.org@33e6aad
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@github-actions
Copy link

github-actions bot commented Jan 8, 2023

✅ TSQLLint Summary

Linted 6 files in 1.0591484 seconds

0 Errors.
0 Warnings.

📄 Full job results.

♻️ This comment has been updated with latest results.

@lowlydba lowlydba changed the title Helpme add incl col sp_helpmre - add incl col Jan 8, 2023
@lowlydba lowlydba changed the title sp_helpmre - add incl col sp_helpme - add incl col Jan 8, 2023
@github-actions
Copy link

github-actions bot commented Jan 8, 2023

badge

Code Coverage Summary

Filename                    Stmts    Miss  Cover    Missing
------------------------  -------  ------  -------  -----------------------------------------------------------------------
[dbo].[sp_doc]                 91       3  96.70%   93-94, 391
[dbo].[sp_estindex]           207       5  97.58%   186, 739-744
[dbo].[sp_helpme]              93       1  98.92%   337
[dbo].[sp_sizeoptimiser]      240      18  92.50%   145, 178, 226, 241, 661, 871, 926, 930, 934, 938, 1078, 1139, 1221-1226
TOTAL                         631      27  95.72%

Results for commit: 76dbe9d

Minimum allowed coverage is 90%

♻️ This comment has been updated with latest results

@lowlydba
Copy link
Owner Author

lowlydba commented Jan 8, 2023

@mattcargile Can you check if the version in this branch fits your needs?

@mattcargile

This comment was marked as outdated.

@mattcargile

This comment was marked as outdated.

@mattcargile
Copy link
Contributor

mattcargile commented Jan 9, 2023

I assume folks like to use this procedure from master. My issue is that I have the procedure stored in master. Based on this article, it could be resolved with sp_ms_marksystemobject. I am not super keen on using this though. I'd rather see the dynamic SQL avenue used with DB_NAME().

@lowlydba
Copy link
Owner Author

lowlydba commented Jan 9, 2023

Oof this is quickly becoming a pain. Thanks for diving in.

I agree that dynamic seems like maybe the best route to go next, I'll give that a shot and see where it gets us.

@lowlydba
Copy link
Owner Author

@mattcargile How's this looking now?

@mattcargile
Copy link
Contributor

mattcargile commented Jan 16, 2023

It is working great.

I had looked at it a couple days ago and left a review above about the sp_executesql context. I didn't realize the #sp_helpindex would exist. I think I was thinking about the new permissions context with sp_executesql instead. Additionally, I don't think I even submitted the review for you to see. I think it was only in Pending.

@lowlydba
Copy link
Owner Author

Great! Thanks for the feedback.

I'll try to merge and release this week.

@lowlydba lowlydba merged commit beb9f04 into main Jan 24, 2023
@lowlydba lowlydba deleted the helpme-add-incl-col branch January 24, 2023 15:55
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.

[Request] sp_helpme - Add Included Columns for INDEX on TABLE
2 participants