Skip to content

Commit

Permalink
Fix false positive in SQL sensor full table scan check (#91134)
Browse files Browse the repository at this point in the history
  • Loading branch information
bdraco committed Apr 10, 2023
1 parent 82c80ec commit 6e9fcbf
Show file tree
Hide file tree
Showing 3 changed files with 58 additions and 6 deletions.
4 changes: 3 additions & 1 deletion homeassistant/components/sql/sensor.py
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,9 @@ async def async_setup_sensor(
# If the query has a unique id and they fix it we can dismiss the issue
# but if it doesn't have a unique id they have to ignore it instead

if "ENTITY_ID" in upper_query and "STATES_META" not in upper_query:
if (
"ENTITY_ID," in upper_query or "ENTITY_ID " in upper_query
) and "STATES_META" not in upper_query:
_LOGGER.error(
"The query `%s` contains the keyword `entity_id` but does not "
"reference the `states_meta` table. This will cause a full table "
Expand Down
17 changes: 17 additions & 0 deletions tests/components/sql/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,23 @@
}
}

YAML_CONFIG_FULL_TABLE_SCAN_WITH_MULTIPLE_COLUMNS = {
"sql": {
CONF_NAME: "Get entity_id",
CONF_QUERY: "SELECT entity_id,state_id from states",
CONF_COLUMN_NAME: "entity_id",
}
}

YAML_CONFIG_WITH_VIEW_THAT_CONTAINS_ENTITY_ID = {
"sql": {
CONF_NAME: "Get entity_id",
CONF_QUERY: "SELECT value from view_sensor_db_unique_entity_ids;",
CONF_COLUMN_NAME: "value",
}
}


YAML_CONFIG_BINARY = {
"sql": {
CONF_DB_URL: "sqlite://",
Expand Down
43 changes: 38 additions & 5 deletions tests/components/sql/test_sensor.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@
YAML_CONFIG_BINARY,
YAML_CONFIG_FULL_TABLE_SCAN,
YAML_CONFIG_FULL_TABLE_SCAN_NO_UNIQUE_ID,
YAML_CONFIG_FULL_TABLE_SCAN_WITH_MULTIPLE_COLUMNS,
YAML_CONFIG_WITH_VIEW_THAT_CONTAINS_ENTITY_ID,
init_integration,
)

Expand Down Expand Up @@ -353,24 +355,55 @@ async def test_issue_when_using_old_query(
assert issue.translation_placeholders == {"query": config[CONF_QUERY]}


@pytest.mark.parametrize(
"yaml_config",
[
YAML_CONFIG_FULL_TABLE_SCAN_NO_UNIQUE_ID,
YAML_CONFIG_FULL_TABLE_SCAN_WITH_MULTIPLE_COLUMNS,
],
)
async def test_issue_when_using_old_query_without_unique_id(
recorder_mock: Recorder, hass: HomeAssistant, caplog: pytest.LogCaptureFixture
recorder_mock: Recorder,
hass: HomeAssistant,
caplog: pytest.LogCaptureFixture,
yaml_config: dict[str, Any],
) -> None:
"""Test we create an issue for an old query that will do a full table scan."""

assert await async_setup_component(
hass, DOMAIN, YAML_CONFIG_FULL_TABLE_SCAN_NO_UNIQUE_ID
)
assert await async_setup_component(hass, DOMAIN, yaml_config)
await hass.async_block_till_done()
assert "Query contains entity_id but does not reference states_meta" in caplog.text

assert not hass.states.async_all()
issue_registry = ir.async_get(hass)

config = YAML_CONFIG_FULL_TABLE_SCAN_NO_UNIQUE_ID["sql"]
config = yaml_config["sql"]
query = config[CONF_QUERY]

issue = issue_registry.async_get_issue(
DOMAIN, f"entity_id_query_does_full_table_scan_{query}"
)
assert issue.translation_placeholders == {"query": query}


async def test_no_issue_when_view_has_the_text_entity_id_in_it(
recorder_mock: Recorder, hass: HomeAssistant, caplog: pytest.LogCaptureFixture
) -> None:
"""Test we do not trigger the full table scan issue for a custom view."""

with patch(
"homeassistant.components.sql.sensor.scoped_session",
):
await init_integration(
hass, YAML_CONFIG_WITH_VIEW_THAT_CONTAINS_ENTITY_ID["sql"]
)
async_fire_time_changed(
hass,
dt.utcnow() + timedelta(minutes=1),
)
await hass.async_block_till_done()

assert (
"Query contains entity_id but does not reference states_meta" not in caplog.text
)
assert hass.states.get("sensor.get_entity_id") is not None

0 comments on commit 6e9fcbf

Please sign in to comment.