Add command to run queries on multiple databases #898
Conversation
shati-patel
changed the title
shati-patel
force-pushed
the
multi-databases
branch
from
6b32a3e
to
541627e
Compare
|
This looks very neat, and is a nice compact PR! Just had a quick read, will actually test it out later and then do a proper review. |
| commandRunnerWithProgress( | ||
| 'codeQL.runQueryOnMultipleDatabases', | ||
| async ( | ||
| progress: ProgressCallback, |
There was a problem hiding this comment.
Future note: A cool enhancement might be to have the progress bar reflect how many of the selected databases the query has completed on. Dealing with progress monitors is tricky, so let's think about this later, writing it here just to record the idea.
| if (quickpick !== undefined) { | ||
| for (const item of quickpick) { | ||
| try { | ||
| await compileAndRunQuery(false, uri, progress, token, item.databaseItem); |
There was a problem hiding this comment.
We don't know the language of the query at this point do we? It would be very cool to automatically filter out databases that are of the wrong language. But no problem if we can't do that yet.
There was a problem hiding this comment.
We don't know the language of the query at this point do we? It would be very cool to automatically filter out databases that are of the wrong language. But no problem if we can't do that yet.
Indeed 😅 I couldn't work out how to extract that language information before displaying the quick pick, so I stuck with this for now!
shati-patel
force-pushed
the
multi-databases
branch
from
541627e
to
6c55f19
Compare
| ): Promise<void> { | ||
| if (qs !== undefined) { | ||
| const dbItem = await databaseUI.getDatabaseItem(progress, token); | ||
| // If no databaseItem is specified, use the database currently selected in the Databases UI | ||
| const dbItem = databaseItem || await databaseUI.getDatabaseItem(progress, token); |
There was a problem hiding this comment.
Minor: you could use only one variable throughout:
databaseItem = databaseItem || await databaseUI.getDatabaseItem(progress, token);and replace dbItem in the two uses below with databaseItem.
Why do this? To avoid accidentally introducing code later that uses databaseItem when we should be using dbItem. (This is a classic mistake I have made often when two variables have similar names but only one variable has the fully initialised value...)
There was a problem hiding this comment.
Changed! Thanks for the tip—I can definitely see myself getting confused by variables otherwise 🙃
Will fix #569.
First pass at adding a new command "CodeQL: Run Query on Multiple Databases", that lets you select multiple databases from a quick pick menu and run a query over all of them. This currently loops over all selected databases and displays results and query history runs for each database separately. If a database is incompatible, we display an error message and skip to the next database.
This might need some design input if we want to do anything fancy with results display 😅
Expand for a (somewhat grainy) demo
Notes
Checklist
@github/docs-content-codeqlhas been cc'd in all issues for UI or other user-facing changes made by this pull request.