[sqlserver] Fix Azure SQL schema query selection#23533
Conversation
1d36032 to
e06368d
Compare
29dad99 to
56330c9
Compare
dcab759 to
ea3a14c
Compare
ea3a14c to
cbebc43
Compare
🎉 All green!❄️ No new flaky tests detected 🎯 Code Coverage (details) 🔗 Commit SHA: 76ac311 | Docs | Datadog PR Page | Give us feedback! |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files🚀 New features to boost your workflow:
|
| def _record_database_compatibility_levels(self, databases: list[DatabaseInfo]) -> None: | ||
| self._database_compatibility_levels = {} | ||
| for database in databases: | ||
| self._database_compatibility_levels[database["name"]] = int(database["compatibility_level"]) |
There was a problem hiding this comment.
Is it possible for this int to raise an error if the value isn't numeric as expected?
There was a problem hiding this comment.
compatibility_level is a required tinyint type so it would't be an issue
|
|
||
| def _should_use_legacy_schema_query(self, database_name: str) -> bool: | ||
| engine_edition = self._check.static_info_cache.get(STATIC_INFO_ENGINE_EDITION) | ||
| if is_azure_database(engine_edition): |
There was a problem hiding this comment.
Do we need to branch on azure here? Is compatibility_level not supported on other SQL Server installations?
There was a problem hiding this comment.
trying to reduce the blast radius but it could also be a good idea to completely rely on compatibility_level for schema. the field is available in all MS documented sqlserver versions
There was a problem hiding this comment.
Good point that compatibility_level is available broadly. I updated this to use it for all engine editions as the database-scoped guard, but kept the Azure branch to skip only the boxed ProductMajorVersion check. Compatibility level alone is not sufficient on boxed SQL Server because SQL Server 2016 can have compatibility level 130 while still lacking STRING_AGG; Azure SQL DB/MI report a non-boxed major version, so they need the engine-edition exception.
|
|
||
| major_version = int(self._check.static_info_cache.get(STATIC_INFO_MAJOR_VERSION) or 0) | ||
| if major_version == 0: | ||
| self._check.log.debug("major_version is not available yet, defaulting to 2016 or earlier") |
There was a problem hiding this comment.
We should maintain the previous logic and not collect if we don't have the version yet.
There was a problem hiding this comment.
Updated this to preserve the legacy fallback for the missing-information cases. Non-Azure still defaults to the legacy schema query when major_version is unavailable, and any engine edition defaults to legacy if the per-database compatibility_level was not recorded. Azure SQL DB/MI only bypass the boxed major-version check because their reported ProductMajorVersion is not comparable.
815c6cc to
0c12d4d
Compare
0c12d4d to
9727169
Compare
41e026c to
b9e74a6
Compare
b9e74a6 to
76ac311
Compare
Validation ReportAll 20 validations passed. Show details
|
What changed
This updates SQL Server schema collection so Azure SQL Database and Azure SQL Managed Instance no longer use boxed SQL Server
ProductMajorVersionalone to choose the schema query path. The collector records each database'ssys.databases.compatibility_leveland uses it as the database-scoped guard for the modern schema query.The emitted database schema payload now includes
compatibility_level; the backend ignores unknown fields, and the metadata tests normalize the value because it varies by SQL Server version.Query selection logic
The modern schema query depends on two different SQL Server capabilities:
STRING_AGGis an engine feature. For boxed SQL Server, the collector still requires SQL Server 2017 or later because SQL Server 2016 can run a database at compatibility level 130 but still does not supportSTRING_AGG.compatibility_leveland uses the legacy query when it is below 130 or missing.ProductMajorVersionis not comparable to boxed SQL Server releases. They can report version 12 while still supportingSTRING_AGG.Why
Azure SQL Database and Azure SQL Managed Instance can report version strings such as
Microsoft SQL Azure (RTM) - 12.0.2000.8, where major version12is not SQL Server 2014. Treating that value as a boxed SQL Server release incorrectly sends modern Azure SQL deployments through the legacy schema path.Connection reuse for the legacy table-detail path is split into #23544. Managed Instance AO secondary lag handling is split into #23558.
https://datadoghq.atlassian.net/browse/SDBM-2571