Skip to content

Commit

Permalink
Merge pull request #60365 from rschu1ze/mysql-friday-evening-fix
Browse files Browse the repository at this point in the history 
MySQL: Force-set `prefer_column_name_to_alias` in handler and set `mysql_map_(fixed)_string_to_text_in_show_columns` by default
  • Loading branch information
@rschu1ze
rschu1ze committed Feb 27, 2024
2 parents c8db540 + ac7a3cd commit de45c26
Show file tree
Hide file tree
Showing 7 changed files with 57 additions and 31 deletions.
4 changes: 2 additions & 2 deletions docs/en/operations/settings/settings.md
View file
Expand Up @@ -3449,7 +3449,7 @@ Has an effect only when the connection is made through the MySQL wire protocol.
- 0 - Use `BLOB`.
- 1 - Use `TEXT`.

Default value: `0`.
Default value: `1`.

## mysql_map_fixed_string_to_text_in_show_columns {#mysql_map_fixed_string_to_text_in_show_columns}

Expand All @@ -3460,7 +3460,7 @@ Has an effect only when the connection is made through the MySQL wire protocol.
- 0 - Use `BLOB`.
- 1 - Use `TEXT`.

Default value: `0`.
Default value: `1`.

## execute_merges_on_single_replica_time_threshold {#execute-merges-on-single-replica-time-threshold}

