Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MySQL compatibility: native type aliases #49168

Closed
slvrtrn opened this issue Apr 26, 2023 · 6 comments · Fixed by #49577
Closed

MySQL compatibility: native type aliases #49168

slvrtrn opened this issue Apr 26, 2023 · 6 comments · Fixed by #49577
Assignees
Labels

Comments

@slvrtrn
Copy link
Contributor

slvrtrn commented Apr 26, 2023

Use case

Currently, while using MySQL protocol, SHOW COLUMNS or SELECT ... FROM system.columns returns ClickHouse types instead of MySQL, and this breaks the type introspection for BI tools.

For example:

SHOW FULL COLUMNS FROM `cell_towers` FROM `default` LIKE '%';

+---------------+-----------------------------------------------------------------------+------+---------+---------+-------+
| field         | type                                                                  | null | key     | default | extra |
+---------------+-----------------------------------------------------------------------+------+---------+---------+-------+
| area          | UInt16                                                                |    0 |         | NULL    |       |
| averageSignal | UInt8                                                                 |    0 |         | NULL    |       |
| cell          | UInt64                                                                |    0 |         | NULL    |       |
| changeable    | UInt8                                                                 |    0 |         | NULL    |       |
| created       | DateTime                                                              |    0 | PRI SOR | NULL    |       |
| lat           | Float64                                                               |    0 |         | NULL    |       |
| lon           | Float64                                                               |    0 |         | NULL    |       |
| mcc           | UInt16                                                                |    0 | PRI SOR | NULL    |       |
| net           | UInt16                                                                |    0 | PRI SOR | NULL    |       |
| radio         | Enum8('' = 0, 'CDMA' = 1, 'GSM' = 2, 'LTE' = 3, 'NR' = 4, 'UMTS' = 5) |    0 | PRI SOR | NULL    |       |
| range         | UInt32                                                                |    0 |         | NULL    |       |
| samples       | UInt32                                                                |    0 |         | NULL    |       |
| unit          | Int16                                                                 |    0 |         | NULL    |       |
| updated       | DateTime                                                              |    0 |         | NULL    |       |
+---------------+-----------------------------------------------------------------------+------+---------+---------+-------+

Describe the solution you'd like

Alias ClickHouse type to MySQL native types via MySQL protocol, i.e., Boolean = TINYINT, String = VARCHAR, etc. The preferred way is to have it enabled by default via MySQL protocol, but a server configuration setting will do if it's not possible.

Additional context

This is the main problem with QuickSight when accessing ClickHouse via MySQL protocol.

For example, the cell_towers data types introspection failed; all the fields except DateTime ones (created and updated, cause MySQL has DateTime type natively) were marked as OTHER and excluded from the dataset.

image

CC @rschu1ze

@tpanetti
Copy link
Contributor

tpanetti commented May 3, 2023

After some initial investigation on this, it looks like we can easily alias these types for SHOW COLUMNS by editing src/Interpreters/InterpreterShowColumnsQuery.cpp to give the alias types in MySQL compatibility mode. However this would not change the types for the query SELECT ... FROM system.columns nor any other SQL command that would give the types.

Should we assume that any and all SQL commands that give types in MySQL compatibility mode should return MySQL types? If so we do have a complete list of commands we would need to cover?

Otherwise would it be sufficient to make this change just for the SHOW COLUMNS command?

@tpanetti
Copy link
Contributor

tpanetti commented May 4, 2023

Attempted the first implementation and hooked up to QuickSights to test. Looks like types are possibly being cast correctly for quicksights (Not sure how latitude and longitude are being parsed correctly when these are doubles), attached screenshot below
image

@slvrtrn
Copy link
Contributor Author

slvrtrn commented May 4, 2023

Otherwise, would it be sufficient to make this change just for the SHOW COLUMNS command?

IIRC all BI tools I tested with MySQL protocol used SHOW FULL COLUMNS

@alexey-milovidov
Copy link
Member

I think it makes sense to add a method in IDataType, something like getSQLCompatibleName.

@rschu1ze
Copy link
Member

rschu1ze commented May 6, 2023

Otherwise would it be sufficient to make this change just for the SHOW COLUMNS command?

Yes, I think it is best to list type aliases only in SHOW COLUMNS but not in system.columns. The reason is that 1. the former is heavily inspired by MySQL (based on a quick check, MySQL and Postgres seem the only DBMSs which implement this syntax) and 2. MySQL is "just" one out of many DBMSs (a popular one though). I don't think we should give it a particular preference over other DBMS in system.tables - the system view ideally only shows ClickHouse's native types.

If so we do have a complete list of commands we would need to cover?

As far as I see, only SHOW COLUMNS is affected.

@alexey-milovidov
Copy link
Member

@rschu1ze, it's ok to add an additional column to system.columns, displaying SQL-compatible type name.
Then use it for SHOW COLUMNS.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants