bmuraykhi commented on code in PR #45961:
URL: https://github.com/apache/superset/pull/45961#discussion_r4465455567


##########
tests/testcontainers/db_engine_specs/test_clickhouse.py:
##########
@@ -137,6 +137,47 @@ def test_get_columns_maps_native_types(engine: Engine) -> 
None:
         assert isinstance(spec.sqla_type, Integer)
 
 
+def test_physical_table_metadata_serializes_expression_columns(
+    engine: Engine,
+) -> None:
+    from tests.integration_tests.test_app import app
+
+    with app.app_context():
+        from superset.connectors.sqla.utils import get_physical_table_metadata
+        from superset.db_engine_specs.clickhouse import 
ClickHouseConnectEngineSpec
+        from superset.utils import json
+
+    with engine.begin() as conn:
+        conn.execute(
+            text(
+                "CREATE TABLE pilot_expressions ("
+                "ts DateTime, price Float64, qty UInt32, "
+                "total Float64 ALIAS price * qty, "
+                "day Date MATERIALIZED toDate(ts), "
+                "note String DEFAULT 'x'"
+                ") ENGINE = MergeTree ORDER BY ts"
+            )
+        )
+
+    database = MagicMock()
+    database.db_engine_spec = ClickHouseConnectEngineSpec
+    database.get_dialect.return_value = engine.dialect
+    database.get_extra.return_value = {}
+    database.get_columns.side_effect = (
+        lambda table: ClickHouseConnectEngineSpec.get_columns(inspect(engine), 
table)
+    )
+
+    columns = get_physical_table_metadata(
+        database, Table("pilot_expressions"), normalize_columns=False
+    )
+
+    by_name = {col["column_name"]: dict(col) for col in columns}
+    assert by_name["total"]["clickhouse_alias"] == "price * qty"
+    assert by_name["day"]["clickhouse_materialized"] == "toDate(ts)"
+    assert by_name["note"]["server_default"] == "'x'"
+    json.dumps(columns, default=json.json_int_dttm_ser, ignore_nan=True)
+
+

Review Comment:
   only the first import of `superset.db_engine_specs.clickhouse` needs the app 
context (see the module docstring); `get_physical_table_metadata` doesn't touch 
`current_app`. i ran it against a real server outside the context and got the 
same column types as inside, so the broad `except` isn't hiding anything. the 
other tests in this file use the same pattern.



-- 
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]

  • [PR] fix(da... bmuraykhi (via GitHub) via notifications
    • Re: [P... bmuraykhi (via GitHub) via notifications
    • Re: [P... codecov[bot] (via GitHub) via notifications
    • Re: [P... codeant-ai-for-open-source[bot] (via GitHub) via notifications
    • Re: [P... Copilot (via GitHub) via notifications
    • Re: [P... bmuraykhi (via GitHub) via notifications
    • Re: [P... bmuraykhi (via GitHub) via notifications

Reply via email to