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

Fix false positive in SQL sensor full table scan check #91134

Merged
merged 2 commits into from
Apr 10, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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