search

Snowflake-Labs / sqltools-snowflake-driver

A Snowflake driver for the SQLTools VSCode extension.
MIT License
35 stars 15 forks source link

Feat/additional schema level objects #59

Closed adamNewell closed 1 year ago

koszti commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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

koszti commented 1 year ago

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 commented 1 year ago

That would be pretty cool.

I'm currently working on intellisense and adding some DDL generation to the context menus of items in the nav tree (as well as dynamic refresh of the tree. That's a little more complex and I want to make sure it's tested thoroughly, so it may take me a bit to finish up.

Intellisense isn't all that difficult, as it turns out. There are plenty of references I'll riff off from other extensions that eliminate most of the hard parts.

Many of the features I plan to add will bring parity between the snowflake driver in vscode and intellij/datagrip. It's been fun to jump in here and contribute, so thanks for the feedback!

Cheers mate

On Wed, Nov 9, 2022 at 11:15 AM Peter Kosztolanyi @.***> wrote:

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. I contacted to the vscode marketplace support to look into it.

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

— Reply to this email directly, view it on GitHub https://github.com/Snowflake-Labs/sqltools-snowflake-driver/pull/59#issuecomment-1309081179, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAKUAEAJA77UMUPJRVON35DWHPL4DANCNFSM6AAAAAARZYXLJA . You are receiving this because you authored the thread.Message ID: @.***>

koszti commented 1 year ago

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 commented 1 year ago

I took some time to clean up the solution to use fewer custom constructs in a new PR. If you have a chance to take a look at that, I think it's a good simplification.

On Wed, Nov 16, 2022 at 10:03 AM Peter Kosztolanyi @.***> wrote:

hey @adamNewell https://github.com/adamNewell . VS marketplace support finally fixed the 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 🙇

— Reply to this email directly, view it on GitHub https://github.com/Snowflake-Labs/sqltools-snowflake-driver/pull/59#issuecomment-1317357476, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAKUAEDWU7E4XAEIASKQ5KTWIUHWLANCNFSM6AAAAAARZYXLJA . You are receiving this because you were mentioned.Message ID: @.***>