microsoft / vscode-mssql

Visual Studio Code SQL Server extension.
Other
1.55k stars 458 forks source link

Intellisense against Azure SQL DBs very inconsistent #998

Closed ransagy closed 3 weeks ago

ransagy commented 7 years ago

This is not really new or limited to the current version(s). Ever since i started using this, back in the 0.2.0 days, It behaved more or less the same.

(Note: Only relevant issue i found is the old #508 )

Steps to Reproduce:

  1. Open a new SQL file.
  2. Connect to an Azure SQL instance.
  3. Wait for the Intellisense cache to finish building (according to the status bar).
  4. Try and auto-complete something basic, like table names.

Most of the time, I can't get intellisense for Azure to work in any sort of reliable way. The same connection to the same DB can work one moment and not the other. Refreshing the cache sometimes helps, Sometimes it does not.

Is Intellisense in this extension dependent on something when working on an Azure SQL DB? Anything else i can try and diagnose/narrow down?

brianjorden commented 7 years ago

Ok I spent quite a bit of time trying to troubleshoot this issue before I ended up finding this issue and the other "closed-wontfix" issue referenced above. I actually only realized this extension was even supposed to have Intellisense when I was reading the latest changelog.

My primary environment is actually Linux and I connect to an Azure SQL Database instance, but I decided to test this a lot deeper. I used a Windows environment with a brand new installation of VS Code and the mssql extension with no settings/customizations to keep it as pure as I could. When that didn't help, it seemed to be specific to Azure SQL Database as eluded to above. So I decided to test that a bit further by installing a local Express version and finally saw what I REALLY hope to be able to use soon, real Intellisense similar to SSDT.

For the record, I'm not trying to complain here, just excited to see it hopefully coming soon and trying to give as much info as I can to help troubleshoot this. I actually dug through the source a fair bit, but didn't have much luck honestly. I'm inserting two screenshots that have a bit more detail on exact versions of everything and clearly demonstrate the exact same scripts/files/environment behaving very differently between the LocalDB connection and Azure SQL Database.

Connected to Azure SQL Database (it almost seems Intellisense is off?): vscode-mssql-intellisense-issues

Connected to LocalDB (everything works as expected): vscode-mssql-intellisense-working

There is one seemingly false positive scenario that I hit that I think is worth mentioning and may be related to the inconsistent testing I had and mentioned above. I originally was testing in single file for my example, but the default VS Code settings will attempt to auto-complete word based suggestions from within the same file. You can see what would have looked like a "working" version in the first screenshot where it is suggesting the word "something" because that had been used in the comments. If you have that setting turned on and define the table in the same file, it will seem like it is somewhat working.

Hope this information helps and I'll maintain my little test setup for a while if anyone wants deeper details. Even without full Intellisense, this extension is quickly replacing my need to fire up SSMS on my Windows machine just to check something and excited to see it go further, so thanks for all the work here.

ransagy commented 7 years ago

That describes the situation i was talking about very well, Thank you for sharing and the effort you put into testing :)

brianjorden commented 7 years ago

I should have edited that huge thing down a bit.

In summary, it seems Intellisense is either not working or completely disabled for Azure SQL Database connections.

kevcunnane commented 7 years ago

One thing I'd ask is what is the service tier for the database? What I've seen is that lower tiers (basic or S0), especially with a larger schema, will result in Intellisense taking too long and the query eventually timing out. This leaves you in the state mentioned.

I'd also ask you to see if SSMS 17.2 has the same issues against an Azure database. We use a very similar stack to that so it that works, then we should be able to. If not, it's the same core issue (the DB is throttled enough that Intellisense building does not work).

ransagy commented 7 years ago

SSMS didn't work for me either, but I didn't try much there because we use a few 3rd party extensions for Intellisense there, since azure support natively came only recently.

A common schema i work on has fewer than a hundred objects and i get issues there. non-prod DBs are probably in S1-S3 tiers, prod is definitely P1 and up.

I'll try and verify against that tomorrow and report back.

i do suggest notifying the user in some way that fetching Intellisense failed, perhaps with a message about tiers as well.

