Antalya 26.3: Bump to 26.3.12

ci/jobs/check_style.py

@@ -6,6 +6,7 @@
 from concurrent.futures import ProcessPoolExecutor
 from pathlib import Path
 
+from praktika.info import Info
 from praktika.result import Result
 from praktika.utils import Shell, Utils
 
@@ -573,7 +574,13 @@ def parse_args():
             )
         )
     testname = "test_numbers_check"
-    if testpattern.lower() in testname.lower():
+    # Skip on release branches and backport PRs: backports cherry-pick a small
+    # subset of test files, which legitimately leaves large gaps in the numbering.
+    info = Info()
+    release_branch_re = re.compile(r"^\d{2}\.\d+$")
+    branch_to_check = (info.base_branch or info.git_branch or "").removeprefix("release/")
+    is_release_branch = bool(release_branch_re.match(branch_to_check))
+    if testpattern.lower() in testname.lower() and not is_release_branch:
         results.append(
             Result.from_commands_run(
                 name=testname,

cmake/autogenerated_versions.txt

@@ -2,13 +2,13 @@
 
 # NOTE: VERSION_REVISION has nothing common with DBMS_TCP_PROTOCOL_VERSION,
 # only DBMS_TCP_PROTOCOL_VERSION should be incremented on protocol changes.
-SET(VERSION_REVISION 54517)
+SET(VERSION_REVISION 54519)
 SET(VERSION_MAJOR 26)
 SET(VERSION_MINOR 3)
-SET(VERSION_PATCH 10)
-SET(VERSION_GITHASH 0d82c1998e3b4d54e110dd8a77a64d247c4bff4a)
-SET(VERSION_DESCRIBE v26.3.10.20001.altinityantalya)
-SET(VERSION_STRING 26.3.10.20001.altinityantalya)
+SET(VERSION_PATCH 12)
+SET(VERSION_GITHASH fa3aa24e79104d29f8bfde078fc586ec6ac3a565)
+SET(VERSION_DESCRIBE v26.3.12.20001.altinityantalya)
+SET(VERSION_STRING 26.3.12.20001.altinityantalya)
 # end of autochange
 
 SET(VERSION_TWEAK 20001)

src/Analyzer/Passes/InverseDictionaryLookupPass.cpp

@@ -13,6 +13,11 @@
 
 #include <Interpreters/Context.h>
 
+#include <DataTypes/DataTypeLowCardinality.h>
+#include <DataTypes/DataTypeNullable.h>
+#include <DataTypes/DataTypeTuple.h>
+
+#include <Functions/FunctionFactory.h>
 #include <Functions/FunctionsExternalDictionaries.h>
 
 #include <Access/ContextAccess.h>
@@ -26,6 +31,9 @@ namespace DB
 
 namespace Setting
 {
+extern const SettingsUInt64 max_bytes_in_set;
+extern const SettingsUInt64 max_rows_in_set;
+extern const SettingsOverflowMode set_overflow_mode;
 extern const SettingsBool optimize_inverse_dictionary_lookup;
 extern const SettingsBool rewrite_in_to_join;
 }
@@ -68,8 +76,7 @@ bool isSupportedDictGetFunction(const String & name)
            "dictGetDateTime",
            "dictGetUUID",
            "dictGetIPv4",
-           "dictGetIPv6",
-           "dictGetOrNull"};
+           "dictGetIPv6"};
 
     return supported_functions.contains(name);
 }
@@ -127,6 +134,98 @@ void resolveNode(const Node & node, const ContextPtr & context)
     QueryAnalysisPass(/*only_analyze*/ false).run(querytree_node, context);
 }
 
