add initial clickhouse support #4057
Conversation
| base.ischema_names["UInt256"] = INTEGER | ||
|
|
||
| register_custom_type(custom_types.common.Array, ArrayTypeClass) | ||
| register_custom_type(custom_types.ip.IPv4, NumberTypeClass) |
There was a problem hiding this comment.
IPv6 is mapped as string but this one as Number, why?
There was a problem hiding this comment.
thanks for the clarificaiton
| yield wu | ||
|
|
||
| def _get_db_name(self) -> str: | ||
| return getattr(self.config, "database_alias") |
There was a problem hiding this comment.
Is database alias a mandatory config parameter?
As I can see there is a database property as well.
Is it possible to set only database and not database_alias?
There was a problem hiding this comment.
database parameter is used only for the connection.
database_alias is used as platform_instance and most likely should be replaced to it.
Both parameters are not mandatory.
There was a problem hiding this comment.
clickhouse has schema.table objects structure
There was a problem hiding this comment.
@ne1r0n : I would recommend using the platform_instance config going forward and not having database_alias option in this connector, now that we have platform instance support in the core classes.
There was a problem hiding this comment.
@shirshanka I've changed database_alias to platform_instance
ne1r0n
force-pushed
the
clickhouse_support
branch
3 times, most recently
from
3d4fc8e to
94b0235
Compare
ne1r0n
force-pushed
the
clickhouse_support
branch
from
94b0235 to
56505b0
Compare
Co-authored-by: Shirshanka Das <[email protected]>
Checklist