it is a bit odd though, to be so dependent on performance in this case. there has to be a more reliable way to implement this, i hope.

brianjorden commented 7 years ago

Well, the plot thickens...and some unexpected and very different resolution. Short version, whatever credentials you are using must ALSO have access to the master database.

When I say credentials I actually mean the specific username/password combination must also be able to connect to the master database directly. So as a workaround, you can create another encapsulated database user in the master database with the exact same username/password, or create a user for that SQL Login in the master database. Kind of a hacky workaround in my opinion, but I'm going to create a separate issue with details more specific to what I've found through some digging.

It seems the confusion around this is because that isn't exactly typical setup for most users/connections with database encapsulated users for Azure SQL Database. What totally threw me off in testing was when I shortcut things a bit and just used the super-global-admin login thing.

I'm now erasing 5 paragraphs of analysis/testing with various new databases being created including the complete failure even after ramping the database instance I care about all the way up to P2 level. I have now gotten this all to work even when I tested scaling DOWN to a Basic tier (not what I normally keep it at, just ruling out possibilities).

@ransagy could you confirm if adding an equivalent user access to your master database resolves this issue for you? At least as a temporary workaround I mean or even as a test if you don't leave that in place.

ransagy commented 7 years ago

As far as i know, the user i connect with does have access, It's a somewhat admin-level account as you said. Did you need to add it specifically to the master DB anyway? I mean by this that i can connect and query around in the master DB with that user.

I tried today to get intellisense running against our P-tier prod DB, and while i initially had it working, one intellisense invoke later - gone. Cache refresh didn't help either. I think there's more here than query timeouts - Unless the extension constantly tries to re-query for intellisense data and the moment one of those times out (and i still doubt it does against the P-tier DB) you lose Intellisense.

brianjorden commented 7 years ago

As for the performance thing, that wasn't the issue in my case at all. I tried ramping the development database I was using up to P2, still didn't work. Then created a user specifically in master for my SQL Login that I'm typically using and it all started working. To prove it out further, I scaled it all the way down to lower than normal, as the Basic tier and it still worked fine consistently. I then tried another developer account created with identical security/authority/permissions where I hadn't done the user in master yet, and it was not working. Closed/reopened VS Code a few times, refreshing cache, etc trying to get any different result, but seemed finally solid where the one user worked, and the other didn't. Added a user for that other account, closed, and reopened VS Code (for some reason this seemed necessary in testing). The additional user was then working perfectly as well.

The timeout thing doesn't quite line up for what I'm seeing. Updating IntelliSense seems to only last a few seconds (under 10) when it is going to work, but pretty consistently between 40-50 seconds when it isn't working. I did notice even when it is all working it seems to refetch the IntelliSense stuff for basically every new tab. Not a problem when everything is working, but makes testing a bit rough when it is taking way longer and/or not consistent.

@ransagy can you connect to master and try this select * from master.sys.database_principals; to confirm the user/login you are expecting shows up there? If you are actually connected directly to master it won't yell about the three-part name thing.

ransagy commented 7 years ago

The user im using does appear there:

name principal_id type type_desc default_schema_name create_date modify_date owning_principal_id sid is_fixed_role authentication_type authentication_type_desc default_language_name default_language_lcid allow_encrypted_value_modifications
USERHERE 5 S SQL_USER dbo 2014-10-31 09:20:33.427 2014-10-31 09:20:33.457 NULL 0x010600000000016480000000003B2F7C7CE8FD89A267414C99AAB0AE82CDF9EF 0 1 INSTANCE NULL NULL 0

What exact actions did you do to add that additional user? I'll try those here too and see.

brianjorden commented 7 years ago

Ok I also just tested and got the exact same behavior with SSMS 17.2 so I'm pretty confident this is just an underlying oddity on how the IntelliSense info is being fetched. The issue is that although on SQL Server the master database is just a 3-part name resolution away, but with Azure SQL Database it is a more explicit permission/access level.

To recreate this whole thing you can do the following:

-- connect to the master database with a sysadmin type account
use master;
create login test_login with password = 'some-really-good-password-here';

Then...

