fix: TemporalWrapperType string representation

[villebro]

SUMMARY

Currently the temporal wrapper class used by the sqla model returns Type[TypeEngine], although it should be returning an instantiated TypeEngine, which results in an incorrect string representation of the type object. This doesn't affect regular query generation (the types and instances can mostly be used interchangeably when writing SqlAlchemy code), but causes trouble in the Presto db engine spec here:

superset/superset/db_engine_specs/presto.py

Lines 385 to 407 in 3fe2e6e

	column_spec = cls.get_column_spec(field_info[1])
	column_type = column_spec.sqla_type if column_spec else None
	if column_type is None:
	column_type = types.String()
	logger.info(
	"Did not recognize type %s of column %s",
	field_info[1],
	field_info[0],
	)
	if field_info[1] == "array" or field_info[1] == "row":
	stack.append((field_info[0], field_info[1]))
	full_parent_path = cls._get_full_name(stack)
	result.append(
	cls._create_column_info(full_parent_path, column_type)
	)
	else: # otherwise this field is a basic data type
	full_parent_path = cls._get_full_name(stack)
	column_name = "{}.{}".format(
	full_parent_path, field_info[0]
	)
	result.append(
	cls._create_column_info(column_name, column_type)
	)

and more specifically here (notice how the expected data type for data_type is types.TypeEngine, not Type[types.TypeEngine]):

superset/superset/db_engine_specs/presto.py

Lines 293 to 303 in 3fe2e6e

	@classmethod
	def _create_column_info(
	cls, name: str, data_type: types.TypeEngine
	) -> Dict[str, Any]:
	"""
	Create column info object
	:param name: column name
	:param data_type: column data type
	:return: column info object
	"""
	return {"name": name, "type": f"{data_type}"}

Without this fix the new test fails with the following error:

>       assert str(new_type) == str(orig_type)
E       assert "<class 'supe...WrapperType'>" == 'DATETIME'
E         - DATETIME
E         + <class 'superset.models.sql_types.base.literal_dttm_type_factory.<locals>.TemporalWrapperType'>

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

• Has associated issue: closes [SQLLab] Timestamp column type metadata shows up incorrectly in left panel #16258
• 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 #16614 (b8df03e) into master (3fe2e6e) will decrease coverage by 0.21%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master   #16614      +/-   ##
==========================================
- Coverage   76.73%   76.51%   -0.22%     
==========================================
  Files        1003     1003              
  Lines       53875    53875              
  Branches     7289     7289              
==========================================
- Hits        41343    41225     -118     
- Misses      12292    12410     +118     
  Partials      240      240              

Flag	Coverage Δ
hive	?
mysql	81.54% <100.00%> (ø)
postgres	81.60% <100.00%> (ø)
presto	?
python	81.69% <100.00%> (-0.43%)	⬇️
sqlite	81.25% <100.00%> (ø)

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

Impacted Files	Coverage Δ
superset/db_engine_specs/base.py	88.00% <ø> (-0.39%)	⬇️
superset/models/sql_types/base.py	52.63% <100.00%> (ø)
superset/db_engines/hive.py	0.00% <0.00%> (-82.15%)	⬇️
superset/db_engine_specs/hive.py	69.80% <0.00%> (-16.87%)	⬇️
superset/db_engine_specs/presto.py	83.47% <0.00%> (-6.91%)	⬇️
superset/views/database/mixins.py	81.03% <0.00%> (-1.73%)	⬇️
superset/connectors/sqla/models.py	88.04% <0.00%> (-1.66%)	⬇️
superset/models/core.py	89.14% <0.00%> (-0.26%)	⬇️
superset/utils/core.py	89.76% <0.00%> (-0.13%)	⬇️

---

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 3fe2e6e...b8df03e. Read the comment docs.

[dpgaspar]

LGTM, nice catch