fix(sqla): replace custom dttm type with literal_column

[villebro]

SUMMARY

Some time ago #13622 introduced support for emitting temporal filters from native/cross filters. This caused a regression that was fixed in #16634, which itself caused another regression #17143. The original feature, which relied on creating a custom temporal type that can be used to compile temporal expressions in SQLAlchemy queries proved to be problematic, in part due to the fact that it required using the DATETIME type which isn't natvely supported by Presto.

For these types of use cases, the documentation recommends creating a custom class that extends TypeDecorator and implementing process_bind_param and/or process_literal_value: https://docs.sqlalchemy.org/en/14/core/custom_types.html#typedecorator-recipes. However, during testing this was shown to not work universally, as some dialects expected the return value to be a date/datetime, while others expected it to always be a str.

A simpler and universally supported solution is to just use literal_column. This simplifies the codebase and should ensure that temporal adhoc filters always work. The temporal conversion functions for Presto, Trino and Athena are also updated to use the recommended casts from the docs (note how from_iso8601_* is no longer recommended):

• https://prestodb.io/docs/current/functions/datetime.html (Presto db. The AWS Athena docs also point to this: https://docs.aws.amazon.com/athena/latest/ug/presto-functions.html)
• https://docs.starburst.io/latest/functions/datetime.html (Starburst)
• https://trino.io/docs/current/functions/datetime.html (Trino)

Note that this change might be seen as making the filter_values_handler helper function slightly messy with the added arguments. However, the function can be streamlined once the native Druid connector is removed and the helper can be made more specific to the SQLAlchemy model.

This PR doesn't add new tests, as this test covers these changes:

superset/tests/integration_tests/charts/data/api_tests.py

Lines 359 to 397 in aff10a7

	@unittest.skip("Failing due to timezone difference")
	@pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
	def test_chart_data_dttm_filter(self):
	"""
	Chart data API: Ensure temporal column filter converts epoch to dttm expression
	"""
	table = self.get_birth_names_dataset()
	if table.database.backend == "presto":
	# TODO: date handling on Presto not fully in line with other engine specs
	return
	self.query_context_payload["queries"][0]["time_range"] = ""
	dttm = self.get_dttm()
	ms_epoch = dttm.timestamp() * 1000
	self.query_context_payload["queries"][0]["filters"][0] = {
	"col": "ds",
	"op": "!=",
	"val": ms_epoch,
	}
	rv = self.post_assert_metric(CHART_DATA_URI, self.query_context_payload, "data")
	response_payload = json.loads(rv.data.decode("utf-8"))
	result = response_payload["result"][0]
	# assert that unconverted timestamp is not present in query
	assert str(ms_epoch) not in result["query"]
	# assert that converted timestamp is present in query where supported
	dttm_col: Optional[TableColumn] = None
	for col in table.columns:
	if col.column_name == table.main_dttm_col:
	dttm_col = col
	if dttm_col:
	dttm_expression = table.database.db_engine_spec.convert_dttm(
	dttm_col.type,
	dttm,
	)
	self.assertIn(dttm_expression, result["query"])
	else:
	raise Exception("ds column not found")

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

• Has associated issue: Closes Presto engine spec broken - Date column mapped to Datetime #17143
• Required feature flags:
• Changes UI
• Includes DB Migration (follow approval process in SIP-59)
  • Migration is atomic, supports rollback & is backwards-compatible
  • Confirm DB migration upgrade and downgrade tested
  • Runtime estimates and downtime expectations provided
• Introduces new feature or API
• Removes existing feature or API

[codecov]

Codecov Report

Merging #19917 (add1492) into master (aff10a7) will decrease coverage by 0.18%.
The diff coverage is 85.71%.

❗ Current head add1492 differs from pull request most recent head b8c4c7e. Consider uploading reports for the commit b8c4c7e to get more accurate results

@@            Coverage Diff             @@
##           master   #19917      +/-   ##
==========================================
- Coverage   66.52%   66.34%   -0.19%     
==========================================
  Files        1714     1713       -1     
  Lines       65052    65035      -17     
  Branches     6722     6722              
==========================================
- Hits        43279    43148     -131     
- Misses      20061    20175     +114     
  Partials     1712     1712              

Flag	Coverage Δ
hive	?
mysql	81.94% <85.71%> (+<0.01%)	⬆️
postgres	81.99% <85.71%> (+<0.01%)	⬆️
presto	?
python	82.04% <85.71%> (-0.37%)	⬇️
sqlite	?
unit	?

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
superset/connectors/druid/models.py	82.99% <ø> (ø)
superset/db_engine_specs/base.py	87.45% <ø> (-0.90%)	⬇️
superset/connectors/base/models.py	88.21% <70.00%> (-0.44%)	⬇️
superset/connectors/sqla/models.py	88.75% <100.00%> (-1.30%)	⬇️
superset/db_engine_specs/athena.py	89.65% <100.00%> (ø)
superset/db_engine_specs/presto.py	83.36% <100.00%> (-5.34%)	⬇️
superset/db_engine_specs/trino.py	71.42% <100.00%> (ø)
superset/db_engines/hive.py	0.00% <0.00%> (-85.19%)	⬇️
superset/db_engine_specs/hive.py	70.11% <0.00%> (-15.71%)	⬇️
superset/db_engine_specs/sqlite.py	91.89% <0.00%> (-5.41%)	⬇️
... and 11 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update aff10a7...b8c4c7e. Read the comment docs.

[nytai]

Confirmed fix works using a presto connection, date column is correctly identified as Date type and time filters work now.