Merge pull request ClickHouse#77453 from ClickHouse/fuzzies

Add iteration limit in QueryFuzzer

Author: alexey-milovidov

src/Common/QueryFuzzer.cpp

@@ -146,6 +146,8 @@ Field QueryFuzzer::getRandomField(int type)
 
 Field QueryFuzzer::fuzzField(Field field)
 {
+    checkIterationLimit();
+
     const auto type = field.getType();
 
     int type_index = -1;
@@ -363,6 +365,8 @@ void QueryFuzzer::fuzzOrderByList(IAST * ast, const size_t nproj)
         return;
     }
 
+    checkIterationLimit();
+
     auto * list = assert_cast<ASTExpressionList *>(ast);
 
     /// Permute list
@@ -414,6 +418,8 @@ void QueryFuzzer::fuzzColumnLikeExpressionList(IAST * ast)
         return;
     }
 
+    checkIterationLimit();
+
     auto * impl = assert_cast<ASTExpressionList *>(ast);
 
     /// Permute list
@@ -477,6 +483,8 @@ void QueryFuzzer::fuzzNullsAction(NullsAction & action)
 
 void QueryFuzzer::fuzzWindowFrame(ASTWindowDefinition & def)
 {
+    checkIterationLimit();
+
     switch (fuzz_rand() % 40)
     {
         case 0: {
@@ -624,6 +632,8 @@ void QueryFuzzer::fuzzColumnDeclaration(ASTColumnDeclaration & column)
 
 DataTypePtr QueryFuzzer::fuzzDataType(DataTypePtr type)
 {
+    checkIterationLimit();
+
     /// Do not replace Array/Tuple/etc. with not Array/Tuple too often.
     const auto * type_array = typeid_cast<const DataTypeArray *>(type.get());
     if (type_array && fuzz_rand() % 4 != 0)
@@ -699,6 +709,8 @@ DataTypePtr QueryFuzzer::fuzzDataType(DataTypePtr type)
 
 DataTypePtr QueryFuzzer::getRandomType()
 {
+    checkIterationLimit();
+
     static const std::vector<TypeIndex> & random_types
         = {TypeIndex::UInt8,       TypeIndex::UInt16,         TypeIndex::UInt32,   TypeIndex::UInt64,     TypeIndex::UInt128,
            TypeIndex::UInt256,     TypeIndex::Int8,           TypeIndex::Int16,    TypeIndex::Int32,      TypeIndex::Int64,
@@ -935,6 +947,8 @@ void QueryFuzzer::notifyQueryFailed(ASTPtr ast)
 
 ASTPtr QueryFuzzer::fuzzLiteralUnderExpressionList(ASTPtr child)
 {
+    checkIterationLimit();
+
     const auto * l = child->as<ASTLiteral>();
     chassert(l);
     const auto type = l->value.getType();
@@ -1073,6 +1087,13 @@ struct ScopedIncrement
     ~ScopedIncrement() { --counter; }
 };
 
+void QueryFuzzer::checkIterationLimit()
+{
+    if (++iteration_count > iteration_limit)
+        throw Exception(ErrorCodes::TOO_DEEP_RECURSION,
+            "AST complexity limit exceeded while fuzzing ({})", iteration_count);
+}
+
 static const Strings comparison_comparators
     = {"equals", "notEquals", "greater", "greaterOrEquals", "less", "lessOrEquals", "isNotDistinctFrom"};
 
@@ -1504,6 +1525,8 @@ void QueryFuzzer::fuzz(ASTPtr & ast)
     if (!ast)
         return;
 
+    checkIterationLimit();
+
     // Check for exceeding max depth.
     ScopedIncrement depth_increment(current_ast_depth);
     if (current_ast_depth > 500)
@@ -1528,7 +1551,7 @@ void QueryFuzzer::fuzz(ASTPtr & ast)
             current_ast_depth,
             debug_visited_nodes.size(),
             (*debug_top_ast)->dumpTree());
-        assert(false);
+        std::abort();
     }
 
     // The fuzzing.
@@ -2169,6 +2192,7 @@ void QueryFuzzer::collectFuzzInfoRecurse(ASTPtr ast)
 void QueryFuzzer::fuzzMain(ASTPtr & ast)
 {
     current_ast_depth = 0;
+    iteration_count = 0;
     debug_visited_nodes.clear();
     debug_top_ast = &ast;
 

src/Common/QueryFuzzer.h

@@ -157,6 +157,26 @@ class QueryFuzzer
     // Used to track added tables in join clauses
     uint32_t alias_counter = 0;
 
+    // Similar to current_ast_depth, this is a limit on some measure of query size or number of
+    // steps we take. Without it, with small probability, query size may explode even when depth is
+    // limited. In particular, array lengths and depths in fuzzField() were seen to do that:
+    // https://github.com/ClickHouse/ClickHouse/issues/77408
+    //
+    // I don't fully understand how this happens, but my impression is that recursive random
+    // generators like this just generally tend to produce size distribution with heavy tail.
+    // Maybe if the query size/depth/some-other-property reaches some critical mass, each recursive
+    // call on average causes more than one additional recursive call (e.g. by copying a huge subtree
+    // with some small-but-not-tiny probability), so the expected number of calls becomes infinite.
+    // Despite infinite expected tree size, the p99 size may still be moderate
+    // (see e.g. "St. Petersburg lottery"), so the failures can be rare in practice.
+    //
+    // (What does "infinite expected value" mean in practice? Suppose you keep generating more and
+    //  more values and averaging them. If the expected value is finite, the average will be
+    //  converging to it. If the expected value is infinite, the average will keep growing without
+    //  bound.)
+    size_t iteration_count = 0;
+    static constexpr size_t iteration_limit = 500000;
+
     // These arrays hold parts of queries that we can substitute into the query
     // we are currently fuzzing. We add some part from each new query we are asked
     // to fuzz, and keep this state between queries, so the fuzzing output becomes
@@ -211,6 +231,7 @@ class QueryFuzzer
     void addTableLike(ASTPtr ast);
     void addColumnLike(ASTPtr ast);
     void collectFuzzInfoRecurse(ASTPtr ast);
+    void checkIterationLimit();
 
     void extractPredicates(const ASTPtr & node, ASTs & predicates, const std::string & op, int negProb);
     ASTPtr permutePredicateClause(const ASTPtr & predicate, int negProb);