From b9f5a2f6205affcf4b558559a5781c68a27bbad9 Mon Sep 17 00:00:00 2001 From: Robert Schulze Date: Fri, 23 Feb 2024 17:42:54 +0000 Subject: [PATCH 1/4] Enable prefer_column_name_to_alias in MySQL Handler, default enable two MySQL settings --- docs/en/operations/settings/settings.md | 4 ++-- src/Core/Settings.h | 4 ++-- src/Server/MySQLHandler.cpp | 6 ++++++ ...968_mysql_prefer_column_name_to_alias.reference | 2 ++ .../02968_mysql_prefer_column_name_to_alias.sh | 14 ++++++++++++++ 5 files changed, 26 insertions(+), 4 deletions(-) create mode 100644 tests/queries/0_stateless/02968_mysql_prefer_column_name_to_alias.reference create mode 100755 tests/queries/0_stateless/02968_mysql_prefer_column_name_to_alias.sh diff --git a/docs/en/operations/settings/settings.md b/docs/en/operations/settings/settings.md index b11a04e10ecf..7dd83a0b786f 100644 --- a/docs/en/operations/settings/settings.md +++ b/docs/en/operations/settings/settings.md @@ -3445,7 +3445,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} @@ -3456,7 +3456,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} diff --git a/src/Core/Settings.h b/src/Core/Settings.h index 433195af9c33..189bddc38931 100644 --- a/src/Core/Settings.h +++ b/src/Core/Settings.h @@ -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.", 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.", 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) \ diff --git a/src/Server/MySQLHandler.cpp b/src/Server/MySQLHandler.cpp index 72fe3b7cea9f..9efcebfc72d7 100644 --- a/src/Server/MySQLHandler.cpp +++ b/src/Server/MySQLHandler.cpp @@ -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 affected_rows {0}; diff --git a/tests/queries/0_stateless/02968_mysql_prefer_column_name_to_alias.reference b/tests/queries/0_stateless/02968_mysql_prefer_column_name_to_alias.reference new file mode 100644 index 000000000000..2491b55b493e --- /dev/null +++ b/tests/queries/0_stateless/02968_mysql_prefer_column_name_to_alias.reference @@ -0,0 +1,2 @@ +b count() +2 1 diff --git a/tests/queries/0_stateless/02968_mysql_prefer_column_name_to_alias.sh b/tests/queries/0_stateless/02968_mysql_prefer_column_name_to_alias.sh new file mode 100755 index 000000000000..4457aafb8b25 --- /dev/null +++ b/tests/queries/0_stateless/02968_mysql_prefer_column_name_to_alias.sh @@ -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"; From 13dd5ce427c406460bcd19eb7c02ce7bc456933d Mon Sep 17 00:00:00 2001 From: Robert Schulze Date: Mon, 26 Feb 2024 09:18:00 +0000 Subject: [PATCH 2/4] Update settings history --- src/Core/Settings.h | 4 ++-- src/Core/SettingsChangesHistory.h | 2 ++ 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/Core/Settings.h b/src/Core/Settings.h index 189bddc38931..2f8fc8ad34d3 100644 --- a/src/Core/Settings.h +++ b/src/Core/Settings.h @@ -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, true, "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, true, "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) \ diff --git a/src/Core/SettingsChangesHistory.h b/src/Core/SettingsChangesHistory.h index e97a411e2c1a..e58d35973aab 100644 --- a/src/Core/SettingsChangesHistory.h +++ b/src/Core/SettingsChangesHistory.h @@ -103,6 +103,8 @@ static std::map 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, "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."}, + {"mysql_map_fixed_string_to_text_in_show_columns", false, 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."}, }}, {"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"}, From b11c2667047c5c9ebff691de3578bcbf67275198 Mon Sep 17 00:00:00 2001 From: Robert Schulze Date: Mon, 26 Feb 2024 10:58:25 +0000 Subject: [PATCH 3/4] Update setting changes history --- src/Core/SettingsChangesHistory.h | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/Core/SettingsChangesHistory.h b/src/Core/SettingsChangesHistory.h index e58d35973aab..522578ef3031 100644 --- a/src/Core/SettingsChangesHistory.h +++ b/src/Core/SettingsChangesHistory.h @@ -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) @@ -103,8 +104,8 @@ static std::map 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, "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."}, - {"mysql_map_fixed_string_to_text_in_show_columns", false, 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."}, + {"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"}, From 1767a6d24492d693da759a103faad0444f219d5f Mon Sep 17 00:00:00 2001 From: Robert Schulze Date: Mon, 26 Feb 2024 14:47:26 +0000 Subject: [PATCH 4/4] Fix test results --- ...2775_show_columns_called_from_mysql.expect | 53 ++++++++++--------- 1 file changed, 27 insertions(+), 26 deletions(-) diff --git a/tests/queries/0_stateless/02775_show_columns_called_from_mysql.expect b/tests/queries/0_stateless/02775_show_columns_called_from_mysql.expect index 8ba5774820e3..3798acf2a935 100755 --- a/tests/queries/0_stateless/02775_show_columns_called_from_mysql.expect +++ b/tests/queries/0_stateless/02775_show_columns_called_from_mysql.expect @@ -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 | |" @@ -132,10 +132,10 @@ 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 | |" @@ -143,13 +143,13 @@ 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 | |" @@ -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 -- "+---------------+-------------------+------+------+---------+-------+" @@ -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 | |" @@ -198,10 +198,10 @@ 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 | |" @@ -209,13 +209,13 @@ 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 | |" @@ -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 -- "+---------------+-------------------+------+------+---------+-------+" @@ -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 | |" @@ -264,10 +264,10 @@ 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 | |" @@ -275,13 +275,13 @@ 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 | |" @@ -293,6 +293,7 @@ expect -- "| ui8 | TINYINT UNSIGNED | NO | | NULL | | expect -- "| uuid | CHAR | NO | | NULL | |" expect -- "+---------------+-------------------+------+------+---------+-------+" + send -- "DROP TABLE tab;" send -- "quit;\r"