microsoft / SqlNexus

SQL Nexus is a tool that helps you identify the root cause of SQL Server performance issues. It loads and analyzes performance data collected by SQL LogScout, SQLDiag or PSSDiag. It can dramatically reduce the amount of time you spend manually analyzing data.
MIT License
345 stars 95 forks source link

Rowset Editor #267

Closed moraja closed 8 months ago

moraja commented 8 months ago

Created Rowset Editor to help create and edit rowsets easily and visually.

PiJoCoder commented 8 months ago

Reviewing/testing:

  1. When I tried to create a new Rowset from Query feature, I got the following error when I tried to connect.

    Unable to load DLL 'Microsoft.Data.SqlClient.SNI.x86.dll': The specified module could not be found. (Exception from HRESULT: 0x8007007E)
    Connection String 
    Data Source=MYLAPTOP10;Initial Catalog=Master;Integrated Security=True;TrustServerCertificate=True

    Is there some prererquisite that needs to be installed or a project Reference that need to be added? I see no errors in my References so they seem to be all in place. So I cannot test this functionality currently.

  2. Return rowset - is a bit confusing. What do you mean by it? Perhaps "Select this rowset", or "Return to main screen", or "Done with definition", or "Done"...

  3. Corner scenario but calling it out as it is UX issue that misleads:

    • I press Save Rowset button
    • I choose Yes on Do you want to save your changes?
    • When the Save As (browse) window comes up, if I escape it or close it without saving, I still get success Document updated and saved successfully even though nothing was saved
  4. Would you consider adding a database to the "From Query " scenario because the query may need to be executed against a specific Db (say msdb or other). Or can the user do "use msdb" in the query window and do it that way? So just document it or use a "gray text" in the text window itself to guide the user?

  5. Could you resolve the conflicts that are showing up with Master? They seem to be just space differences, but feel free to bring in the lastest from master so we don't have an issue when merging

PiJoCoder commented 8 months ago

Tested the manual addition of columns and it works really well. Thank you @moraja for a great idea and implementation

moraja commented 8 months ago

@PiJoCoder

Thank you so much for testing this and your great feedback.

  1. I fixed the corner scenario, the truth is it did save, it just overwritten the source without consulting with you :)
  2. I fixed the button "Back to Main" now.
  3. The DB I can add it but then I need to add SMO, but I think simple use of "use dbName" in the query worked fine for me, can you test that?
  4. I am using Microsoft.Data.SqlClient which is a nuget package and the supported one, when I build the project the dll is there for me and in the release folder. Can you try installing Microsoft.Data nuget package ? although it hought this is not needed by other developers once installed on the main project.
  5. conflicts will be resolved soon
PiJoCoder commented 8 months ago

@PiJoCoder

Thank you so much for testing this and your great feedback.

  1. I fixed the corner scenario, the truth is it did save, it just overwritten the source without consulting with you :)
  2. I fixed the button "Back to Main" now.
  3. The DB I can add it but then I need to add SMO, but I think simple use of "use dbName" in the query worked fine for me, can you test that?
  4. I am using Microsoft.Data.SqlClient which is a nuget package and the supported one, when I build the project the dll is there for me and in the release folder. Can you try installing Microsoft.Data nuget package ? although it hought this is not needed by other developers once installed on the main project.
  5. conflicts will be resolved soon

Thank you. With the latest changes, I didn't have use a nuget package, it works out of the box. All the things work.

One more thing I found: when I clicked to save, I chose to over-write my TextRowsets.xml in the RowsetImportEngine folder. It made lots of changes (as compared to previous file). Most of them seem to be indentation or space related. But there's one I spotted that is concerning. It replaces -> with -&gt (html stuff).

image

It's a whole different conversation why anyone would choose to use -> as an identifier start. :)

Perhaps for now we will not be using the tool to over-write the core TextRowsets.xml but create a separate file and copy from it into the project. What do you think?

Thank you - it looks great. The "from Query" feature is pure magic.

moraja commented 8 months ago

@PiJoCoder

character is not allowed in Xml file, it is concerning that it was used inside TextRowset, also this rowset is duplicate, we have this one with tbl_loaded_modules and another with tbl_dm_os_loaded_modules.

so it shouldn't have existed :) but whichever way it works should be fine... the loss of indentation is true but the content remains the same.. either way it saves us a lot of time and encourages us to load more data .

PiJoCoder commented 8 months ago

@PiJoCoder

character is not allowed in Xml file, it is concerning that it was used inside TextRowset, also this rowset is duplicate, we have this one with tbl_loaded_modules and another with tbl_dm_os_loaded_modules.

so it shouldn't have existed :) but whichever way it works should be fine... the loss of indentation is true but the content remains the same.. either way it saves us a lot of time and encourages us to load more data .

We can live with this rowset being logged like that as it is not used anywhere - possibly in PSSDIAG and it may fail to import that but no big deal. Great work on this.