DiggidyDave commented on a change in pull request #7391: feat: see Presto row
and array data types
URL:
https://github.com/apache/incubator-superset/pull/7391#discussion_r279549058
##########
File path: superset/db_engine_specs.py
##########
@@ -804,6 +809,21 @@ class PrestoEngineSpec(BaseEngineSpec):
date_add('day', 1, CAST({col} AS TIMESTAMP))))",
}
+ type_map = {
+ 'boolean': types.Boolean,
+ 'tinyint': mysql.MSTinyInteger,
Review comment:
Looks like skeleton types are now causing CI failures:
```
AttributeError: 'GenericTypeCompiler' object has no attribute 'visit_ARRAY'
During handling of the above exception, another exception occurred:
... lots of stuff ...
sqlalchemy.exc.UnsupportedCompilationError: Compiler
<sqlalchemy.sql.compiler.GenericTypeCompiler object at 0x7f9f6189ed68> can't
render element of type <class
'superset.models.sql_types.presto_sql_types.ARRAY'>
```
As annoying as it would be to revert the types you added (and going back to
the original question I sort of raised) I wonder if it is a viable path to just
leave it with the mysql type there? Obviously it would be less "correct" than
having a well-defined presto dialect but... not sure if the superset project
should be the entity that owns that. :-)
@john-bodley @graceguo-supercat @michellethomas @mistercrunch Does that
value get passed around to other bits of code that might actually care if it is
_actually_ a mysql column? How bad is it to use the only existing built-in
representation of tinyint here, under the mysql dialect?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]