drivaspreset commented on code in PR #43607:
URL: https://github.com/apache/superset/pull/43607#discussion_r3883099485
##########
tests/unit_tests/db_engine_specs/test_presto.py:
##########
@@ -113,6 +160,203 @@ def test_get_schema_from_engine_params() -> None:
)
[email protected](
+ "schema",
+ [
+ pytest.param("with/slash", id="slash"),
+ pytest.param("with space", id="space"),
+ pytest.param("with%percent", id="percent"),
+ pytest.param("地区", id="unicode"),
+ pytest.param("plain", id="plain"),
+ ],
+)
+def test_schema_survives_engine_params_round_trip(schema: str) -> None:
+ from superset.db_engine_specs.presto import PrestoEngineSpec
+
+ uri, _connect_args = PrestoEngineSpec.adjust_engine_params(
+ make_url("presto://localhost:8080/hive"),
+ {},
+ schema=schema,
+ )
+
+ assert PrestoEngineSpec.get_schema_from_engine_params(uri, {}) == schema
+
+
+def test_get_catalog_names_lists_catalogs() -> None:
+ from superset.db_engine_specs.presto import PrestoEngineSpec
+
+ inspector = mock.MagicMock()
+ conn = inspector.engine.connect.return_value.__enter__.return_value
+ conn.execute.return_value = [("jmx",), ("tpch",), ("memory",)]
+
+ result = PrestoEngineSpec.get_catalog_names(mock.MagicMock(), inspector)
+
+ assert result == {"jmx", "tpch", "memory"}
+ assert str(conn.execute.call_args[0][0]) == "SHOW CATALOGS"
+
+
+def test_get_view_names_queries_information_schema_with_schema() -> None:
+ from superset.db_engine_specs.presto import PrestoEngineSpec
+
+ database = mock.MagicMock()
+ cursor = database.get_raw_connection().__enter__().cursor()
+ cursor.fetchall.return_value = [["a", "b,", "c"], ["d", "e"]]
+
+ result = PrestoEngineSpec.get_view_names(database, mock.Mock(),
"my_schema")
+
+ assert result == {"a", "d"}
+ cursor.execute.assert_called_once_with(
+ dedent(
+ """
+ SELECT table_name FROM information_schema.tables
+ WHERE table_schema = %(schema)s
+ AND table_type = 'VIEW'
+ """
+ ).strip(),
+ {"schema": "my_schema"},
+ )
+
+
+def test_get_view_names_queries_information_schema_without_schema() -> None:
+ from superset.db_engine_specs.presto import PrestoEngineSpec
+
+ database = mock.MagicMock()
+ cursor = database.get_raw_connection().__enter__().cursor()
+ cursor.fetchall.return_value = [["a", "b,", "c"], ["d", "e"]]
+
+ result = PrestoEngineSpec.get_view_names(database, mock.Mock(), None)
+
+ assert result == {"a", "d"}
+ cursor.execute.assert_called_once_with(
+ dedent(
+ """
+ SELECT table_name FROM information_schema.tables
+ WHERE table_type = 'VIEW'
+ """
+ ).strip(),
+ {},
+ )
+
+
+def test_get_view_names_returns_empty_set_when_no_views() -> None:
+ from superset.db_engine_specs.presto import PrestoEngineSpec
+
+ database = mock.MagicMock()
+ database.get_raw_connection().__enter__().cursor().fetchall.return_value =
[]
+
+ assert PrestoEngineSpec.get_view_names(database, mock.Mock(), "empty") ==
set()
+
+
+def test_get_view_names_propagates_driver_error() -> None:
+ from pyhive.exc import DatabaseError
+
+ from superset.db_engine_specs.presto import PrestoEngineSpec
+
+ database = mock.MagicMock()
+ database.get_raw_connection().__enter__().cursor().execute.side_effect = (
+ DatabaseError("Access Denied: Cannot select from table
information_schema")
+ )
+
+ with pytest.raises(DatabaseError, match="Access Denied"):
+ PrestoEngineSpec.get_view_names(database, mock.Mock(), "my_schema")
+
+
+def test_get_table_names_subtracts_views() -> None:
+ from superset.db_engine_specs.presto import PrestoEngineSpec
+
+ inspector = mock.MagicMock()
+ inspector.get_table_names.return_value = ["t1", "t2", "v1", "v2"]
+ database = mock.MagicMock()
+ database.get_raw_connection().__enter__().cursor().fetchall.return_value =
[
+ ["v1"],
+ ["v2"],
+ ]
+
+ result = PrestoEngineSpec.get_table_names(database, inspector, "my_schema")
+
+ assert result == {"t1", "t2"}
+
+
+def test_get_table_names_returns_empty_set_for_empty_schema() -> None:
+ from superset.db_engine_specs.presto import PrestoEngineSpec
+
+ inspector = mock.MagicMock()
+ inspector.get_table_names.return_value = []
+ database = mock.MagicMock()
+ database.get_raw_connection().__enter__().cursor().fetchall.return_value =
[]
+
+ assert PrestoEngineSpec.get_table_names(database, inspector, "empty") ==
set()
+
+
+def test_get_create_view_returns_view_definition() -> None:
+ from superset.db_engine_specs.presto import PrestoEngineSpec
+
+ database = mock.MagicMock()
+ cursor = database.get_raw_connection().__enter__().cursor()
+ cursor.fetchall.return_value = [["CREATE VIEW v AS SELECT 1", "b"], ["d"]]
+
+ result = PrestoEngineSpec.get_create_view(database, schema="s", table="v")
+
+ assert result == "CREATE VIEW v AS SELECT 1"
+
+
[email protected](
+ "schema,table,expected_sql",
+ [
+ pytest.param("s", "v", "SHOW CREATE VIEW s.v", id="simple"),
+ pytest.param(
+ "analytics",
+ "daily_users",
+ "SHOW CREATE VIEW analytics.daily_users",
+ id="schema_qualified",
+ ),
+ pytest.param(
+ "s",
+ 'v" OR 1=1',
Review Comment:
Agreed, that case encoded "identifiers aren't validated" as expected
behaviour, which would turn a future quoting or validation fix into a red test
that looks like a regression.
Swapped it for a valid qualified identifier as you suggested:
Raw_2024.Daily_Active_Users_v2, which still covers something the other two
cases don't, that schema and table are passed through without normalisation (no
lower-casing).
On the bot's suggestion of parameterised queries: identifiers can't be bound
as parameters in SQL, only values can. The equivalent fix would be identifier
quoting via the dialect's preparer, which is a production change and out of
scope for this test-only PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
