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

Enable IsLedger only for SQL 2022 #1804

Merged
merged 3 commits into from
Jan 10, 2023
Merged

Enable IsLedger only for SQL 2022 #1804

merged 3 commits into from
Jan 10, 2023

Conversation

cheenamalhotra
Copy link
Member

@cheenamalhotra cheenamalhotra commented Jan 9, 2023

It turns out SMO queries fail for non-supported properties that do not apply for SQL Server versions we are connecting to, as in case of IsLedger for Database list, we are getting below error for SQL Server 19 and below:

sqltools Error: 0 : Failed getting smo objects. parent:<server>/Databases querier: Microsoft.SqlTools.ServiceLayer.ObjectExplorer.SmoModel.SqlDatabaseQuerier error:unknown property IsLedger
 inner: stacktrace:   at Microsoft.SqlServer.Management.Sdk.Sfc.EnumObject.GetProperty(String name, ObjectPropertyUsages usage)
   at Microsoft.SqlServer.Management.Smo.SqlObjectBase.AddRequestProperties()
   at Microsoft.SqlServer.Management.Smo.SqlObjectBase.RetrieveParentRequestLinks(SqlRequest sr)
   at Microsoft.SqlServer.Management.Smo.SqlObjectBase.RetrieveParentRequest()
   at Microsoft.SqlServer.Management.Smo.DatabaseLevel.RetrieveParentRequest()
   at Microsoft.SqlServer.Management.Sdk.Sfc.Environment.InitObjects(Request req)
   at Microsoft.SqlServer.Management.Sdk.Sfc.Environment.GetData(Request req, Object ci)
   at Microsoft.SqlServer.Management.Sdk.Sfc.Enumerator.GetData(Object connectionInfo, Request request)
   at Microsoft.SqlServer.Management.Smo.ExecutionManager.GetEnumeratorDataReader(Request req)
   at Microsoft.SqlServer.Management.Smo.SqlSmoObject.DoDatabaseSpecialCase(Request levelQuery, Urn levelFilter, Boolean forScripting, List`1 urnList, Int32 startLeafIdx, IList`1 defaultFields)
   at Microsoft.SqlServer.Management.Smo.SqlSmoObject.InitChildLevel(Urn levelFilter, ScriptingPreferences sp, Boolean forScripting, IEnumerable`1 extraFields)
   at Microsoft.SqlServer.Management.Smo.SmoCollectionBase.InitializeChildCollection(Boolean refresh, ScriptingPreferences sp, String filterQuery, IEnumerable`1 extraFields)
   at Microsoft.SqlServer.Management.Smo.SmoCollectionBase.ClearAndInitialize(String filterQuery, IEnumerable`1 extraFields)
   at Microsoft.SqlTools.ServiceLayer.ObjectExplorer.SmoModel.SqlDatabaseQuerier.Query(SmoQueryContext context, String filter, Boolean refresh, IEnumerable`1 extraProperties) in E:\sts\src\Microsoft.SqlTools.ServiceLayer\ObjectExplorer\SmoModel\SmoQueryModel.cs:line 38
   at Microsoft.SqlTools.ServiceLayer.ObjectExplorer.SmoModel.SmoChildFactoryBase.OnExpandPopulateNonFolders(IList`1 allChildren, TreeNode parent, Boolean refresh, String name, CancellationToken cancellationToken) in E:\sts\src\Microsoft.SqlTools.ServiceLayer\ObjectExplorer\SmoModel\SmoChildFactoryBase.cs:line 141
sqltools Error: 0 : Failed expanding oe children. parent:<server>/Databases error:unknown property IsLedger
...
sqltools Error: 0 : Failed populating oe children. error:unknown property IsLedger
...

This PR fixes that by enabling 'IsLedger' only for SQL 2022 instead of 'AllOnPrem' - I probably didn't realize it applies to SQL 2022 only.. (ref: #1798 (comment))
cc @shueybubbles

Thanks @caohai for capturing this!

Copy link
Contributor

@Charles-Gagnon Charles-Gagnon left a comment

Choose a reason for hiding this comment

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

Thanks for cleaning up the other property filters as well! 😄

@cheenamalhotra cheenamalhotra merged commit ac460ad into main Jan 10, 2023
@cheenamalhotra cheenamalhotra deleted the cheena/fix-isledger branch January 10, 2023 01:47
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.

3 participants