Expand Down
4 changes: 2 additions & 2 deletions src/Core/Settings.h
View file
Expand Up @@ -224,8 +224,8 @@ class IColumn;
M(Bool, allow_experimental_inverted_index, false, "If it is set to true, allow to use experimental inverted index.", 0) \
\
M(UInt64, mysql_max_rows_to_insert, 65536, "The maximum number of rows in MySQL batch insertion of the MySQL storage engine", 0) \
M(Bool, mysql_map_string_to_text_in_show_columns, false, "If enabled, String type will be mapped to TEXT in SHOW [FULL] COLUMNS, BLOB otherwise.", 0) \
M(Bool, mysql_map_fixed_string_to_text_in_show_columns, false, "If enabled, FixedString type will be mapped to TEXT in SHOW [FULL] COLUMNS, BLOB otherwise.", 0) \
M(Bool, mysql_map_string_to_text_in_show_columns, true, "If enabled, String type will be mapped to TEXT in SHOW [FULL] COLUMNS, BLOB otherwise. Has an effect only when the connection is made through the MySQL wire protocol.", 0) \
M(Bool, mysql_map_fixed_string_to_text_in_show_columns, true, "If enabled, FixedString type will be mapped to TEXT in SHOW [FULL] COLUMNS, BLOB otherwise. Has an effect only when the connection is made through the MySQL wire protocol.", 0) \
\
M(UInt64, optimize_min_equality_disjunction_chain_length, 3, "The minimum length of the expression `expr = x1 OR ... expr = xN` for optimization ", 0) \
M(UInt64, optimize_min_inequality_conjunction_chain_length, 3, "The minimum length of the expression `expr <> x1 AND ... expr <> xN` for optimization ", 0) \
Expand Down
5 changes: 4 additions & 1 deletion src/Core/SettingsChangesHistory.h
View file
Expand Up @@ -78,7 +78,8 @@ namespace SettingsChangesHistory
/// History of settings changes that controls some backward incompatible changes
/// across all ClickHouse versions. It maps ClickHouse version to settings changes that were done
/// in this version. This history contains both changes to existing settings and newly added settings.
/// Settings changes is a vector of structs {setting_name, previous_value, new_value}.
/// Settings changes is a vector of structs
/// {setting_name, previous_value, new_value, reason}.
/// For newly added setting choose the most appropriate previous_value (for example, if new setting
/// controls new feature and it's 'true' by default, use 'false' as previous_value).
/// It's used to implement `compatibility` setting (see https://github.com/ClickHouse/ClickHouse/issues/35972)
Expand All @@ -103,6 +104,8 @@ static std::map<ClickHouseVersion, SettingsChangesHistory::SettingsChanges> sett
{"min_external_table_block_size_bytes", DEFAULT_INSERT_BLOCK_SIZE * 256, DEFAULT_INSERT_BLOCK_SIZE * 256, "Squash blocks passed to external table to specified size in bytes, if blocks are not big enough."},
{"parallel_replicas_prefer_local_join", true, true, "If true, and JOIN can be executed with parallel replicas algorithm, and all storages of right JOIN part are *MergeTree, local JOIN will be used instead of GLOBAL JOIN."},
{"extract_key_value_pairs_max_pairs_per_row", 0, 0, "Max number of pairs that can be produced by the `extractKeyValuePairs` function. Used as a safeguard against consuming too much memory."},
{"mysql_map_string_to_text_in_show_columns", false, true, "Reduce the configuration effort to connect ClickHouse with BI tools."},
{"mysql_map_fixed_string_to_text_in_show_columns", false, true, "Reduce the configuration effort to connect ClickHouse with BI tools."},
}},
{"24.1", {{"print_pretty_type_names", false, true, "Better user experience."},
{"input_format_json_read_bools_as_strings", false, true, "Allow to read bools as strings in JSON formats by default"},
Expand Down
6 changes: 6 additions & 0 deletions src/Server/MySQLHandler.cpp
View file
Expand Up @@ -461,6 +461,12 @@ void MySQLHandler::comQuery(ReadBuffer & payload, bool binary_protocol)

auto query_context = session->makeQueryContext();
query_context->setCurrentQueryId(fmt::format("mysql:{}:{}", connection_id, toString(UUIDHelpers::generateV4())));

/// --- Workaround for Bug 56173. Can be removed when the analyzer is on by default.
auto settings = query_context->getSettings();
settings.prefer_column_name_to_alias = true;
query_context->setSettings(settings);

CurrentThread::QueryScope query_scope{query_context};

std::atomic<size_t> affected_rows {0};
Expand Down
53 changes: 27 additions & 26 deletions tests/queries/0_stateless/02775_show_columns_called_from_mysql.expect
View file
Expand Up @@ -123,7 +123,7 @@ expect -- "| dt_tz2 | DATETIME | NO | | NULL | |
expect -- "| enm | TEXT | NO | | NULL | |"
expect -- "| f32 | FLOAT | NO | | NULL | |"
expect -- "| f64 | DOUBLE | NO | | NULL | |"
expect -- "| fs | BLOB | NO | | NULL | |"
expect -- "| fs | TEXT | NO | | NULL | |"
expect -- "| i128 | TEXT | NO | | NULL | |"
expect -- "| i16 | SMALLINT | NO | | NULL | |"
expect -- "| i256 | TEXT | NO | | NULL | |"
Expand All @@ -132,24 +132,24 @@ expect -- "| i64 | BIGINT | NO | | NULL | |
expect -- "| i8 | TINYINT | NO | | NULL | |"
expect -- "| ip4 | TEXT | NO | | NULL | |"
expect -- "| ip6 | TEXT | NO | | NULL | |"
expect -- "| lfs | BLOB | NO | | NULL | |"
expect -- "| lnfs | BLOB | YES | | NULL | |"
expect -- "| lns | BLOB | YES | | NULL | |"
expect -- "| ls | BLOB | NO | | NULL | |"
expect -- "| lfs | TEXT | NO | | NULL | |"
expect -- "| lnfs | TEXT | YES | | NULL | |"
expect -- "| lns | TEXT | YES | | NULL | |"
expect -- "| ls | TEXT | NO | | NULL | |"
expect -- "| m | JSON | NO | | NULL | |"
expect -- "| m_complex | JSON | NO | | NULL | |"
expect -- "| mpg | TEXT | NO | | NULL | |"
expect -- "| ndt64 | DATETIME | YES | | NULL | |"
expect -- "| ndt64_tz | DATETIME | YES | | NULL | |"
expect -- "| nested.col1 | TEXT | NO | | NULL | |"
expect -- "| nested.col2 | TEXT | NO | | NULL | |"
expect -- "| nfs | BLOB | YES | | NULL | |"
expect -- "| ns | BLOB | YES | | NULL | |"
expect -- "| nfs | TEXT | YES | | NULL | |"
expect -- "| ns | TEXT | YES | | NULL | |"
expect -- "| o | JSON | NO | | NULL | |"
expect -- "| p | TEXT | NO | | NULL | |"
expect -- "| pg | TEXT | NO | | NULL | |"
expect -- "| r | TEXT | NO | | NULL | |"
expect -- "| s | BLOB | NO | | NULL | |"
expect -- "| s | TEXT | NO | | NULL | |"
expect -- "| sagg | TEXT | NO | | NULL | |"
expect -- "| t | JSON | NO | | NULL | |"
expect -- "| ui128 | TEXT | NO | | NULL | |"
Expand All @@ -161,7 +161,7 @@ expect -- "| ui8 | TINYINT UNSIGNED | NO | | NULL | |
expect -- "| uuid | CHAR | NO | | NULL | |"
expect -- "+---------------+-------------------+------+------+---------+-------+"

send -- "SHOW COLUMNS FROM tab SETTINGS mysql_map_string_to_text_in_show_columns=1;\r"
send -- "SHOW COLUMNS FROM tab SETTINGS mysql_map_string_to_text_in_show_columns=0;\r"
expect -- "+---------------+-------------------+------+------+---------+-------+"
expect -- "| field | type | null | key | default | extra |"
expect -- "+---------------+-------------------+------+------+---------+-------+"
Expand Down Expand Up @@ -189,7 +189,7 @@ expect -- "| dt_tz2 | DATETIME | NO | | NULL | |
expect -- "| enm | TEXT | NO | | NULL | |"
expect -- "| f32 | FLOAT | NO | | NULL | |"
expect -- "| f64 | DOUBLE | NO | | NULL | |"
expect -- "| fs | BLOB | NO | | NULL | |"
expect -- "| fs | TEXT | NO | | NULL | |"
expect -- "| i128 | TEXT | NO | | NULL | |"
expect -- "| i16 | SMALLINT | NO | | NULL | |"
expect -- "| i256 | TEXT | NO | | NULL | |"
Expand All @@ -198,24 +198,24 @@ expect -- "| i64 | BIGINT | NO | | NULL | |
expect -- "| i8 | TINYINT | NO | | NULL | |"
expect -- "| ip4 | TEXT | NO | | NULL | |"
expect -- "| ip6 | TEXT | NO | | NULL | |"
expect -- "| lfs | BLOB | NO | | NULL | |"
expect -- "| lnfs | BLOB | YES | | NULL | |"
expect -- "| lns | TEXT | YES | | NULL | |"
expect -- "| ls | TEXT | NO | | NULL | |"
expect -- "| lfs | TEXT | NO | | NULL | |"
expect -- "| lnfs | TEXT | YES | | NULL | |"
expect -- "| lns | BLOB | YES | | NULL | |"
expect -- "| ls | BLOB | NO | | NULL | |"
expect -- "| m | JSON | NO | | NULL | |"
expect -- "| m_complex | JSON | NO | | NULL | |"
expect -- "| mpg | TEXT | NO | | NULL | |"
expect -- "| ndt64 | DATETIME | YES | | NULL | |"
expect -- "| ndt64_tz | DATETIME | YES | | NULL | |"
expect -- "| nested.col1 | TEXT | NO | | NULL | |"
expect -- "| nested.col2 | TEXT | NO | | NULL | |"
expect -- "| nfs | BLOB | YES | | NULL | |"
expect -- "| ns | TEXT | YES | | NULL | |"
expect -- "| nfs | TEXT | YES | | NULL | |"
expect -- "| ns | BLOB | YES | | NULL | |"
expect -- "| o | JSON | NO | | NULL | |"
expect -- "| p | TEXT | NO | | NULL | |"
expect -- "| pg | TEXT | NO | | NULL | |"
expect -- "| r | TEXT | NO | | NULL | |"
expect -- "| s | TEXT | NO | | NULL | |"
expect -- "| s | BLOB | NO | | NULL | |"
expect -- "| sagg | TEXT | NO | | NULL | |"
expect -- "| t | JSON | NO | | NULL | |"
expect -- "| ui128 | TEXT | NO | | NULL | |"
Expand All @@ -227,7 +227,7 @@ expect -- "| ui8 | TINYINT UNSIGNED | NO | | NULL | |
expect -- "| uuid | CHAR | NO | | NULL | |"
expect -- "+---------------+-------------------+------+------+---------+-------+"

send -- "SHOW COLUMNS FROM tab SETTINGS mysql_map_fixed_string_to_text_in_show_columns=1;\r"
send -- "SHOW COLUMNS FROM tab SETTINGS mysql_map_fixed_string_to_text_in_show_columns=0;\r"
expect -- "+---------------+-------------------+------+------+---------+-------+"
expect -- "| field | type | null | key | default | extra |"
expect -- "+---------------+-------------------+------+------+---------+-------+"
Expand Down Expand Up @@ -255,7 +255,7 @@ expect -- "| dt_tz2 | DATETIME | NO | | NULL | |
expect -- "| enm | TEXT | NO | | NULL | |"
expect -- "| f32 | FLOAT | NO | | NULL | |"
expect -- "| f64 | DOUBLE | NO | | NULL | |"
expect -- "| fs | TEXT | NO | | NULL | |"
expect -- "| fs | BLOB | NO | | NULL | |"
expect -- "| i128 | TEXT | NO | | NULL | |"
expect -- "| i16 | SMALLINT | NO | | NULL | |"
expect -- "| i256 | TEXT | NO | | NULL | |"
Expand All @@ -264,24 +264,24 @@ expect -- "| i64 | BIGINT | NO | | NULL | |
expect -- "| i8 | TINYINT | NO | | NULL | |"
expect -- "| ip4 | TEXT | NO | | NULL | |"
expect -- "| ip6 | TEXT | NO | | NULL | |"
expect -- "| lfs | TEXT | NO | | NULL | |"
expect -- "| lnfs | TEXT | YES | | NULL | |"
expect -- "| lns | BLOB | YES | | NULL | |"
expect -- "| ls | BLOB | NO | | NULL | |"
expect -- "| lfs | BLOB | NO | | NULL | |"
expect -- "| lnfs | BLOB | YES | | NULL | |"
expect -- "| lns | TEXT | YES | | NULL | |"
expect -- "| ls | TEXT | NO | | NULL | |"
expect -- "| m | JSON | NO | | NULL | |"
expect -- "| m_complex | JSON | NO | | NULL | |"
expect -- "| mpg | TEXT | NO | | NULL | |"
expect -- "| ndt64 | DATETIME | YES | | NULL | |"
expect -- "| ndt64_tz | DATETIME | YES | | NULL | |"
expect -- "| nested.col1 | TEXT | NO | | NULL | |"
expect -- "| nested.col2 | TEXT | NO | | NULL | |"
expect -- "| nfs | TEXT | YES | | NULL | |"
expect -- "| ns | BLOB | YES | | NULL | |"
expect -- "| nfs | BLOB | YES | | NULL | |"
expect -- "| ns | TEXT | YES | | NULL | |"
expect -- "| o | JSON | NO | | NULL | |"
expect -- "| p | TEXT | NO | | NULL | |"
expect -- "| pg | TEXT | NO | | NULL | |"
expect -- "| r | TEXT | NO | | NULL | |"
expect -- "| s | BLOB | NO | | NULL | |"
expect -- "| s | TEXT | NO | | NULL | |"
expect -- "| sagg | TEXT | NO | | NULL | |"
expect -- "| t | JSON | NO | | NULL | |"
expect -- "| ui128 | TEXT | NO | | NULL | |"
Expand All @@ -293,6 +293,7 @@ expect -- "| ui8 | TINYINT UNSIGNED | NO | | NULL | |
expect -- "| uuid | CHAR | NO | | NULL | |"
expect -- "+---------------+-------------------+------+------+---------+-------+"


send -- "DROP TABLE tab;"

send -- "quit;\r"
Expand Down
2 changes: 2 additions & 0 deletions tests/queries/0_stateless/02968_mysql_prefer_column_name_to_alias.reference
View file
@@ -0,0 +1,2 @@
b count()
2 1
14 changes: 14 additions & 0 deletions tests/queries/0_stateless/02968_mysql_prefer_column_name_to_alias.sh
View file
@@ -0,0 +1,14 @@
#!/usr/bin/env bash
# Tags: no-fasttest
# Tag no-fasttest: requires mysql client

CURDIR=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)
# shellcheck source=../shell_config.sh
. "$CURDIR"/../shell_config.sh

# Some BI tools which connect to ClickHouse's MySQL port, run queries which succeed only with (the analyzer enabled)
# or (without analyzer and setting prefer_column_name_to_alias = 1). Since the setting is too impactful to enable it
# globally, it is enabled only by the MySQL handler internally as a workaround. Run a query from Bug 56173 to verify.
#
# When the analyzer is the new default, the test and the workaround can be deleted.
${MYSQL_CLIENT} --execute "select a + b as b, count() from (select 1 as a, 1 as b) group by a + b";

0 comments on commit de45c26

Please sign in to comment.