+bool hasNullableComponentInComplexKey(const QueryTreeNodePtr & key_expr_node)
+{
+    auto type = removeNullable(key_expr_node->getResultType());
+    const auto * tuple_type = typeid_cast<const DataTypeTuple *>(type.get());
+    if (!tuple_type)
+        return false;
+
+    for (const auto & element : tuple_type->getElements())
+    {
+        if (isNullableOrLowCardinalityNullable(element))
+            return true;
+    }
+    return false;
+}
+
+bool isRewriteSemanticallySafe(
+    const DataTypePtr & dict_attr_type,
+    const DataTypePtr & dictget_result_type,
+    const Field & attr_null_value,
+    const ConstantNode & const_arg_node,
+    bool default_is_lhs,
+    const String & attr_comparison_function_name,
+    const ContextPtr & context)
+{
+    /// Same underlying type after stripping `Nullable` / `LowCardinality` needed. If attribute `n`
+    /// is `UInt32`, `dictGetUInt16(..., 'n', id) = 42` throws because the underlying types differ (`UInt32` vs `UInt16`)
+    const bool stripped_types_match
+        = removeLowCardinalityAndNullable(dict_attr_type)->equals(*removeLowCardinalityAndNullable(dictget_result_type));
+    if (!stripped_types_match)
+        return false;
+
+    /// `dictGet` and `IN` don't have the same stored-NULL attribute semantics.
+    /// Example: if dictionary has `id = 1, name = NULL`, `dictGet(..., 1) = 'x'` gives
+    /// `NULL`. The `IN` rewrite uses `WHERE name = 'x'`, so the row is filtered out and
+    /// `1 IN (...)` gives `0`. This is visible in projection or `isNull(predicate)`.
+    /// Skip optimization when the attribute can contain `NULL`, including
+    /// `LowCardinality(Nullable(...))`.
+    if (isNullableOrLowCardinalityNullable(dict_attr_type))
+        return false;
+
+    const DataTypePtr const_arg_type = const_arg_node.getResultType();
+    const Field & const_arg_value = const_arg_node.getValue();
+
+    auto default_column = ColumnWithTypeAndName(dict_attr_type->createColumnConst(1, attr_null_value), dict_attr_type, "default_value");
+    auto const_arg_column = ColumnWithTypeAndName(const_arg_type->createColumnConst(1, const_arg_value), const_arg_type, "const_value");
+
+    ColumnsWithTypeAndName comparison_arguments;
+    if (default_is_lhs)
+        comparison_arguments = {std::move(default_column), std::move(const_arg_column)};
+    else
+        comparison_arguments = {std::move(const_arg_column), std::move(default_column)};
+
+    /// `dictGet` and `IN` don't have the same missing-key default semantics.
+    /// e.g: `dictGet(..., id) = ''` vs `id IN (SELECT id FROM dictionary(...) WHERE name = '')`
+    /// Example: if dictionary has one row `id = 1, name = 'x'`, data has `id = 2`, and
+    /// attribute `DEFAULT` is `''`, `dictGet(..., 2)` returns `''`, so
+    /// `dictGet(..., id) = ''` is true for `id = 2`. The `IN` rewrite scans only
+    /// dictionary keys, so the subquery has no `id = 2` and `2 IN (...)` is false.
+    ///
+    /// One of the alternatives is to add `OR id NOT IN (SELECT id FROM dictionary(...))` when the
+    /// predicate is true for `DEFAULT`, but it requires another set with all dictionary keys.
+    /// This can be expensive to materialize, so skip optimization for such case.
+    ///
+    /// As a result, given the current rewrite, if `const <op> DEFAULT` is false, only then the
+    /// transformation is semantically correct.
+    Field comparison_result;
+    try
+    {
+        auto function_resolver = FunctionFactory::instance().get(attr_comparison_function_name, context);
+        auto comparison_function_base = function_resolver->build(comparison_arguments);
+        auto comparison_result_column
+            = comparison_function_base->execute(comparison_arguments, comparison_function_base->getResultType(), 1, /* dry_run = */ false);
+        comparison_result = (*comparison_result_column)[0];
+    }
+    catch (const Exception &)
+    {
+        /// The constant fold runs during optimization and can throw for values that runtime
+        /// would not evaluate. Example: `match('', '(')` throws `CANNOT_COMPILE_REGEXP`, but
+        /// `id < 0 AND match(dictGetString(...), '(')` can skip the `match` branch due to
+        /// short-circuit evaluation. If we throw here, the optimization breaks a query that
+        /// works without it. Skip optimization for such case.
+        return false;
+    }
+
+    if (comparison_result.isNull())
+        return false;
+
+    /// Check `const <op> DEFAULT` is false
+    UInt64 comparison_result_uint = 0;
+    return comparison_result.tryGet<UInt64>(comparison_result_uint) && comparison_result_uint == 0;
+}
+
 
 class InverseDictionaryLookupVisitor : public InDepthQueryTreeVisitorWithContext<InverseDictionaryLookupVisitor>
 {
@@ -142,6 +241,14 @@ class InverseDictionaryLookupVisitor : public InDepthQueryTreeVisitorWithContext
         if (getSettings()[Setting::rewrite_in_to_join])
             return;
 
+        /// We build an `IN` set from the dictionary subquery, which respects `max_rows_in_set`,
+        /// `max_bytes_in_set` and `set_overflow_mode`. With `set_overflow_mode = 'break'`, the set
+        /// can be truncated and not contain all required elements, so the optimization can produce
+        /// wrong results. Skip optimization for such case.
+        if ((getSettings()[Setting::max_rows_in_set] != 0 || getSettings()[Setting::max_bytes_in_set] != 0)
+            && getSettings()[Setting::set_overflow_mode] == OverflowMode::BREAK)
+            return;
+
         auto * node_function = node->as<FunctionNode>();
 
         if (!node_function)
@@ -226,12 +333,37 @@ class InverseDictionaryLookupVisitor : public InDepthQueryTreeVisitorWithContext
             return;
         }
 