-- connect to your actual database instance
use [whatever-your-db-name-is];
create user test_db_user for login test_login;
alter role db_owner add member test_db_user;

At this point, you should be able to connect to your database with the new test_login credentials, but I would NOT expect IntelliSense to be working. Once you confirm it isn't working, you can then...

-- connect to the master database
use master;
create user test_master_user for login test_login;

FInally, if you disconnect/reconnect to your database with the test_login credentials, I would expect IntelliSense to start working. Although in SSMS 17.2 it did take 20-30 seconds before it seemed to really kick in after trying a query or two.

I put in the use statements just to help ensure the connection really is to the database you expect. Azure will just throw an error if you attempt a use statement for any database besides the one you are currently connected.

brianjorden commented 7 years ago

Ok, I've now been trying to find info on Azure SQL Database and SSMS 17.2 and seems a decent number of people are having issues with no one really providing answers/resolutions yet. @kevcunnane when you said the VS Code extension is using something similar for IntelliSense, did you mean re-use of the code base? More so, if it is an issue/oddity with how SSMS 17.2 and this extension works, where would the best place to log details actually be?

kevcunnane commented 7 years ago

@brianjorden yes, we use the same Intellisense Parser and the SMO MetadataProvider for connected intellisense that SSMS uses. Our actual implementation that hooks this up (first populating the provider, then querying) is different but it's the same core code.

I think this is the best place to log this. I'm sending a mail to the area experts including @shueybubbles and @Matteo-T to ensure this is on their radar. The information you've all added is very helpful, so I'll look to see if we can fix / work around this issue with the user account needing to be in master for this to work.

brianjorden commented 7 years ago

Appreciate the updates/responses here. I've been in the consulting world and entirely closed source ecosystem for many years and this was an item I was hoping to be able to really dig into if I could. I do have a bit more information to share that I've collected from MSDN/TechNet/StackOverflow/several other different Microsoft and additional sources. Is there any chat type place (Slack/Gitter/Discord/Twitter DMs/etc) I could share some additional info? Asking about chat just cause I have a handful of questions as well and likely easier with some back-and-forth.

ransagy commented 7 years ago

@brianjorden no go for me with VSCode/the current extension. I followed along your lines but i still get no intellisense and the "updating intellisense" line takes a while to disappear, as you suggested it does when its not working.

brianjorden commented 7 years ago

@ransagy that is really unfortunate, I thought I was seeing a big part of what was going on as it seems to be working reasonably well on my end at this point. If you haven't tried SSMS 17.2 as @kevcunnane suggested before, you may want to give that a shot at least. I am often a few updates behind on SSMS and hadn't tried the latest version yet, lots of improvements in general, and the Intellisense is at least working as well as to be expected there now too.

A couple notes on Intellisense in both applications though, it is still throwing a fair number of false-positive errors for official syntax of some of the slightly newer features. Just to be sure, I went and copy/pasted a handful of examples from the docs and although you can still execute the queries, lots of errors including everything below some of them gets pretty botched. The definition of system versioned temporal tables is the easiest example.

In comparison, SSDT is still holding very strong, only a few false-errors there...but when those get thrown you can't even always compile the project itself. Moving to using SSDT as the primary tool for developing/maintaining the real schema has been a massive improvement, but it feels like Azure SQL Database features and functionality aren't as well supported there, unfortunately. Funny enough, some of the things not working in SSDT actually work fine in SSMS/VS Code (stuff like STRING_SPLIT or a stored proc that uses a CTE table).

kevcunnane commented 7 years ago

A quick update: @shueybubbles has confirmed this is an issue in both SSMS and the MSSQL extension. He's looking at a fix but doesn't have a firm date yet as fixing the immediate issue highlighted some more deeper in the stack. The root cause is the same - Azure + no user in master is blocking intellisense.

@brianjorden if you want a more interactive discussion you can go to https://gitter.im/Microsoft/mssql. I get notifications from there as does most of the team. We often have delayed responses since we only check when we get pinged. Also for the issues you raised, we'd definitely recommend opening Connect bugs (or issues here if they affect the MSSQL extension). We do try to track these and Intellisense etc. are things we can push on partners to do if they own the feature area.

ransagy commented 7 years ago

I've noticed SSMS 17.3 was released with a few release note items about Azure intellisense, But i didn't see any change or improvement on my end, If it's relevant enough. I doubt that release contains the mentioned bug fixes with such a short time in between them, either way.

brianjorden commented 7 years ago

Forgot to ever come back and leave some comments here. It seems SSMS 17.3 actually resolved the vast majority of Intellisense issues I was still seeing using that tool. Obviously, I'm sure that updated code hasn't made it to this extension yet. Also, it seems all of my current issues with SSDT was also resolved in the 17.3 release of it along with the DACFX updates. Great to see the progress, but at this point, it seems the issues are pretty well understood. Hopefully @ransagy has his issues resolved with the future releases too...

kevcunnane commented 7 years ago

The fixes for this specific issue (requiring a use in master for intellisense to work) missed the 17.3 release but will be included in a future release. It'll also be in the next release of this extension. I'm going to pre-emptively close this as it'll be in our next release. Thanks all for your help tracking this down.

ransagy commented 7 years ago

is it committed in the extension/sqltoolsservice? if so, ill compile and test. looking forward to see if it helps in my case, despite it all :)

brianjorden commented 7 years ago

@ransagy if you have a chance, had one more suggestion you may want to try (leading to a possible temp workaround for you)...that would be to use SSMS 17.3 and connect directly to the master database itself. Then to confirm the connection, just select * from information_schema.tables and should get something like the results in this screenshot. You can also see in the screenshot the functioning Intellisense for column completion. Really curious if that ends up working for you. Can DM me on Twitter @brian_jorden if easier than here for back/forth.

image

kevcunnane commented 7 years ago

@ransagy we haven't re-synced yet. We'll likely be doing an integration in the next few weeks.

ransagy commented 7 years ago

Just as a little sum-up of the back-and-forth I've had with @brianjorden - He had the good idea to set up a test instance for me to connect to to rule out individual server issues; He then discovered that a user is needed for the login in EVERY database on the instance for Intellisense to work reliably.

This caused my secondary, ~20 databases instance at work to gain Intellisense reliably in both VSCode-MSSQL and SSMS. My primary, ~70 databases instance still didn't work, suggesting there is a performance issue with how large the server/how many databases there are.

I'm currently splitting non-essential databases from our primary instance to another instance to see if this affects the performance of Intellisense.

shueybubbles commented 7 years ago

I think the underlying SMO metadata provider enumerates objects in every database so it can support 3 part names and the like; it’s a legacy of being written to support the box product primarily.

I am curious whether the behavior improves if you set the Initial Catalog to a specific database on your server instead of connecting to master. IIRC the provider will use sys.databases in the user database in that case, but I may be mistaken.

Sent from Mailhttps://go.microsoft.com/fwlink/?LinkId=550986 for Windows 10


From: Ran Sagy notifications@github.com Sent: Monday, November 6, 2017 9:15:14 AM To: Microsoft/vscode-mssql Cc: David Shiflet; Mention Subject: Re: [Microsoft/vscode-mssql] Intellisense against Azure SQL DBs very inconsistent (#998)

Just as a little sum-up of the back-and-forth I've had with @brianjordenhttps://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fbrianjorden&data=02%7C01%7Cshueybubbles%40hotmail.com%7C77e8eaf3ac374997a86508d52520cdc3%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C636455745171039376&sdata=93jszV6uIjbbrmyhTKWQ7S%2FdLPnkJr6WDlPGd0V4nZM%3D&reserved=0 - He had the good idea to set up a test instance for me to connect to to rule out individual server issues; He then discovered that a user is needed for the login in EVERY database on the instance for Intellisense to work reliably.

This caused my secondary, ~20 databases instance at work to gain Intellisense reliably in both VSCode-MSSQL and SSMS. My primary, ~70 databases instance still didn't work, suggesting there is a performance issue with how large the server/how many databases there are.

I'm currently splitting non-essential databases from our primary instance to another instance to see if this affects the performance of Intellisense.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2FMicrosoft%2Fvscode-mssql%2Fissues%2F998%23issuecomment-342160635&data=02%7C01%7Cshueybubbles%40hotmail.com%7C77e8eaf3ac374997a86508d52520cdc3%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C636455745171039376&sdata=bpdYOLg%2F2l6xuR%2FtJJ2iCAOeE7CH9tDmbpmAbC26TpU%3D&reserved=0, or mute the threadhttps://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FACHzCuAkfmGdfsOqGK6C1UniI0mN8Dwpks5szxRygaJpZM4Pio61&data=02%7C01%7Cshueybubbles%40hotmail.com%7C77e8eaf3ac374997a86508d52520cdc3%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C636455745171039376&sdata=Vp93pKH5YY3tyPPHmrYUuuwNBWpRVdcqBl9R7hguSJo%3D&reserved=0.

ransagy commented 7 years ago

As far as i remember from what we tested that didn't seem to matter. @brianjorden, Correct me if I'm wrong here.

brianjorden commented 7 years ago

@shueybubbles I can say with confidence that didn't seem to matter simply based on how I was setting things up trying to help @ransagy test the remaining issues. Came to the same conclusion/assumption on the 3 part names likely being a culprit after I realized the Intellisense was showing me the objects from other databases within the same Azure instance even though there was no way to actually use/reference them. When we were swapping messages about testing this, I created a new temp database within an instance I had been using successfully before. The previously working accounts stopped working, and the test account (login with contained users in master/new database) that I created for @ransagy was also not working. On a bit of a hunch/desperation to understand, I also added that test account to my regular development database (only other on that instance) and it started working. Some more testing and the previously working then non-working accounts resumed working after they also got a new contained user in that new temp database. Fortunately, I think this one is pretty easy/straight forward to replicate.

Steps to replicate/prove out:

  1. Create a new Azure SQL Server instance
  2. Create two Azure SQL Databases (testdb1 & testdb2)
  3. Connect to master with the SA account
  4. Create two logins with passwords for testing:
    • create login account1 with password = 'SOME_PASSWORD'
    • create login account2 with password = 'SOME_PASSWORD'
  5. Create a contained user for both accounts in master:
    • create user account1 for login account1
    • create user account2 for login account2
  6. Create a contained user for both accounts in testdb1:
    • create user account1 for login account1
    • create user account2 for login account2
  7. Create a contained user for only one account in testdb2:
    • create user account1 for login account1
  8. When connecting as account1 everything should work as expected
  9. When connecting as account2 it won't work even in testdb1

Twitter messages where we swapped info on some of the testing: https://twitter.com/RanSagy/status/922016688777908224

ransagy commented 7 years ago

I can confirm splitting off the databases to other instances improves on Intellisense availability. Also, the last tidbit of information i can add is that this workaround of creating users for the login won't work for the designated "server admin" login that is created when making a new database from Azure's portal, as you can't create users for that login on the various databases ("The logon already has a user under a different name").

brianjorden commented 7 years ago

@ransagy Your last comment actually surprises me or makes me a little bit less sure of my understanding here. I can't remember how well I tested the server admin login account, but I thought I found that it simply worked without any other effort. My thinking was that because it naturally had permission to access any of the databases it didn't actually need the localized contained users.

ransagy commented 7 years ago

The user i was testing with all along (aside from the ones created following our discussion of new test login and users) was the one created when the server and databases were made through the Azure portal, hence the "server admin" account shown in the portal. Intellisense never worked for it, Only for the new login when i created a new user for it in each DB as discussed.

brianjorden commented 7 years ago

huh, well I know I didn't test very thoroughly with that account, but that does make me question exactly what is going on under the hood a bit. Hopefully the info here is at least enough for the right team to figure out exactly what is going on a bit easier. I actually took some time to look through this vscode-mssql extension and the sqltoolsservice codebases and honestly didn't have much luck.

kevcunnane commented 7 years ago

Folks, we'll have an updated release going out soon to support multi-root workspaces and the SMO fix should be included in that. All the relevant code is referenced as DLLs, not in the tools service as it's the same intellisense + SMO code that ships in SSMS. I will update once we have a release you can test against, and see if the issue is fully resolved.

ransagy commented 7 years ago

If the release in question is the 1.2.1 RC; It seems to make very little difference on this issue.

While i am able to get intellisense on my smaller instance now connecting with the server admin account, I also get it in SSMS 17.3, suggesting its more due to performance/size rather than the updated SMO bits (unless 17.3 includes those; i understood it does not). Even in that case, It takes significantly longer than when a user/login was made.

On the larger instance, I still don't get any intellisense with the server admin account, Not even in master. The 'updating Intellisense..' marker disappears after a minute or so, but without result, similar to what @brianjorden suggested at the beginning.

Let me know if i can provide more information or test differently.

EDIT: I noticed even after uninstalling and reinstalling the extension, I didn't get any prompt that the SQLToolsService is being downloaded, like i usually did. Is that a change in behavior in the new version?

kevcunnane commented 7 years ago

@ransagy sorry to hear this doesn't resolve the problem for you. You're correct in that for large size schemas in a lower pricing tier, we may not be able to build the intellisense cache within our timeout. This is unfortunately a factor of intellisense doing a few large queries that don't scale well to the lower pricing tiers and may be the issue in your case

@brianjorden if you can test out the new release (it should be out today), it would be great to see whether it resolves the problem for you. The fix to handle users not being able to log into other DBs should be in the reference Intellisense/SMO bits.

ransagy commented 7 years ago

@kevcunnane Most DBs in the main instance are S0-S3, some P1 and above. Is there a plan to improve this reliance or configure the time out? An official guideline/pre-requirement defined for Azure SQL Intellisense to be 'supported'?

It still seems very odd that various 3rd party tools manage this feature without the requirement for expensive pricing tiers and specific architectures (i.e. splitting DBs between instances).

I'd love to discuss and help with testing if needed to reach a better, more reliable solution.

kevcunnane commented 7 years ago

@shueybubbles is a good contact on this one. The nice thing is that improvements in this area will benefit both this extension, SSMS, and SQL Operations Studio since they share the same common code path. There have been improvements in general to support Azure better and we're continuing to drive these forward.

The best ways to proceed are if you have good reference schemas or scenarios that are public enough to share with the team, but which reliably. Create a Connect bug and attach the schema and contact @shueybubbles - as we go through additional waves of Azure reliability improvements we can see if this helps identify bottlenecks and areas where we can resolve the blocking issues.

shueybubbles commented 7 years ago

Thanx Kevin for the vote of confidence! Intellisense in SMO/SSMS has a long history, rooted in development for SQL Server’s box product. Initial development was done in an era when SSMS was running either on the same computer as the SQL engine or no further away than the same subnet. If a customer needed dozens or hundreds of databases, they would have spread them over multiple servers instead of on one machine.

Azure SQL Database presents Intellisense with a vastly different set of values for operational parameters like query latency and ratio of user databases per master. When we first enabled Intellisense for Azure SQL database in SSMS this year, we did so without knowing how much these variables would impact the performance and functionality in the “real world”. Feedback such as yours is critical for us to identify bottlenecks and areas to focus our future development.

The first fix we’ve made has been to handle the case of the user not having read access to the master database. That fix will be included in the build Kevin mentioned. We will look at the feedback from this thread and identify future fixes to prioritize.

Thanx again!

Sent from Mailhttps://go.microsoft.com/fwlink/?LinkId=550986 for Windows 10


From: Kevin Cunnane notifications@github.com Sent: Wednesday, November 8, 2017 1:14:26 PM To: Microsoft/vscode-mssql Cc: David Shiflet; Mention Subject: Re: [Microsoft/vscode-mssql] Intellisense against Azure SQL DBs very inconsistent (#998)

@shueybubbleshttps://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fshueybubbles&data=02%7C01%7Cshueybubbles%40hotmail.com%7C63162c3e75a640374ee908d526d48d06%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C636457616692790137&sdata=86GWJOZkpr8lA2AhHekuUP54pxYiJypziw2AfWn%2BnEA%3D&reserved=0 is a good contact on this one. The nice thing is that improvements in this area will benefit both this extension, SSMS, and SQL Operations Studio since they share the same common code path. There have been improvements in general to support Azure better and we're continuing to drive these forward.

The best ways to proceed are if you have good reference schemas or scenarios that are public enough to share with the team, but which reliably. Create a Connect bug and attach the schema and contact @shueybubbleshttps://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fshueybubbles&data=02%7C01%7Cshueybubbles%40hotmail.com%7C63162c3e75a640374ee908d526d48d06%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C636457616692790137&sdata=86GWJOZkpr8lA2AhHekuUP54pxYiJypziw2AfWn%2BnEA%3D&reserved=0 - as we go through additional waves of Azure reliability improvements we can see if this helps identify bottlenecks and areas where we can resolve the blocking issues.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2FMicrosoft%2Fvscode-mssql%2Fissues%2F998%23issuecomment-342906200&data=02%7C01%7Cshueybubbles%40hotmail.com%7C63162c3e75a640374ee908d526d48d06%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C636457616692790137&sdata=bFCAh8QeGcI7yCElyV%2BIDrBgoyFMuZaDFAtYTnACgtY%3D&reserved=0, or mute the threadhttps://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FACHzCqLpG3x99jlQi4OHzEyZFIMQNGptks5s0e-CgaJpZM4Pio61&data=02%7C01%7Cshueybubbles%40hotmail.com%7C63162c3e75a640374ee908d526d48d06%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C636457616692790137&sdata=qgAKCV8xuAEVBv6Z0sfiKL%2Fyf%2F2VGSmJtYixyGN0tPQ%3D&reserved=0.

brianjorden commented 7 years ago

@kevcunnane and @shueybubbles I spent some time today trying to test this a bit with some mixed results that had enough nuance that I can't say as much with absolute confidence here. Hopefully some of these notes will be helpful in trying to track down exactly what is going on. I'm going to try to condense what I am pretty confident is happening as of now, I updated to latest VSCode and vscode-mssql earlier today. For brevity/clarity when I mention login, I mean a login created in master

Definitions (for brevity/clarity)

Scenarios

  1. Working - Contained user in a single database
  2. Working - Login with a user in every database including master
  3. Working - Login with a user in a single database
  4. Working - Login with a user in both user databases, but not master
  5. Not Working - Login with a user in master and only one user database
  6. Not Working - Stored connection profile that was tested in number 2 then attempted to be re-tested as number 3
  7. Not Working - MS SQL: Refresh IntelliSense Cache - produces no visual update/results
  8. Not Working - Testing a not working scenario, updating security to a working scenario, then retesting without closing and reopening VS Code (this includes attempting to open a new tab/connection)

One of the odd things I ran into had to do with number 6. I had a login with a user in every database including master. To save some keystrokes I had created a connection profile to the new temp/blank database I created earlier today for that account. It tested successfully. I then dropped each user and the login. Then recreated the same login with a user only in one of the user databases (the one I saved the connection profile for). I restarted VS Code, and attempted to connect using the saved profile. This it wasn't just Intellisense that wasn't working, I was getting connection errors and unable to even run a select 1. A significant note is that I actually got two connection errors including both user database names. I'm guessing because something about the connection to that other database had been cached for that connection profile that was no longer valid. When I recreated the connection profile in a new tab (without even restarting VS Code) using the same (copy/pasted) credentials it worked, including Intellisense.

That does bring up another point/question on the viability of actually using VS Code for much besides a laziness of going over to SSMS. Is there any intention/plan of caching some of this Intellisense information locally or at least persisting it across multiple tabs for the same connection? What I'm finding is that I have a 20 second or so delay in updating Intellisense even when everything is working perfectly when I just open a new tab or file. I had hoped to leverage this and/or the new SQL Operations Studio in some of the same ways I've been using SSDT. I understand the idea of different tools targeting different user types, but I'm guessing a similar type of functionality will also be the foundation for SQL Operations Studio.

Using SSDT as a comparison on Intellisense is also a bit unfair due to the ability to leverage a local dacpac definition. Although as I type that, couldn't that potentially be a longer term answer to Intellisense type stuff that would include cross tab caching and even offline development? I know the tools are out there for extracting a dacpac definition of a database, but I'm not sure what the performance of that might look like. Also unsure if any of the SSDT style Intellisense based on the local dacpac could be leveraged. Just seems it could solve a few things (and introduce some new challenges of course) such as being able to snapshot and reference 1k databases because they are sitting as local files.

ransagy commented 5 years ago

I know i'm poking the bear a bit here, But anyone saw any progress on this? I still, to this day and latest versions of VSCode and MSSQL, cannot get any reliable intellisense against almost all of my Azure SQL instances. I am wondering if I'm the only one at this point..

brianjorden commented 5 years ago

@ransagy it is fairly late for me here but saw the email about this comment. I think there are options, can you DM me on Twitter sometime? Fairly late for me here right now, but didn't want to forget to reply. (I'm US Eastern timezone)

ransagy commented 5 years ago

@brianjorden will do.

In another direction on this - Would be it be any good to debug vscode/vscode-mssql to drill down into this? Or will i just see a black-box SMO call somewhere down the stack?

shueybubbles commented 5 years ago

I don't think trying to debug further into the tools service would provide much illumination. There are a few "known" issues that require a variety of solutions to address.

  1. The basic dependency on fetching schema from the database every time a new connection is established can lead to a pretty poor experience, as has been mentioned above. A better local cache/dacpac/differencing mechanism would go a long way to improving the experience across the board, for all connection types.

  2. The current implementation was built in the days when SSMS was often being used on the same subnet or even same computer as SQL Server, and it was designed for the SQL Server "box" model. Connection latencies really didn't come into play during R&D so engineers didn't bother accounting for internet latencies. If your SQL instance was slow to respond to a query, you were advised to buy a faster CPU or not to use SSMS against your production server. There's probably a fair amount of optimization that could be performed within the current architecture to further minimize the number of server round trips and the load we put on the server.

  3. Another side effect of being rooted in SQL Server "box" architecture is that there are a bunch of implicit assumptions in the code about the user having access to master and at least public access to other databases on the same server. For example, in box you could use three part names like "dbname.schemaname.objectname" so intellisense wants to know the names of objects in every database on the server. The db_owner role etc didn't exist before Azure. Furthermore, when the first version of Azure SQL Database was created, there were a bunch of catalog views that still only existed in the logical master. AFAIK in current Azure SQL DB/DW all the views we need are now available in the user database. We need to spend time working in the SMO layers to remove all the attempts to query master explicitly (in Azure) and to stop attempting to query data about databases other than the one specified in the connection string. Intellisense shouldn't try to handle three part names in Azure.

For my part, I will be actively working on numbers 2 and 3 in 2019, because fixes in SMO will directly impact a broad range of SQL system components, tools like SSMS and ADS, Data Sync, and customer-built applications. Moving to a completely new architecture for schema discovery/caching etc is currently beyond my scope, but I will be working with leaders like Kevin to try to get more engineering resources allocated to get focus on it in future sprints. I see no reason for ADS and SSMS to continue to be coupled to the same archaic intellisense provider if we can innovate in the ADS space and get more community contributions.

ransagy commented 5 years ago

Thank you for the detailed explanation! I was aware of some of it, but not all, so good to know.

It's also very good that this is something MS are aware of (or will be made aware) on a larger scale, i think that indication was missing across the various reported issues here, on UserVoice etc.

Much appreciated!

ransagy commented 5 years ago

Is there anything new to share here regarding the mentioned work?

shueybubbles commented 5 years ago

I believe we have made good progress on #3 from my list - users without access to other databases should see much better results from intellisense than before, in both recent ADS releases and SSMS 18.2. I've gotten direct feedback from customers to that effect. One noticeable side effect is their auditing shows no more failed logins on their idle user databases from SSMS's intellisense.

Benjin commented 3 weeks ago

Given that there hasn't been any new activity on this item since 2019, I'm guessing that the major improvements in the underlying libraries that have occurred in the past several years have largely resolved the issue, and I'm going to close it. If you're still seeing issues, please create a new issue with details specific to what you're encountering, but feel free to link back to this one for context