Skip to content
This repository has been archived by the owner on Mar 15, 2021. It is now read-only.

Database metadata #10

Merged
merged 7 commits into from
Mar 25, 2016
Merged

Database metadata #10

merged 7 commits into from
Mar 25, 2016

Conversation

BornaP
Copy link
Member

@BornaP BornaP commented Mar 24, 2016

Pull request related to issue #98 on sqlectron-gui repository. As a part of solving that issue, I've implemented functions for listing views and routines which can now be used in sqelctron-gui. Since PostgreSQL only has functions as routines type, while MySQL and Microsoft SQL Server have both procedures and functions as a routines, listRoutines is returning { routineName, routineType } objects, so that procedures and functions can be distributed in separate collections in GUI, if needed.

Furthermore, I added one view and routine in schema.sql for both DBs and wrote and ran specs in db.spec.js .
All tests are passing expect two regarding Microsoft SQL Server, because I don't have it installed on same machine. I'll probably make same changes for SQL Server when I'll be able to test it.
sqlectron_tests

This is it for a start, maybe latter we could fetch triggers related to specific table?

Cheers,
Borna

@BornaP
Copy link
Member Author

BornaP commented Mar 24, 2016

I'll add support for Microsoft SQL Server tomorrow. I forgot to put if statements in db.spec.js for SQL Server in those two tests.

@@ -107,6 +109,15 @@ async function listTables(server, database) {
return database.connection.listTables();
}

async function listViews(server, database) {
Copy link
Member

Choose a reason for hiding this comment

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

as does not have usage of await inside block it does not need to have async

@BornaP
Copy link
Member Author

BornaP commented Mar 25, 2016

Support for SQL Server is now up and tests are passing. Sorry for few extra builds, I had some troubles debugging as I don't use SQL Server 2008 regularly.

Please check client.js, I saw async used in several more places (ex. listTables) where await is not used inside of a block. So I can remove them all at once.

Next, I think it's best to enable viewing routines and views in GUI, and later add triggers and columns lists under each table separately.

Cheers,
Borna

ORDER BY routine_name
`;
const params = [];
client.query(sql, params, (err, data) => {
Copy link
Member

Choose a reason for hiding this comment

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

as params is not used anywhere else, perhaps you can pass directly one []...

 client.query(sql, [], (err, data) => {

@maxcnunes
Copy link
Member

@BornaP thanks for the pull request. I guess is everything right. I agree breaking the load of triggers and column names in a different pull request. Will be easier for you to implement and for us to review.

@krolow I will accept the pull request with those params variables because I think is more readable.

@maxcnunes maxcnunes merged commit ab8c3e2 into sqlectron:master Mar 25, 2016
@BornaP
Copy link
Member Author

BornaP commented Mar 26, 2016

Just what I wanted to say for *params * variables. Glad to see pull request accepted!

@BornaP BornaP deleted the database-metadata branch March 29, 2016 07:39
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants