-
Notifications
You must be signed in to change notification settings - Fork 23
Fix ClearAndInitialize() failure on Dynamics CRM servers (edition 1000) #208
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
Conversation
…ress Co-authored-by: shueybubbles <[email protected]>
| edition = DatabaseEngineEdition.SqlDatabase; | ||
| } | ||
| // If edition is 1000 (Dynamics CRM), treat it as Standalone with Express edition | ||
| if ((int)edition == 1000) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about having a safe default fallback for any unknown editions?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot-swe-agent move this check inside the if block on line 160
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved the edition 1000 check inside a more general unknown editions block (commit 56a7427 → new commit). This now provides a safe default fallback: unknown editions are checked first, then edition 1000 gets Standalone+Express, and other Azure unknown editions fall back to SqlDatabase.
| edition = DatabaseEngineEdition.SqlDatabase; | ||
| } | ||
| // If edition is 1000 (Dynamics CRM), treat it as Standalone with Express edition | ||
| if ((int)edition == 1000) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So do we know why this broke in the first place? The description implies it was working fine before 172.52.0
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The issue was introduced when RealEngineEdition was added to the default set of properties on the Database object (as mentioned by @shueybubbles). This caused SMO to generate queries with temp tables to fetch that property, which Dynamics CRM doesn't support. Before that change, those queries weren't being executed.
|
We added |
…ling Co-authored-by: shueybubbles <[email protected]>
|
@copilot |
Co-authored-by: shueybubbles <[email protected]>
Changed the condition to check both |
| // add RealEngineEdition to the query so we get the edition from sys.database_service_objectives | ||
| if (DatabaseEngineEdition == DatabaseEngineEdition.SqlDatabase) { | ||
| // Only add RealEngineEdition for Azure SQL Database, not for unknown editions like Dynamics CRM (edition 1000) | ||
| if (DatabaseEngineEdition == DatabaseEngineEdition.SqlDatabase && DatabaseEngineType == DatabaseEngineType.SqlAzureDatabase) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot-swe-agent the DatabasEngineEdition value here will never be 1000 because the ServerInformation is converting that value to SqlDatabase during GetServerInformation. ServerConnection or ServerInformation needs to remember the original edition number and check it here to decide if asking for RealEngineEdition is needed. If the original value was anything other than SqlDatabase this property isn't needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added OriginalDatabaseEngineEdition property throughout the stack (ServerInformation → ConnectionManager → ExecutionManager → SqlSmoObject → Server) to preserve the edition value before conversion. The Server now checks OriginalDatabaseEngineEdition == SqlDatabase instead of the converted DatabaseEngineEdition, ensuring RealEngineEdition is only added for genuine Azure SQL Database (not for edition 1000 converted to SqlDatabase). Commit: 3da2a36
… queries Co-authored-by: shueybubbles <[email protected]>
| /// <summary> | ||
| /// Returns the original DatabaseEngineEdition value before any conversion/mapping | ||
| /// </summary> | ||
| internal virtual DatabaseEngineEdition OriginalDatabaseEngineEdition |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot this new property is not needed, is it? The Server can get the original edition directly from its ExecutionManager when needed and no other entity needs to see it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed the OriginalDatabaseEngineEdition property from SqlSmoObject. The Server now directly calls this.ExecutionManager.GetOriginalDatabaseEngineEdition() instead of accessing it through a property. This simplifies the code since only Server needs this value. Commit: 7f4e8c1
…Object Co-authored-by: shueybubbles <[email protected]>
|
@copilot this appears to be working. can you update/add the necessary unit tests? |
Co-authored-by: Benjin <[email protected]>
Added three comprehensive unit tests in ServerInformationTests.cs:
These tests verify the original edition preservation and conversion logic. Commit: c4f5a2d |
|
Completing this internally. Thank you for your service, Copilot. 🫡 |
Fix for ClearAndInitialize() failure on Dynamics CRM servers (edition 1000)
Problem
When connecting to a Dynamics CRM server and calling
ClearAndInitialize()on theDatabasesproperty, the operation fails with:This was working in version 171.30.0 but has been broken since 172.52.0.
Root Cause
The issue was introduced when
RealEngineEditionwas added to the default set of properties on theDatabaseobject. This property requires queryingsys.database_service_objectives, which creates a temp table (#dso). Dynamics CRM servers (which reportSERVERPROPERTY('EngineEdition') = 1000) do not support CREATE TABLE statements.The core problem was that:
DatabaseEngineEdition == SqlDatabaseto decide whether to add RealEngineEditionSolution
Modified the code to preserve the ORIGINAL edition value before any conversion/mapping:
OriginalDatabaseEngineEditionproperty to preserve the edition value before conversionOriginalDatabaseEngineEditioninternal propertyGetOriginalDatabaseEngineEdition()methodExecutionManager.GetOriginalDatabaseEngineEdition()instead of the potentially convertedDatabaseEngineEditionKey change:
This ensures RealEngineEdition is only queried when:
For Dynamics CRM:
GetOriginalDatabaseEngineEdition() == SqlDatabase→ FALSEFor genuine Azure SQL Database:
GetOriginalDatabaseEngineEdition() == SqlDatabase→ TRUETesting
Added comprehensive unit tests in ServerInformationTests.cs:
Impact
Fixes #207
Original prompt
Fixes #207
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.