+        /// For complex-key dictionaries, `dictGet` and `IN` don't have the same `NULL` key semantics.
+        /// e.g: `dictGet(..., (k1, k2))` vs `(k1, k2) IN (SELECT k1, k2 FROM dictionary(...))`
+        /// Example: if `k1` is `Nullable(UInt64)` and the dictionary has `(NULL, 'a')`,
+        /// `dictGet(..., (k1, k2))` can match it, but `(NULL, 'a') IN (...)` is not a match
+        /// with `transform_null_in = 0`.
+        /// Single-key dictionaries are not affected. Example: if `id` is `Nullable(UInt64)`,
+        /// `dictGet(..., id) = 'x'` gives `NULL` for `id = NULL`, and `id IN (...)` also gives
+        /// `NULL` for `id = NULL`.
+        if (dict_structure.key && hasNullableComponentInComplexKey(dictget_function_info.key_expr_node))
+            return;
+
         const String attr_col_name = dictget_function_info.attr_col_name_node->getValue().safeGet<String>();
 
         if (!dict_structure.hasAttribute(attr_col_name))
             return;
 
-        DataTypePtr dict_attr_col_type = dict_structure.getAttribute(attr_col_name).type;
+        const DictionaryAttribute & attr = dict_structure.getAttribute(attr_col_name);
+        DataTypePtr dict_attr_col_type = attr.type;
+
+        const auto * const_arg_node = (dict_side == Side::LHS) ? arguments[1]->as<ConstantNode>() : arguments[0]->as<ConstantNode>();
+
+        /// Skip rewrites that would change query behavior. Details are in the function.
+        if (!isRewriteSemanticallySafe(
+                dict_attr_col_type,
+                dictget_function_info.return_type,
+                attr.null_value,
+                *const_arg_node,
+                dict_side == Side::LHS,
+                attr_comparison_function_name,
+                getContext()))
+            return;
 
         auto dict_table_function = std::make_shared<TableFunctionNode>("dictionary");
         dict_table_function->getArguments().getNodes().push_back(dictget_function_info.dict_name_node);
@@ -240,23 +372,16 @@ class InverseDictionaryLookupVisitor : public InDepthQueryTreeVisitorWithContext
         NameAndTypePair attr_col{attr_col_name, dict_attr_col_type};
         auto attr_col_node = std::make_shared<ColumnNode>(attr_col, dict_table_function);
 
-        /// Needed for dictGet functions like `dictGetString`, `dictGetInt32`, etc.
-        QueryTreeNodePtr attr_col_node_casted = attr_col_node;
-        if (!attr_col_node->getResultType()->equals(*dictget_function_info.return_type))
-        {
-            attr_col_node_casted = createCastFunction(attr_col_node, dictget_function_info.return_type, getContext());
-        }
-
         auto attr_comparison_function_node = std::static_pointer_cast<FunctionNode>(node_function->clone());
         attr_comparison_function_node->markAsOperator();
 
         if (dict_side == Side::LHS)
         {
-            attr_comparison_function_node->getArguments().getNodes() = { attr_col_node_casted, arguments[1] };
+            attr_comparison_function_node->getArguments().getNodes() = { attr_col_node, arguments[1] };
         }
         else
         {
-            attr_comparison_function_node->getArguments().getNodes() = { arguments[0], attr_col_node_casted };
+            attr_comparison_function_node->getArguments().getNodes() = { arguments[0], attr_col_node };
         }
         resolveOrdinaryFunctionNodeByName(*attr_comparison_function_node, attr_comparison_function_name, getContext());
 

src/Backups/BackupEntriesCollector.cpp

@@ -11,9 +11,12 @@
 #include <Databases/IDatabase.h>
 #include <Interpreters/Context.h>
 #include <Interpreters/DatabaseCatalog.h>
+#include <Interpreters/StorageID.h>
 #include <Parsers/ASTCreateQuery.h>
 #include <Storages/IStorage.h>
 #include <Storages/MergeTree/extractZooKeeperPathFromReplicatedTableDef.h>
+#include <Storages/StorageMaterializedView.h>
+#include <Common/typeid_cast.h>
 #include <base/chrono_io.h>
 #include <base/insertAtEnd.h>
 #include <base/scope_guard.h>
@@ -524,6 +527,14 @@ void BackupEntriesCollector::gatherTablesMetadata()
     checkIsQueryCancelled();
 
     table_infos.clear();
+
+    /// Collect target tables of refreshable materialized views that use the REPLACE
+    /// refresh strategy (APPEND is excluded) among the tables being backed up.
+    /// We build this snapshot from the storages we've already found, so the decision
+    /// in `shouldBackupTableData` doesn't need to query `DatabaseCatalog` again and
+    /// is scoped to the tables within the backup.
+    std::unordered_set<StorageID, StorageID::DatabaseAndTableNameHash, StorageID::DatabaseAndTableNameEqual> rmv_replace_target_ids;
+
     for (const auto & [database_name, database_info] : database_infos)
     {
         std::vector<std::pair<ASTPtr, StoragePtr>> db_tables = findTablesInDatabase(database_name);
@@ -551,35 +562,48 @@ void BackupEntriesCollector::gatherTablesMetadata()
                     / escapeForFileName(table_name_in_backup.table);
             }
 
-            /// Add information to `table_infos`.
             const auto qualified_name = QualifiedTableName{database_name, table_name};
             auto & res_table_info = table_infos[qualified_name];
             res_table_info.database = database_info.database;
             res_table_info.storage = storage;
             res_table_info.create_table_query = create_table_query;
             res_table_info.metadata_path_in_backup = metadata_path_in_backup;
             res_table_info.data_path_in_backup = data_path_in_backup;
-            res_table_info.should_backup_data = shouldBackupTableData(qualified_name, storage);
 
-            if (res_table_info.should_backup_data)
+            if (const auto * mv = typeid_cast<const StorageMaterializedView *>(storage.get()))
             {
-                auto it = database_info.tables.find(table_name);
-                if (it != database_info.tables.end())
-                {
-                    const auto & partitions = it->second.partitions;
-                    if (partitions && storage && !storage->supportsBackupPartition())
-                    {
-                        throw Exception(
-                            ErrorCodes::CANNOT_BACKUP_TABLE,
-                            "Table engine {} doesn't support partitions, cannot backup {}",
-                            storage->getName(),
-                            tableNameWithTypeToString(database_name, table_name, false));
-                    }
-                    res_table_info.partitions = partitions;
-                }
+                if (mv->isRefreshable() && !mv->isAppendRefreshStrategy())
+                    rmv_replace_target_ids.insert(mv->getTargetTableId());
             }
         }
     }
