Support inspection of Redshift datatypes#242
Conversation
jklukas
left a comment
There was a problem hiding this comment.
Left some early thoughts. Feel free to force push the content to the trigger-integration branch of the main repo when you're ready to have integration tests run.
| return value | ||
|
|
||
| # Mapping for database schema inspection of Amazon Redshift datatypes | ||
| redshift_ischema_names = { |
There was a problem hiding this comment.
Can we define this constant in UPPER_CAMEL_CASE and move it to the top of the file with other constants?
|
I added a few more changes here as I noticed there were still some issues with type inspection
Use case: from sqlalchemy_redshift.dialect import TIMETZ, SUPER, TIMESTAMPTZ, GEOMETRY
for x in (TIMETZ(), TIMESTAMPTZ(), SUPER(), GEOMETRY()):
print(x)TIMETZ
TIMESTAMPTZ
SUPER
GEOMETRYprevious behavior
I don’t think we can move |
Hmm, I pushed to |
I'll contact Travis support to get this unstuck.
Likewise! |
|
Thanks for getting Travis going again, @jklukas! Integration tests are all passing. Am I ok to merge? |
|
Looks good! Feel free to merge. |
This PR modifies
RedshiftDialectMixinto support database schema inspection of Amazon Redshift datatypes. It adds entries inischema_namesfor these Redshift specific datatypes. It also makes a small change to the definition ofTimetzandTimestamptzdatatype definition for compatibility with SqlAlchemy insepctor.Use case: Support for Redshift datatypes in Querybook Metastore
Issue: When
inspect.get_columns(...)is called,NullType()is returned as the type for columns with typegeometryandsuper, which causes issues in downstream tools like Querybook's metastore.Repro:
where
my_tableis defined asFor columns with type
timetzandtimestamptz,sa.dialects.postgresql.TIMEandsa.dialects.postgresql.TIMESTAMPare returned, which isn't ideal since we've definedTIMETZandTIMESTAMPTZdatatypes withinsqlalchemy-redshift.sqlalchemy.engine.dialects.postgresql.base.PGDialect._get_column_infouses theischema_namesto map datatype string names to datatype classes.screenshots below show the behavior before & after this change when the above use case is run:
note entries 11, 12, 15, 16
behavior before:
behavior after:

Todos