From 7bdb70723a954f396556ce60f80f8d5b99bc5197 Mon Sep 17 00:00:00 2001 From: robot-clickhouse Date: Fri, 9 Aug 2024 10:12:16 +0000 Subject: [PATCH] Backport #62206 to 24.3: Fix parsing booleans in settings --- src/Core/Field.h | 8 ++- src/Parsers/ParserSetQuery.cpp | 12 +--- .../test_distributed_config/test.py | 2 +- .../02494_query_cache_secrets.reference | 2 +- ...inguish_bool_and_int_in_settings.reference | 35 ++++++++++ ...1_distinguish_bool_and_int_in_settings.sql | 65 +++++++++++++++++++ .../03032_async_backup_restore.reference | 5 ++ .../0_stateless/03032_async_backup_restore.sh | 56 ++++++++++++++++ 8 files changed, 170 insertions(+), 15 deletions(-) create mode 100644 tests/queries/0_stateless/03031_distinguish_bool_and_int_in_settings.reference create mode 100644 tests/queries/0_stateless/03031_distinguish_bool_and_int_in_settings.sql create mode 100644 tests/queries/0_stateless/03032_async_backup_restore.reference create mode 100755 tests/queries/0_stateless/03032_async_backup_restore.sh diff --git a/src/Core/Field.h b/src/Core/Field.h index aed5fab2106..c3689d8cff1 100644 --- a/src/Core/Field.h +++ b/src/Core/Field.h @@ -893,11 +893,13 @@ NearestFieldType> & Field::get() template auto & Field::safeGet() { - const Types::Which requested = TypeToEnum>>::value; + const Types::Which target = TypeToEnum>>::value; - if (which != requested) + /// We allow converting int64 <-> uint64, int64 <-> bool, uint64 <-> bool in safeGet(). + if (target != which + && (!isInt64OrUInt64orBoolFieldType(target) || !isInt64OrUInt64orBoolFieldType(which))) throw Exception(ErrorCodes::BAD_GET, - "Bad get: has {}, requested {}", getTypeName(), requested); + "Bad get: has {}, requested {}", getTypeName(), target); return get(); } diff --git a/src/Parsers/ParserSetQuery.cpp b/src/Parsers/ParserSetQuery.cpp index 13b881635cd..f08d2b978c6 100644 --- a/src/Parsers/ParserSetQuery.cpp +++ b/src/Parsers/ParserSetQuery.cpp @@ -210,12 +210,8 @@ bool ParserSetQuery::parseNameValuePair(SettingChange & change, IParser::Pos & p if (!s_eq.ignore(pos, expected)) return false; - if (ParserKeyword(Keyword::TRUE_KEYWORD).ignore(pos, expected)) - value = std::make_shared(Field(static_cast(1))); - else if (ParserKeyword(Keyword::FALSE_KEYWORD).ignore(pos, expected)) - value = std::make_shared(Field(static_cast(0))); /// for SETTINGS disk=disk(type='s3', path='', ...) - else if (function_p.parse(pos, function_ast, expected) && function_ast->as()->name == "disk") + if (function_p.parse(pos, function_ast, expected) && function_ast->as()->name == "disk") { tryGetIdentifierNameInto(name, change.name); change.value = createFieldFromAST(function_ast); @@ -276,11 +272,7 @@ bool ParserSetQuery::parseNameValuePairWithParameterOrDefault( } /// Setting - if (ParserKeyword(Keyword::TRUE_KEYWORD).ignore(pos, expected)) - node = std::make_shared(Field(static_cast(1))); - else if (ParserKeyword(Keyword::FALSE_KEYWORD).ignore(pos, expected)) - node = std::make_shared(Field(static_cast(0))); - else if (function_p.parse(pos, function_ast, expected) && function_ast->as()->name == "disk") + if (function_p.parse(pos, function_ast, expected) && function_ast->as()->name == "disk") { change.name = name; change.value = createFieldFromAST(function_ast); diff --git a/tests/integration/test_distributed_config/test.py b/tests/integration/test_distributed_config/test.py index bf4bb5a4335..e551e69b93f 100644 --- a/tests/integration/test_distributed_config/test.py +++ b/tests/integration/test_distributed_config/test.py @@ -31,7 +31,7 @@ def test_distibuted_settings(start_cluster): DETACH TABLE dist_1; """ ) - assert "flush_on_detach = 1" in node.query("SHOW CREATE dist_1") + assert "flush_on_detach = true" in node.query("SHOW CREATE dist_1") # flush_on_detach=true, so data_1 should have 1 row assert int(node.query("SELECT count() FROM data_1")) == 1 diff --git a/tests/queries/0_stateless/02494_query_cache_secrets.reference b/tests/queries/0_stateless/02494_query_cache_secrets.reference index 306374eed4b..82833f28369 100644 --- a/tests/queries/0_stateless/02494_query_cache_secrets.reference +++ b/tests/queries/0_stateless/02494_query_cache_secrets.reference @@ -1,2 +1,2 @@ A2193552DCF8A9F99AC35F86BC4D2FFD -SELECT hex(encrypt(\'aes-128-ecb\', \'[HIDDEN]\')) SETTINGS use_query_cache = 1 +SELECT hex(encrypt(\'aes-128-ecb\', \'[HIDDEN]\')) SETTINGS use_query_cache = true diff --git a/tests/queries/0_stateless/03031_distinguish_bool_and_int_in_settings.reference b/tests/queries/0_stateless/03031_distinguish_bool_and_int_in_settings.reference new file mode 100644 index 00000000000..fc1a2052b68 --- /dev/null +++ b/tests/queries/0_stateless/03031_distinguish_bool_and_int_in_settings.reference @@ -0,0 +1,35 @@ +-- Custom settings from system.settings +custom_f1 Bool_0 Custom +custom_f2 Bool_0 Custom +custom_f3 Bool_0 Custom +custom_n0 UInt64_0 Custom +custom_n1 UInt64_1 Custom +custom_t1 Bool_1 Custom +custom_t2 Bool_1 Custom +custom_t3 Bool_1 Custom +-- Custom settings via getSetting() +custom_f1 false Bool +custom_f2 false Bool +custom_f3 false Bool +custom_n0 0 UInt8 +custom_n1 1 UInt8 +custom_t1 true Bool +custom_t2 true Bool +custom_t3 true Bool +-- Built-in settings +async_insert 0 Bool +async_insert false Bool +async_insert 0 Bool +async_insert false Bool +async_insert 0 Bool +async_insert false Bool +async_insert 0 Bool +async_insert false Bool +async_insert 1 Bool +async_insert true Bool +async_insert 1 Bool +async_insert true Bool +async_insert 1 Bool +async_insert true Bool +async_insert 1 Bool +async_insert true Bool diff --git a/tests/queries/0_stateless/03031_distinguish_bool_and_int_in_settings.sql b/tests/queries/0_stateless/03031_distinguish_bool_and_int_in_settings.sql new file mode 100644 index 00000000000..33be34a40a9 --- /dev/null +++ b/tests/queries/0_stateless/03031_distinguish_bool_and_int_in_settings.sql @@ -0,0 +1,65 @@ +-- Custom settings must remember their types - whether it's a boolean or an integer. + +-- Different ways to set a boolean. +SET custom_f1 = false; +SET custom_f2 = False; +SET custom_f3 = FALSE; + +SET custom_n0 = 0; +SET custom_n1 = 1; + +SET custom_t1 = true; +SET custom_t2 = True; +SET custom_t3 = TRUE; + +SELECT '-- Custom settings from system.settings'; + +SELECT name, value, type FROM system.settings WHERE startsWith(name, 'custom_') ORDER BY name; + +SELECT '-- Custom settings via getSetting()'; + +SELECT 'custom_f1' AS name, getSetting(name) AS value, toTypeName(value); +SELECT 'custom_f2' AS name, getSetting(name) AS value, toTypeName(value); +SELECT 'custom_f3' AS name, getSetting(name) AS value, toTypeName(value); + +SELECT 'custom_n0' AS name, getSetting(name) AS value, toTypeName(value); +SELECT 'custom_n1' AS name, getSetting(name) AS value, toTypeName(value); + +SELECT 'custom_t1' AS name, getSetting(name) AS value, toTypeName(value); +SELECT 'custom_t2' AS name, getSetting(name) AS value, toTypeName(value); +SELECT 'custom_t3' AS name, getSetting(name) AS value, toTypeName(value); + +-- Built-in settings have hardcoded types. +SELECT '-- Built-in settings'; + +SET async_insert = false; +SELECT name, value, type FROM system.settings WHERE name = 'async_insert'; +SELECT 'async_insert' AS name, getSetting(name) AS value, toTypeName(value); + +SET async_insert = False; +SELECT name, value, type FROM system.settings WHERE name = 'async_insert'; +SELECT 'async_insert' AS name, getSetting(name) AS value, toTypeName(value); + +SET async_insert = FALSE; +SELECT name, value, type FROM system.settings WHERE name = 'async_insert'; +SELECT 'async_insert' AS name, getSetting(name) AS value, toTypeName(value); + +SET async_insert = 0; +SELECT name, value, type FROM system.settings WHERE name = 'async_insert'; +SELECT 'async_insert' AS name, getSetting(name) AS value, toTypeName(value); + +SET async_insert = 1; +SELECT name, value, type FROM system.settings WHERE name = 'async_insert'; +SELECT 'async_insert' AS name, getSetting(name) AS value, toTypeName(value); + +SET async_insert = true; +SELECT name, value, type FROM system.settings WHERE name = 'async_insert'; +SELECT 'async_insert' AS name, getSetting(name) AS value, toTypeName(value); + +SET async_insert = True; +SELECT name, value, type FROM system.settings WHERE name = 'async_insert'; +SELECT 'async_insert' AS name, getSetting(name) AS value, toTypeName(value); + +SET async_insert = TRUE; +SELECT name, value, type FROM system.settings WHERE name = 'async_insert'; +SELECT 'async_insert' AS name, getSetting(name) AS value, toTypeName(value); diff --git a/tests/queries/0_stateless/03032_async_backup_restore.reference b/tests/queries/0_stateless/03032_async_backup_restore.reference new file mode 100644 index 00000000000..de99716769b --- /dev/null +++ b/tests/queries/0_stateless/03032_async_backup_restore.reference @@ -0,0 +1,5 @@ +BACKUP_CREATED +RESTORED +2 +80 +-12345 diff --git a/tests/queries/0_stateless/03032_async_backup_restore.sh b/tests/queries/0_stateless/03032_async_backup_restore.sh new file mode 100755 index 00000000000..81fe12bb0f1 --- /dev/null +++ b/tests/queries/0_stateless/03032_async_backup_restore.sh @@ -0,0 +1,56 @@ +#!/usr/bin/env bash + +CUR_DIR=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd) +# shellcheck source=../shell_config.sh +. "$CUR_DIR"/../shell_config.sh + +${CLICKHOUSE_CLIENT} -nm --query " +DROP TABLE IF EXISTS tbl; +DROP TABLE IF EXISTS tbl2; +CREATE TABLE tbl (a Int32) ENGINE = MergeTree() ORDER BY tuple(); +INSERT INTO tbl VALUES (2), (80), (-12345); +" + +function start_async() +{ + local command="$1" + local first_column="s/^\([^\t]*\)\t.*/\1/" + echo $(${CLICKHOUSE_CLIENT} --query "$command" | sed "${first_column}") +} + +function wait_status() +{ + local operation_id="$1" + local expected_status="$2" + local timeout=60 + local start=$EPOCHSECONDS + while true; do + local current_status=$(${CLICKHOUSE_CLIENT} --query "SELECT status FROM system.backups WHERE id='${operation_id}'") + if [ "${current_status}" == "${expected_status}" ]; then + echo "${current_status}" + break + fi + if ((EPOCHSECONDS-start > timeout )); then + echo "Timeout while waiting for operation ${operation_id} to come to status ${expected_status}. The current status is ${current_status}." + exit 1 + fi + sleep 0.1 + done +} + +# Making a backup. +backup_name="Disk('backups', '${CLICKHOUSE_TEST_UNIQUE_NAME}')" +backup_operation_id=$(start_async "BACKUP TABLE tbl TO ${backup_name} ASYNC") +wait_status ${backup_operation_id} "BACKUP_CREATED" + +# Restoring from that backup. +restore_operation_id=$(start_async "RESTORE TABLE tbl AS tbl2 FROM ${backup_name} ASYNC") +wait_status ${restore_operation_id} "RESTORED" + +# Check the result of that restoration. +${CLICKHOUSE_CLIENT} --query "SELECT * FROM tbl2" + +${CLICKHOUSE_CLIENT} -nm --query " +DROP TABLE tbl; +DROP TABLE tbl2; +"