From cd5ea2085ee505e8a06da958cd7967d90fb56e58 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 9 Apr 2023 09:30:18 -1000 Subject: [PATCH 1/2] Fix false positive in SQL sensor full table scan check --- homeassistant/components/sql/sensor.py | 2 +- tests/components/sql/__init__.py | 10 ++++++++++ tests/components/sql/test_sensor.py | 24 ++++++++++++++++++++++++ 3 files changed, 35 insertions(+), 1 deletion(-) diff --git a/homeassistant/components/sql/sensor.py b/homeassistant/components/sql/sensor.py index 8408b98730b99a..1d14f3d2302eb1 100644 --- a/homeassistant/components/sql/sensor.py +++ b/homeassistant/components/sql/sensor.py @@ -162,7 +162,7 @@ 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 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 " diff --git a/tests/components/sql/__init__.py b/tests/components/sql/__init__.py index c976f87f50f503..a2fff2ddf382e0 100644 --- a/tests/components/sql/__init__.py +++ b/tests/components/sql/__init__.py @@ -81,6 +81,16 @@ } } + +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://", diff --git a/tests/components/sql/test_sensor.py b/tests/components/sql/test_sensor.py index 811bb3f45bf8b0..2e83fc134c6f6f 100644 --- a/tests/components/sql/test_sensor.py +++ b/tests/components/sql/test_sensor.py @@ -24,6 +24,7 @@ YAML_CONFIG_BINARY, YAML_CONFIG_FULL_TABLE_SCAN, YAML_CONFIG_FULL_TABLE_SCAN_NO_UNIQUE_ID, + YAML_CONFIG_WITH_VIEW_THAT_CONTAINS_ENTITY_ID, init_integration, ) @@ -374,3 +375,26 @@ async def test_issue_when_using_old_query_without_unique_id( 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 From 34230c76ae714d6fb6b3cdd511e6c559f1b53ee6 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 9 Apr 2023 11:07:24 -1000 Subject: [PATCH 2/2] adjust --- homeassistant/components/sql/sensor.py | 4 +++- tests/components/sql/__init__.py | 7 +++++++ tests/components/sql/test_sensor.py | 19 ++++++++++++++----- 3 files changed, 24 insertions(+), 6 deletions(-) diff --git a/homeassistant/components/sql/sensor.py b/homeassistant/components/sql/sensor.py index 1d14f3d2302eb1..b6cce467e1fae7 100644 --- a/homeassistant/components/sql/sensor.py +++ b/homeassistant/components/sql/sensor.py @@ -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 " diff --git a/tests/components/sql/__init__.py b/tests/components/sql/__init__.py index a2fff2ddf382e0..97df7fe253ecc7 100644 --- a/tests/components/sql/__init__.py +++ b/tests/components/sql/__init__.py @@ -81,6 +81,13 @@ } } +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": { diff --git a/tests/components/sql/test_sensor.py b/tests/components/sql/test_sensor.py index 2e83fc134c6f6f..7e289565b373a1 100644 --- a/tests/components/sql/test_sensor.py +++ b/tests/components/sql/test_sensor.py @@ -24,6 +24,7 @@ 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, ) @@ -354,21 +355,29 @@ 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(