Merge pull request #9365 from ClickHouse/ubsan-index-set

Fixed UBSan report in MergeTreeIndexSet

dbms/src/Disks/DiskSpaceMonitor.cpp

@@ -12,6 +12,16 @@
 
 namespace DB
 {
+
+namespace ErrorCodes
+{
+    extern const int EXCESSIVE_ELEMENT_IN_CONFIG;
+    extern const int UNKNOWN_DISK;
+    extern const int UNKNOWN_POLICY;
+    extern const int LOGICAL_ERROR;
+}
+
+
 DiskSelector::DiskSelector(const Poco::Util::AbstractConfiguration & config, const String & config_prefix, const Context & context)
 {
     Poco::Util::AbstractConfiguration::Keys keys;
@@ -253,10 +263,10 @@ DiskPtr StoragePolicy::getAnyDisk() const
     /// StoragePolicy must contain at least one Volume
     /// Volume must contain at least one Disk
     if (volumes.empty())
-        throw Exception("StoragePolicy has no volumes. It's a bug.", ErrorCodes::NOT_ENOUGH_SPACE);
+        throw Exception("StoragePolicy has no volumes. It's a bug.", ErrorCodes::LOGICAL_ERROR);
 
     if (volumes[0]->disks.empty())
-        throw Exception("Volume '" + volumes[0]->getName() + "' has no disks. It's a bug.", ErrorCodes::NOT_ENOUGH_SPACE);
+        throw Exception("Volume '" + volumes[0]->getName() + "' has no disks. It's a bug.", ErrorCodes::LOGICAL_ERROR);
 
     return volumes[0]->disks[0];
 }

dbms/src/Disks/DiskSpaceMonitor.h

@@ -16,17 +16,6 @@
 
 namespace DB
 {
-namespace ErrorCodes
-{
-    extern const int LOGICAL_ERROR;
-    extern const int NOT_ENOUGH_SPACE;
-    extern const int NOT_IMPLEMENTED;
-    extern const int SYSTEM_ERROR;
-    extern const int UNKNOWN_ELEMENT_IN_CONFIG;
-    extern const int EXCESSIVE_ELEMENT_IN_CONFIG;
-    extern const int UNKNOWN_POLICY;
-    extern const int UNKNOWN_DISK;
-}
 
 /// Parse .xml configuration and store information about disks
 /// Mostly used for introspection.

dbms/src/Functions/bitWrapperFunc.cpp

@@ -27,7 +27,6 @@ namespace DB
 
 #if USE_EMBEDDED_COMPILER
         static constexpr bool compilable = false;
-
 #endif
     };
 

dbms/src/Storages/MergeTree/MergeTreeData.cpp

@@ -104,6 +104,8 @@ namespace ErrorCodes
     extern const int ABORTED;
     extern const int UNKNOWN_PART_TYPE;
     extern const int UNEXPECTED_AST_STRUCTURE;
+    extern const int UNKNOWN_DISK;
+    extern const int NOT_ENOUGH_SPACE;
 }
 
 

dbms/src/Storages/MergeTree/MergeTreeIndexSet.cpp

@@ -18,7 +18,7 @@ namespace ErrorCodes
 }
 
 /// 0b11 -- can be true and false at the same time
-const Field UNKNOWN_FIELD(3u);
+static const Field UNKNOWN_FIELD(3u);
 
 
 MergeTreeIndexGranuleSet::MergeTreeIndexGranuleSet(const MergeTreeIndexSet & index_)
@@ -236,8 +236,6 @@ MergeTreeIndexConditionSet::MergeTreeIndexConditionSet(
         expression_ast = select.where()->clone();
     else if (select.prewhere())
         expression_ast = select.prewhere()->clone();
-    else
-        expression_ast = std::make_shared<ASTLiteral>(UNKNOWN_FIELD);
 
     useless = checkASTUseless(expression_ast);
     /// Do not proceed if index is useless for this query.
@@ -260,6 +258,9 @@ bool MergeTreeIndexConditionSet::alwaysUnknownOrTrue() const
 
 bool MergeTreeIndexConditionSet::mayBeTrueOnGranule(MergeTreeIndexGranulePtr idx_granule) const
 {
+    if (useless)
+        return true;
+
     auto granule = std::dynamic_pointer_cast<MergeTreeIndexGranuleSet>(idx_granule);
     if (!granule)
         throw Exception(
@@ -405,8 +406,11 @@ bool MergeTreeIndexConditionSet::operatorFromAST(ASTPtr & node) const
     return true;
 }
 
-bool MergeTreeIndexConditionSet::checkASTUseless(const ASTPtr &node, bool atomic) const
+bool MergeTreeIndexConditionSet::checkASTUseless(const ASTPtr & node, bool atomic) const
 {
+    if (!node)
+        return true;
+
     if (const auto * func = node->as<ASTFunction>())
     {
         if (key_columns.count(func->getColumnName()))
@@ -422,7 +426,7 @@ bool MergeTreeIndexConditionSet::checkASTUseless(const ASTPtr &node, bool atomic
             return checkASTUseless(args[0], atomic);
         else
             return std::any_of(args.begin(), args.end(),
-                    [this](const auto & arg) { return checkASTUseless(arg, true); });
+                [this](const auto & arg) { return checkASTUseless(arg, true); });
     }
     else if (const auto * literal = node->as<ASTLiteral>())
         return !atomic && literal->value.get<bool>();

dbms/src/Storages/MergeTree/MergeTreeIndexSet.h

@@ -80,7 +80,7 @@ private:
     bool atomFromAST(ASTPtr & node) const;
     bool operatorFromAST(ASTPtr & node) const;
 
-    bool checkASTUseless(const ASTPtr &node, bool atomic = false) const;
+    bool checkASTUseless(const ASTPtr & node, bool atomic = false) const;
 
     const MergeTreeIndexSet & index;
 

dbms/src/Storages/StorageMergeTree.cpp

@@ -36,12 +36,8 @@ namespace ErrorCodes
     extern const int ABORTED;
     extern const int BAD_ARGUMENTS;
     extern const int INCORRECT_DATA;
-    extern const int INCORRECT_FILE_NAME;
     extern const int CANNOT_ASSIGN_OPTIMIZE;
-    extern const int INCOMPATIBLE_COLUMNS;
-    extern const int PART_IS_TEMPORARILY_LOCKED;
-    extern const int UNKNOWN_SETTING;
-    extern const int TOO_BIG_AST;
+    extern const int NOT_ENOUGH_SPACE;
 }
 
 namespace ActionLocks

dbms/tests/queries/0_stateless/01087_index_set_ubsan.reference

+1	1
+1	2
+1	3
+1	4

dbms/tests/queries/0_stateless/01087_index_set_ubsan.sql

+DROP TABLE IF EXISTS t;
+create table t (i Int, a Int, s String, index ind_s (s) type set(1) granularity 1) engine = MergeTree order by i;
+insert into t values (1, 1, 'a') (2, 1, 'a') (3, 1, 'a') (4, 1, 'a');
+SELECT a, i from t ORDER BY a, i;
+DROP TABLE t;