From f52870c8d2afd19a58bfb31d18c2bc0e34241a5d Mon Sep 17 00:00:00 2001 From: Nikita Taranov Date: Mon, 29 Apr 2024 21:34:23 +0100 Subject: [PATCH] impl --- .../functions/other-functions.md | 7 +++++++ src/Common/ErrorCodes.cpp | 1 + src/Core/Settings.h | 1 + src/Core/SettingsChangesHistory.h | 2 ++ src/Databases/DatabaseReplicated.cpp | 1 + src/Functions/neighbor.cpp | 15 ++++++++++++- src/Functions/runningAccumulate.cpp | 12 ++++++++++- src/Functions/runningDifference.h | 21 +++++++++++++------ .../03131_deprecated_functions.reference | 0 .../03131_deprecated_functions.sql | 13 ++++++++++++ 10 files changed, 65 insertions(+), 8 deletions(-) create mode 100644 tests/queries/0_stateless/03131_deprecated_functions.reference create mode 100644 tests/queries/0_stateless/03131_deprecated_functions.sql diff --git a/docs/en/sql-reference/functions/other-functions.md b/docs/en/sql-reference/functions/other-functions.md index 26351301a3b..7ee9d0366e9 100644 --- a/docs/en/sql-reference/functions/other-functions.md +++ b/docs/en/sql-reference/functions/other-functions.md @@ -1024,6 +1024,7 @@ The result of the function depends on the affected data blocks and the order of :::note Only returns neighbor inside the currently processed data block. +Because of this error-prone behavior the function is DEPRECATED. ::: The order of rows during calculation of `neighbor()` can differ from the order of rows returned to the user. @@ -1134,6 +1135,7 @@ Returns 0 for the first row, and for subsequent rows the difference to the previ :::note Only returns differences inside the currently processed data block. +Because of this error-prone behavior the function is DEPRECATED. ::: The result of the function depends on the affected data blocks and the order of data in the block. @@ -1207,6 +1209,10 @@ WHERE diff != 1 ## runningDifferenceStartingWithFirstValue +:::note +This function is DEPRECATED (see the note for `runningDifference`). +::: + Same as [runningDifference](./other-functions.md#other_functions-runningdifference), but returns the value of the first row as the value on the first row. ## runningConcurrency @@ -1930,6 +1936,7 @@ Accumulates the states of an aggregate function for each row of a data block. :::note The state is reset for each new block of data. +Because of this error-prone behavior the function is DEPRECATED. ::: **Syntax** diff --git a/src/Common/ErrorCodes.cpp b/src/Common/ErrorCodes.cpp index 97a339b2bac..33ccc1e41bf 100644 --- a/src/Common/ErrorCodes.cpp +++ b/src/Common/ErrorCodes.cpp @@ -598,6 +598,7 @@ M(717, EXPERIMENTAL_FEATURE_ERROR) \ M(718, TOO_SLOW_PARSING) \ M(719, QUERY_CACHE_USED_WITH_SYSTEM_TABLE) \ + M(720, DEPRECATED_FUNCTION) \ \ M(900, DISTRIBUTED_CACHE_ERROR) \ M(901, CANNOT_USE_DISTRIBUTED_CACHE) \ diff --git a/src/Core/Settings.h b/src/Core/Settings.h index 375bdb1c516..7b1a04c9b95 100644 --- a/src/Core/Settings.h +++ b/src/Core/Settings.h @@ -910,6 +910,7 @@ class IColumn; M(Int64, ignore_cold_parts_seconds, 0, "Only available in ClickHouse Cloud. Exclude new data parts from SELECT queries until they're either pre-warmed (see cache_populated_by_fetch) or this many seconds old. Only for Replicated-/SharedMergeTree.", 0) \ M(Int64, prefer_warmed_unmerged_parts_seconds, 0, "Only available in ClickHouse Cloud. If a merged part is less than this many seconds old and is not pre-warmed (see cache_populated_by_fetch), but all its source parts are available and pre-warmed, SELECT queries will read from those parts instead. Only for ReplicatedMergeTree. Note that this only checks whether CacheWarmer processed the part; if the part was fetched into cache by something else, it'll still be considered cold until CacheWarmer gets to it; if it was warmed, then evicted from cache, it'll still be considered warm.", 0) \ M(Bool, iceberg_engine_ignore_schema_evolution, false, "Ignore schema evolution in Iceberg table engine and read all data using latest schema saved on table creation. Note that it can lead to incorrect result", 0) \ + M(Bool, allow_deprecated_functions, false, "Allow usage of deprecated functions", 0) \ // End of COMMON_SETTINGS // Please add settings related to formats into the FORMAT_FACTORY_SETTINGS, move obsolete settings to OBSOLETE_SETTINGS and obsolete format settings to OBSOLETE_FORMAT_SETTINGS. diff --git a/src/Core/SettingsChangesHistory.h b/src/Core/SettingsChangesHistory.h index d3b5de06e70..35b81c50aea 100644 --- a/src/Core/SettingsChangesHistory.h +++ b/src/Core/SettingsChangesHistory.h @@ -85,6 +85,8 @@ namespace SettingsChangesHistory /// It's used to implement `compatibility` setting (see https://github.com/ClickHouse/ClickHouse/issues/35972) static std::map settings_changes_history = { + {"24.5", {{"allow_deprecated_functions", true, false, "Allow usage of deprecated functions"}, + }}, {"24.4", {{"input_format_json_throw_on_bad_escape_sequence", true, true, "Allow to save JSON strings with bad escape sequences"}, {"ignore_drop_queries_probability", 0, 0, "Allow to ignore drop queries in server with specified probability for testing purposes"}, {"lightweight_deletes_sync", 2, 2, "The same as 'mutation_sync', but controls only execution of lightweight deletes"}, diff --git a/src/Databases/DatabaseReplicated.cpp b/src/Databases/DatabaseReplicated.cpp index 57dbcad565f..01da242b24e 100644 --- a/src/Databases/DatabaseReplicated.cpp +++ b/src/Databases/DatabaseReplicated.cpp @@ -935,6 +935,7 @@ void DatabaseReplicated::recoverLostReplica(const ZooKeeperPtr & current_zookeep query_context->setSetting("allow_experimental_window_functions", 1); query_context->setSetting("allow_experimental_geo_types", 1); query_context->setSetting("allow_experimental_map_type", 1); + query_context->setSetting("allow_deprecated_functions", 1); query_context->setSetting("allow_suspicious_low_cardinality_types", 1); query_context->setSetting("allow_suspicious_fixed_string_types", 1); diff --git a/src/Functions/neighbor.cpp b/src/Functions/neighbor.cpp index d367695448a..49b73aabe3d 100644 --- a/src/Functions/neighbor.cpp +++ b/src/Functions/neighbor.cpp @@ -4,6 +4,7 @@ #include #include #include +#include #include namespace DB @@ -13,6 +14,7 @@ namespace ErrorCodes extern const int ILLEGAL_TYPE_OF_ARGUMENT; extern const int NUMBER_OF_ARGUMENTS_DOESNT_MATCH; extern const int ARGUMENT_OUT_OF_BOUND; + extern const int DEPRECATED_FUNCTION; } namespace @@ -31,7 +33,18 @@ class FunctionNeighbor : public IFunction { public: static constexpr auto name = "neighbor"; - static FunctionPtr create(ContextPtr) { return std::make_shared(); } + + static FunctionPtr create(ContextPtr context) + { + if (!context->getSettingsRef().allow_deprecated_functions) + throw Exception( + ErrorCodes::DEPRECATED_FUNCTION, + "Function {} is deprecated since its usage is error-prone (see docs)." + "Set `allow_deprecated_functions` setting to enable it", + name); + + return std::make_shared(); + } /// Get the name of the function. String getName() const override { return name; } diff --git a/src/Functions/runningAccumulate.cpp b/src/Functions/runningAccumulate.cpp index 793e79cdf46..b2bc1ea02b9 100644 --- a/src/Functions/runningAccumulate.cpp +++ b/src/Functions/runningAccumulate.cpp @@ -4,6 +4,7 @@ #include #include #include +#include #include #include #include @@ -16,6 +17,7 @@ namespace ErrorCodes extern const int ILLEGAL_COLUMN; extern const int ILLEGAL_TYPE_OF_ARGUMENT; extern const int NUMBER_OF_ARGUMENTS_DOESNT_MATCH; + extern const int DEPRECATED_FUNCTION; } namespace @@ -34,8 +36,16 @@ class FunctionRunningAccumulate : public IFunction { public: static constexpr auto name = "runningAccumulate"; - static FunctionPtr create(ContextPtr) + + static FunctionPtr create(ContextPtr context) { + if (!context->getSettingsRef().allow_deprecated_functions) + throw Exception( + ErrorCodes::DEPRECATED_FUNCTION, + "Function {} is deprecated since its usage is error-prone (see docs)." + "Set `allow_deprecated_functions` setting to enable it", + name); + return std::make_shared(); } diff --git a/src/Functions/runningDifference.h b/src/Functions/runningDifference.h index f1ec4f9e523..ea1408b4c08 100644 --- a/src/Functions/runningDifference.h +++ b/src/Functions/runningDifference.h @@ -1,16 +1,17 @@ #pragma once -#include -#include -#include #include -#include +#include #include #include #include #include +#include #include #include -#include +#include +#include +#include +#include namespace DB @@ -19,6 +20,7 @@ namespace DB namespace ErrorCodes { extern const int ILLEGAL_TYPE_OF_ARGUMENT; + extern const int DEPRECATED_FUNCTION; } @@ -135,8 +137,15 @@ private: public: static constexpr auto name = FunctionRunningDifferenceName::name; - static FunctionPtr create(ContextPtr) + static FunctionPtr create(ContextPtr context) { + if (!context->getSettingsRef().allow_deprecated_functions) + throw Exception( + ErrorCodes::DEPRECATED_FUNCTION, + "Function {} is deprecated since its usage is error-prone (see docs)." + "Set `allow_deprecated_functions` setting to enable it", + name); + return std::make_shared>(); } diff --git a/tests/queries/0_stateless/03131_deprecated_functions.reference b/tests/queries/0_stateless/03131_deprecated_functions.reference new file mode 100644 index 00000000000..e69de29bb2d diff --git a/tests/queries/0_stateless/03131_deprecated_functions.sql b/tests/queries/0_stateless/03131_deprecated_functions.sql new file mode 100644 index 00000000000..4cd7c0f473f --- /dev/null +++ b/tests/queries/0_stateless/03131_deprecated_functions.sql @@ -0,0 +1,13 @@ +SELECT number, neighbor(number, 2) FROM system.numbers LIMIT 10; -- { serverError 720 } + +SELECT runningDifference(number) FROM system.numbers LIMIT 10; -- { serverError 720 } + +SELECT k, runningAccumulate(sum_k) AS res FROM (SELECT number as k, sumState(k) AS sum_k FROM numbers(10) GROUP BY k ORDER BY k); -- { serverError 720 } + +SET allow_deprecated_functions=1; + +SELECT number, neighbor(number, 2) FROM system.numbers LIMIT 10 FORMAT Null; + +SELECT runningDifference(number) FROM system.numbers LIMIT 10 FORMAT Null; + +SELECT k, runningAccumulate(sum_k) AS res FROM (SELECT number as k, sumState(k) AS sum_k FROM numbers(10) GROUP BY k ORDER BY k) FORMAT Null;