Skip to content

Commit 8af850d

Browse files
authored
handle case insensitive db name in schema collection (#19384)
* handle case insensitive db name in schema collection * add unit test * add changelog * update log
1 parent 64cafe2 commit 8af850d

File tree

5 files changed

+79
-7
lines changed

5 files changed

+79
-7
lines changed

sqlserver/changelog.d/19384.fixed

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Fix KeyError in SQL Server schema collection caused by case-insensitive database name mismatches.

sqlserver/datadog_checks/sqlserver/connection.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
from datadog_checks.base import AgentCheck, ConfigurationError
99
from datadog_checks.base.log import get_check_logger
1010
from datadog_checks.sqlserver.cursor import CommenterCursorWrapper
11-
from datadog_checks.sqlserver.utils import construct_use_statement
11+
from datadog_checks.sqlserver.utils import construct_use_statement, is_collation_case_insensitive
1212

1313
try:
1414
import adodbapi
@@ -361,7 +361,7 @@ def _check_db_exists(self):
361361
cursor.execute(DATABASE_EXISTS_QUERY)
362362
for row in cursor.fetchall():
363363
# collation_name can be NULL if db offline, in that case assume its case_insensitive
364-
case_insensitive = not row.collation_name or 'CI' in row.collation_name
364+
case_insensitive = is_collation_case_insensitive(row.collation_name)
365365
self.existing_databases[row.name.lower()] = (
366366
case_insensitive,
367367
row.name,

sqlserver/datadog_checks/sqlserver/schemas.py

Lines changed: 24 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,13 @@
3131
SCHEMA_QUERY,
3232
TABLES_IN_SCHEMA_QUERY,
3333
)
34-
from datadog_checks.sqlserver.utils import convert_to_bool, execute_query, get_list_chunks, is_azure_sql_database
34+
from datadog_checks.sqlserver.utils import (
35+
convert_to_bool,
36+
execute_query,
37+
get_list_chunks,
38+
is_azure_sql_database,
39+
is_collation_case_insensitive,
40+
)
3541

3642

3743
class SubmitData:
@@ -57,9 +63,23 @@ def reset(self):
5763
self.db_to_schemas.clear()
5864
self.db_info.clear()
5965

60-
def store_db_infos(self, db_infos):
66+
def store_db_infos(self, db_infos, databases):
67+
dbs = set(databases)
6168
for db_info in db_infos:
62-
self.db_info[db_info['name']] = db_info
69+
case_insensitive = is_collation_case_insensitive(db_info.get('collation'))
70+
db_name = db_info['name']
71+
db_name_lower = db_name.lower()
72+
if db_name not in dbs:
73+
if db_name.lower() in dbs and case_insensitive:
74+
db_name = db_name_lower
75+
else:
76+
self._log.debug(
77+
"Skipping db {} as it is not in the databases list {} or collation is case sensitive".format(
78+
db_name, dbs
79+
)
80+
)
81+
continue
82+
self.db_info[db_name] = db_info
6383

6484
def store(self, db_name, schema, tables, columns_count):
6585
self._columns_count += columns_count
@@ -277,7 +297,7 @@ def _collect_schemas_data(self):
277297

278298
databases = self._check.get_databases()
279299
db_infos = self._query_db_information(databases)
280-
self._data_submitter.store_db_infos(db_infos)
300+
self._data_submitter.store_db_infos(db_infos, databases)
281301
self._fetch_for_databases()
282302
self._data_submitter.submit()
283303
self._log.debug("Finished collect_schemas_data")

sqlserver/datadog_checks/sqlserver/utils.py

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -215,3 +215,12 @@ def convert_to_bool(value):
215215
return bool(value)
216216
else:
217217
return value
218+
219+
220+
def is_collation_case_insensitive(collation):
221+
"""
222+
Checks if the collation is case insensitive
223+
:param collation: The collation string
224+
:return: bool
225+
"""
226+
return not collation or 'CI' in collation.upper()

sqlserver/tests/test_unit.py

Lines changed: 43 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -754,7 +754,9 @@ def test_submit_data():
754754

755755
dataSubmitter, submitted_data = set_up_submitter_unit_test()
756756

757-
dataSubmitter.store_db_infos([{"id": 3, "name": "test_db1"}, {"id": 4, "name": "test_db2"}])
757+
dataSubmitter.store_db_infos(
758+
[{"id": 3, "name": "test_db1"}, {"id": 4, "name": "test_db2"}], ["test_db1", "test_db2"]
759+
)
758760
schema1 = {"id": "1"}
759761
schema2 = {"id": "2"}
760762
schema3 = {"id": "3"}
@@ -787,6 +789,46 @@ def test_submit_data():
787789
assert deep_compare(data, expected_data)
788790

789791

792+
@pytest.mark.parametrize(
793+
"db_infos, databases, expected_dbs",
794+
[
795+
pytest.param(
796+
[
797+
{"id": 3, "name": "test_db1", "collation": "SQL_Latin1_General_CP1_CI_AS"},
798+
{"id": 4, "name": "TEST_DB2", "collation": "SQL_Latin1_General_CP1_CI_AS"},
799+
],
800+
["test_db1", "test_db2"],
801+
["test_db1", "test_db2"],
802+
id="case_insensitive",
803+
),
804+
pytest.param(
805+
[{"id": 3, "name": "test_db1", "collation": "SQL_Latin1_General_CP1_CS_AS"}],
806+
["TEST_DB1"],
807+
[],
808+
id="case_sensitive",
809+
),
810+
pytest.param(
811+
[{"id": 3, "name": "test_db1", "collation": "SQL_Latin1_General_CP1_CS_AS"}],
812+
["test_db1"],
813+
["test_db1"],
814+
id="case_sensitive_lowercase",
815+
),
816+
pytest.param(
817+
[{"id": 3, "name": "TEST_DB1", "collation": "SQL_Latin1_General_CP1_CS_AS"}],
818+
["TEST_DB1"],
819+
["TEST_DB1"],
820+
id="case_sensitive_uppercase",
821+
),
822+
],
823+
)
824+
def test_store_db_infos_case_sensitive(db_infos, databases, expected_dbs):
825+
dataSubmitter, _ = set_up_submitter_unit_test()
826+
dataSubmitter.db_info.clear()
827+
828+
dataSubmitter.store_db_infos(db_infos, databases)
829+
assert list(dataSubmitter.db_info.keys()) == expected_dbs
830+
831+
790832
def test_fetch_throws(instance_docker):
791833
check = SQLServer(CHECK_NAME, {}, [instance_docker])
792834
schemas = Schemas(check, check._config)

0 commit comments

Comments
 (0)