seunggabi commented on code in PR #44500:
URL: https://github.com/apache/superset/pull/44500#discussion_r4067893175
##########
superset/db_engine_specs/base.py:
##########
@@ -1529,6 +1536,21 @@ def epoch_ms_to_dttm(cls) -> str:
"""
return cls.epoch_to_dttm().replace("{col}", "({col}/1000)")
+ @classmethod
+ def epoch_us_to_dttm(cls) -> str:
+ """
+ SQL expression that converts epoch (microseconds) to datetime that can
be used
+ in a query.
+
+ The default routes through ``epoch_ms_to_dttm`` so engines that already
+ override the millisecond conversion keep their validated SQL; the
result
+ has millisecond resolution. Engines with a native microsecond function
Review Comment:
Good catch, and thanks for verifying on SQLite. Reworded the docstring with
your phrasing in 4ba5eb3.
##########
tests/unit_tests/db_engine_specs/test_base.py:
##########
@@ -1652,3 +1652,79 @@ def
test_base_spec_extended_aggregation_func_defaults_to_unsupported(
def test_base_spec_extended_aggregation_func_unknown_name_is_unsupported() ->
None:
"""An aggregate name outside the known extended set is also just None."""
assert
BaseEngineSpec.get_extended_aggregation_func("NOT_A_REAL_AGGREGATE") is None
+
+
+class _EpochSpec(BaseEngineSpec):
+ """Minimal spec implementing only ``epoch_to_dttm``, like most engines."""
+
+ engine = "epoch"
+ engine_name = "epoch"
+ _time_grain_expressions = {
+ None: "{col}",
+ "P1D": "DATE_TRUNC('day', {col})",
+ }
+
+ @classmethod
+ def epoch_to_dttm(cls) -> str:
+ return "from_unixtime({col})"
+
+
[email protected](
+ "pdf,time_grain,expected",
+ [
+ ("epoch_s", None, "from_unixtime(ts)"),
+ ("epoch_ms", None, "from_unixtime((ts/1000))"),
+ ("epoch_us", None, "from_unixtime(((ts/1000)/1000))"),
+ (
+ "epoch_us",
+ "P1D",
+ "DATE_TRUNC('day', from_unixtime(((ts/1000)/1000)))",
+ ),
+ (None, None, "ts"),
+ ],
+)
+def test_get_timestamp_expr_epoch_formats(
+ pdf: str | None, time_grain: str | None, expected: str
+) -> None:
+ """
+ Every ``epoch_*`` python_date_format routes the raw column through the
+ matching ``epoch*_to_dttm`` template before the time grain is applied; the
+ default ``epoch_us_to_dttm`` reuses the millisecond template.
+ """
+ expr = _EpochSpec.get_timestamp_expr(column("ts"), pdf, time_grain)
+ assert str(expr.compile(compile_kwargs={"literal_binds": True})) ==
expected
+
+
[email protected](
+ "spec_path,expected",
+ [
+ # engines relying on the default: their millisecond template is reused
+ ("athena.AthenaEngineSpec", "from_unixtime((({col}/1000)/1000))"),
+ ("crate.CrateEngineSpec", "({col}/1000)"),
+ ("druid.DruidEngineSpec", "MILLIS_TO_TIMESTAMP(({col}/1000))"),
+ ("couchbase.CouchbaseEngineSpec", "MILLIS_TO_STR(({col}/1000))"),
+ # engines with a native microsecond conversion
+ ("bigquery.BigQueryEngineSpec", "TIMESTAMP_MICROS({col})"),
+ ("snowflake.SnowflakeEngineSpec", "DATEADD(US, {col}, '1970-01-01')"),
+ ("kusto.KustoKqlEngineSpec",
"unixtime_microseconds_todatetime({col})"),
+ (
+ "pinot.PinotEngineSpec",
+ "DATETIMECONVERT({col}, '1:MICROSECONDS:EPOCH', "
+ "'1:MICROSECONDS:EPOCH', '1:MICROSECONDS')",
+ ),
+ ],
+)
+def test_epoch_us_to_dttm(spec_path: str, expected: str) -> None:
+ """
+ ``epoch_us_to_dttm`` yields valid SQL for engines that override
+ ``epoch_ms_to_dttm`` (via the default) and for engines with a native
+ microsecond function (via their own override).
+ """
+ import importlib
Review Comment:
Agreed, there was no import-cost reason. Switched to direct class imports in
4ba5eb3.
--
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]
