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: Force-set prefer_column_name_to_alias in handler and set mysql_map_(fixed)_string_to_text_in_show_columns by default #60365

Merged
merged 5 commits into from Feb 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions docs/en/operations/settings/settings.md
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
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
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
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
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
@@ -0,0 +1,2 @@
b count()
2 1
@@ -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";