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

Feat/additional schema level objects #59

Merged

Conversation

adamNewell
Copy link
Contributor

No description provided.

@koszti koszti mentioned this pull request Nov 8, 2022
@koszti
Copy link
Collaborator

koszti commented Nov 8, 2022

Really nice one, thanks for working on this. Can you please resolve the conflicts and I think it should be good.

Btw, iirc some time ago these features were not implemented because selecting from INFORMATION_SCHEMA was too slow at the time and was causing queuing queries on busy warehouses with lot of concurrent users. EoD it resulted a poor user experience.

The idea at that time was that instead of selecting from INFORMATION_SCHEMA we can try to use the SHOW command which was much quicker at that time (a year ago or so) but the results of the SHOW command can be collected only by running a second RESULT_SCAN query. SQLTools was not supporting this mechanism and fetching the list of missing objects was never implemented.

Anyway, as of today it's maybe just fine to use the INFORMATION_SCHEMA. Did you experience any performance issues when working on this PR? Like, long running information_schema queries or queuing queries that's blocking other users to run other SQLs?

Am I right that the new queries running only when the users clicking a certain nodes in the db-objects-tree and we're not fetching all objects all in one go? Can you please confirm if my assumptions are correct?

@koszti
Copy link
Collaborator

koszti commented Nov 9, 2022

sorry my bad, I’ve just realised that you’re using the SHOW command everywhere 🙈 . Were you able to manage processing the results without running RESULT_SCAN in another query?

@adamNewell
Copy link
Contributor Author

These don't require the result scan, no. The big thing was not forcing the coupling of the query with the object construction. The way I've done it allows the query to be fully run and then the final objects to be returned from the get functions in driver.ts.

The show functions should be pretty quick, even for large schemas and result sets. Using the show commands is definitely the way to go. In my testing, they were relatively close in terms of speed, but it's significantly more concise and clear to use show.

@adamNewell
Copy link
Contributor Author

Changes built and validated after merge of #58. Required an update to the way I was handling unnamed props on db objects.

@koszti koszti merged commit 0882c6a into Snowflake-Labs:master Nov 9, 2022
@koszti
Copy link
Collaborator

koszti commented Nov 9, 2022

hey, thanks for the contribution. I was playing with it and it's just working fine. Very nice PR.

Sadly I'm having trouble publishing the latest version to vscode marketplace. I was trying the GH action, and later manually but I'm not allowed to access the vscode extension publisher page in the marketplace, which is very weird. Using access tokens and manual upload both giving same permission problems. I contacted to the vscode marketplace support to look into it.

I hope they will get back to me and we can make it public soon.

@adamNewell
Copy link
Contributor Author

adamNewell commented Nov 10, 2022 via email

@koszti
Copy link
Collaborator

koszti commented Nov 16, 2022

hey @adamNewell . VS marketplace support fixed my publisher account and v0.5.0 has finally been published to the marketplace and to open-vsx. You should see the new version in your vscode at the next startup.

Thanks for your contribution 🙇

@adamNewell
Copy link
Contributor Author

adamNewell commented Nov 16, 2022 via email

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