Merge pull request #35334 from amosbird/fixpartitionpruneroverflow

Fix incorrect partition pruning when constant in predicate has no supertype of related key columns

src/Client/ClientBase.cpp

@@ -1976,25 +1976,23 @@ void ClientBase::readArguments(
 
     for (int arg_num = 1; arg_num < argc; ++arg_num)
     {
-        const char * arg = argv[arg_num];
+        std::string_view arg = argv[arg_num];
 
-        if (arg == "--external"sv)
+        if (arg == "--external")
         {
             in_external_group = true;
             external_tables_arguments.emplace_back(Arguments{""});
         }
         /// Options with value after equal sign.
-        else if (in_external_group
+        else if (
-            && (0 == strncmp(arg, "--file=", strlen("--file=")) || 0 == strncmp(arg, "--name=", strlen("--name="))
+            in_external_group
-                || 0 == strncmp(arg, "--format=", strlen("--format=")) || 0 == strncmp(arg, "--structure=", strlen("--structure="))
+            && (arg.starts_with("--file=") || arg.starts_with("--name=") || arg.starts_with("--format=") || arg.starts_with("--structure=")
-                || 0 == strncmp(arg, "--types=", strlen("--types="))))
+                || arg.starts_with("--types=")))
         {
             external_tables_arguments.back().emplace_back(arg);
         }
         /// Options with value after whitespace.
-        else if (in_external_group
+        else if (in_external_group && (arg == "--file" || arg == "--name" || arg == "--format" || arg == "--structure" || arg == "--types"))
-            && (arg == "--file"sv || arg == "--name"sv || arg == "--format"sv
-                || arg == "--structure"sv || arg == "--types"sv))
         {
             if (arg_num + 1 < argc)
             {
@@ -2011,20 +2009,12 @@ void ClientBase::readArguments(
             in_external_group = false;
 
             /// Parameter arg after underline.
-            if (startsWith(arg, "--param_"))
+            if (arg.starts_with("--param_"))
             {
-                const char * param_continuation = arg + strlen("--param_");
+                auto param_continuation = arg.substr(strlen("--param_"));
-                const char * equal_pos = strchr(param_continuation, '=');
+                auto equal_pos = param_continuation.find_first_of('=');
 
-                if (equal_pos == param_continuation)
+                if (equal_pos == std::string::npos)
-                    throw Exception("Parameter name cannot be empty", ErrorCodes::BAD_ARGUMENTS);
-
-                if (equal_pos)
-                {
-                    /// param_name=value
-                    query_parameters.emplace(String(param_continuation, equal_pos), String(equal_pos + 1));
-                }
-                else
                 {
                     /// param_name value
                     ++arg_num;
@@ -2033,12 +2023,20 @@ void ClientBase::readArguments(
                     arg = argv[arg_num];
                     query_parameters.emplace(String(param_continuation), String(arg));
                 }
+                else
+                {
+                    if (equal_pos == 0)
+                        throw Exception("Parameter name cannot be empty", ErrorCodes::BAD_ARGUMENTS);
+
+                    /// param_name=value
+                    query_parameters.emplace(param_continuation.substr(0, equal_pos), param_continuation.substr(equal_pos + 1));
                 }
-            else if (startsWith(arg, "--host") || startsWith(arg, "-h"))
+            }
+            else if (arg.starts_with("--host") || arg.starts_with("-h"))
             {
                 std::string host_arg;
                 /// --host host
-                if (arg == "--host"sv || arg == "-h"sv)
+                if (arg == "--host" || arg == "-h")
                 {
                     ++arg_num;
                     if (arg_num >= argc)
@@ -2065,11 +2063,11 @@ void ClientBase::readArguments(
                     prev_host_arg = host_arg;
                 }
             }
-            else if (startsWith(arg, "--port"))
+            else if (arg.starts_with("--port"))
             {
-                std::string port_arg = arg;
+                auto port_arg = String{arg};
                 /// --port port
-                if (arg == "--port"sv)
+                if (arg == "--port")
                 {
                     port_arg.push_back('=');
                     ++arg_num;
@@ -2094,7 +2092,7 @@ void ClientBase::readArguments(
                     prev_port_arg = port_arg;
                 }
             }
-            else if (arg == "--allow_repeated_settings"sv)
+            else if (arg == "--allow_repeated_settings")
                 allow_repeated_settings = true;
             else
                 common_arguments.emplace_back(arg);

src/Storages/MergeTree/KeyCondition.cpp

@@ -715,18 +715,12 @@ bool KeyCondition::transformConstantWithValidFunctions(
 
             if (is_valid_chain)
             {
-                /// Here we cast constant to the input type.
-                /// It is not clear, why this works in general.
-                /// I can imagine the case when expression like `column < const` is legal,
-                /// but `type(column)` and `type(const)` are of different types,
-                /// and const cannot be casted to column type.
-                /// (There could be `superType(type(column), type(const))` which is used for comparison).
-                ///
-                /// However, looks like this case newer happenes (I could not find such).
-                /// Let's assume that any two comparable types are castable to each other.
                 auto const_type = cur_node->result_type;
                 auto const_column = out_type->createColumnConst(1, out_value);
-                auto const_value = (*castColumn({const_column, out_type, ""}, const_type))[0];
+                auto const_value = (*castColumnAccurateOrNull({const_column, out_type, ""}, const_type))[0];
+
+                if (const_value.isNull())
+                    return false;
 
                 while (!chain.empty())
                 {

tests/queries/0_stateless/02232_partition_pruner_mixed_constant_type.reference

@@ -0,0 +1,4 @@
+1647353101000
+1647353101001
+1647353101002
+1647353101003

tests/queries/0_stateless/02232_partition_pruner_mixed_constant_type.sql

@@ -0,0 +1,7 @@
+DROP TABLE IF EXISTS broken;
+
+CREATE TABLE broken (time UInt64) ENGINE = MergeTree PARTITION BY toYYYYMMDD(toDate(time / 1000)) ORDER BY time;
+INSERT INTO broken (time) VALUES (1647353101000), (1647353101001), (1647353101002), (1647353101003);
+SELECT * FROM broken WHERE time>-1;
+
+DROP TABLE broken;