+
+    /// Second pass: now that we have the full snapshot of tables and RMV targets,
+    /// decide whether the data of each table should be backed up and validate
+    /// partition-related constraints.
+    for (auto & [qualified_name, res_table_info] : table_infos)
+    {
+        res_table_info.should_backup_data = shouldBackupTableData(qualified_name, res_table_info.storage, rmv_replace_target_ids);
+
+        if (!res_table_info.should_backup_data)
+            continue;
+
+        const auto & database_info = database_infos.at(qualified_name.database);
+        auto it = database_info.tables.find(qualified_name.table);
+        if (it == database_info.tables.end())
+            continue;
+
+        const auto & partitions = it->second.partitions;
+        if (partitions && res_table_info.storage && !res_table_info.storage->supportsBackupPartition())
+        {
+            throw Exception(
+                ErrorCodes::CANNOT_BACKUP_TABLE,
+                "Table engine {} doesn't support partitions, cannot backup {}",
+                res_table_info.storage->getName(),
+                tableNameWithTypeToString(qualified_name.database, qualified_name.table, false));
+        }
+        res_table_info.partitions = partitions;
+    }
 }
 
 std::vector<std::pair<ASTPtr, StoragePtr>> BackupEntriesCollector::findTablesInDatabase(const String & database_name) const
@@ -857,7 +881,10 @@ void BackupEntriesCollector::makeBackupEntriesForTableData(const QualifiedTableN
     }
 }
 
-bool BackupEntriesCollector::shouldBackupTableData(const QualifiedTableName & table_name, const StoragePtr & storage) const
+bool BackupEntriesCollector::shouldBackupTableData(
+    const QualifiedTableName & table_name,
+    const StoragePtr & storage,
+    const std::unordered_set<StorageID, StorageID::DatabaseAndTableNameHash, StorageID::DatabaseAndTableNameEqual> & rmv_replace_target_ids) const
 {
     if (backup_settings.structure_only)
         return false;
@@ -866,7 +893,7 @@ bool BackupEntriesCollector::shouldBackupTableData(const QualifiedTableName & ta
         return true;
 
     if (!backup_settings.backup_data_from_refreshable_materialized_view_targets
-        && BackupUtils::isTargetForReplaceRefreshableMaterializedView(storage->getStorageID(), context))
+        && rmv_replace_target_ids.contains(storage->getStorageID()))
     {
         LOG_TRACE(log, "Skipping table data for {} (a target of a refreshable materialized view)", table_name.getFullName());
         return false;

src/Backups/BackupEntriesCollector.h

@@ -5,6 +5,7 @@
 #include <Backups/BackupSettings.h>
 #include <Core/QualifiedTableName.h>
 #include <Databases/DDLRenamingVisitor.h>
+#include <Interpreters/StorageID.h>
 #include <Parsers/ASTBackupQuery.h>
 #include <Storages/IStorage_fwd.h>
 #include <Storages/TableLockHolder.h>
@@ -20,7 +21,6 @@ using BackupEntries = std::vector<std::pair<String, BackupEntryPtr>>;
 class IBackupCoordination;
 class IDatabase;
 using DatabasePtr = std::shared_ptr<IDatabase>;
-struct StorageID;
 struct IAccessEntity;
 using AccessEntityPtr = std::shared_ptr<const IAccessEntity>;
 class QueryStatus;
@@ -97,7 +97,10 @@ class BackupEntriesCollector : private boost::noncopyable
     void makeBackupEntriesForTablesDefs();
     void makeBackupEntriesForTablesData();
     void makeBackupEntriesForTableData(const QualifiedTableName & table_name);
-    bool shouldBackupTableData(const QualifiedTableName & table_name, const StoragePtr & storage) const;
+    bool shouldBackupTableData(
+        const QualifiedTableName & table_name,
+        const StoragePtr & storage,
+        const std::unordered_set<StorageID, StorageID::DatabaseAndTableNameHash, StorageID::DatabaseAndTableNameEqual> & rmv_replace_target_ids) const;
 
     void addBackupEntryUnlocked(const String & file_name, BackupEntryPtr backup_entry);