Merge pull request #16406 from ClickHouse/fix-tsan-lgamma

Fix TSan report in lgamma

src/Access/ContextAccess.cpp

@@ -100,7 +100,7 @@ namespace
             if (res & alter_table)
                 res |= alter_view;
 
-            /// CREATE TABLE (on any database/table) => CREATE_TEMPORARY_TABLE (global) 
+            /// CREATE TABLE (on any database/table) => CREATE_TEMPORARY_TABLE (global)
             static const AccessFlags create_temporary_table = AccessType::CREATE_TEMPORARY_TABLE;
             if ((level == 0) && (max_flags_with_children & create_table))
                 res |= create_temporary_table;

src/AggregateFunctions/AggregateFunctionStudentTTest.h

@@ -20,9 +20,17 @@
 
 namespace ErrorCodes
 {
-extern const int BAD_ARGUMENTS;
+    extern const int BAD_ARGUMENTS;
 }
 
+#if defined(OS_DARWIN)
+extern "C"
+{
+    double lgammal_r(double x, int * signgamp);
+}
+#endif
+
+
 namespace DB
 {
 
@@ -98,7 +106,7 @@ struct AggregateFunctionStudentTTestData final
 
     Float64 getSSquared() const
     {
-        /// The original formulae looks like  
+        /// The original formulae looks like
         /// \frac{\sum_{i = 1}^{n_x}{(x_i - \bar{x}) ^ 2} + \sum_{i = 1}^{n_y}{(y_i - \bar{y}) ^ 2}}{n_x + n_y - 2}
         /// But we made some mathematical transformations not to store original sequences.
         /// Also we dropped sqrt, because later it will be squared later.
@@ -150,7 +158,8 @@ struct AggregateFunctionStudentTTestData final
         const Float64 t = getTStatisticSquared();
         auto f = [&v] (double x) { return std::pow(x, v/2 - 1) / std::sqrt(1 - x); };
         Float64 numenator = integrateSimpson(0, v / (t + v), f);
-        Float64 denominator = std::exp(std::lgammal(v/2) + std::lgammal(0.5) - std::lgammal(v/2 + 0.5));
+        int unused;
+        Float64 denominator = std::exp(lgammal_r(v / 2, &unused) + lgammal_r(0.5, &unused) - lgammal_r(v / 2 + 0.5, &unused));
         return numenator / denominator;
     }
 

src/AggregateFunctions/AggregateFunctionWelchTTest.h

@@ -18,11 +18,20 @@
 
 #include <type_traits>
 
+
 namespace ErrorCodes
 {
-extern const int BAD_ARGUMENTS;
+    extern const int BAD_ARGUMENTS;
 }
 
+#if defined(OS_DARWIN)
+extern "C"
+{
+    double lgammal_r(double x, int * signgamp);
+}
+#endif
+
+
 namespace DB
 {
 
@@ -159,9 +168,10 @@ struct AggregateFunctionWelchTTestData final
     {
         const Float64 v = getDegreesOfFreedom();
         const Float64 t = getTStatisticSquared();
-        auto f = [&v] (double x) { return std::pow(x, v/2 - 1) / std::sqrt(1 - x); };
+        auto f = [&v] (double x) { return std::pow(x, v / 2 - 1) / std::sqrt(1 - x); };
         Float64 numenator = integrateSimpson(0, v / (t + v), f);
-        Float64 denominator = std::exp(std::lgammal(v/2) + std::lgammal(0.5) - std::lgammal(v/2 + 0.5));
+        int unused;
+        Float64 denominator = std::exp(lgammal_r(v / 2, &unused) + lgammal_r(0.5, &unused) - lgammal_r(v / 2 + 0.5, &unused));
         return numenator / denominator;
     }
 

src/Columns/ColumnNullable.cpp

@@ -344,7 +344,7 @@ void ColumnNullable::updatePermutation(bool reverse, size_t limit, int null_dire
         /// Shift all NULL values to the end.
         for (const auto & [first, last] : equal_ranges)
         {
-            /// Current interval is righter than limit. 
+            /// Current interval is righter than limit.
             if (limit && first > limit)
                 break;
 

src/Common/ZooKeeper/ZooKeeperImpl.cpp

@@ -1294,7 +1294,7 @@ void ZooKeeper::receiveEvent()
         if (request_info.watch)
         {
             bool add_watch = false;
-            /// 3 indicates the ZooKeeperExistsRequest. 
+            /// 3 indicates the ZooKeeperExistsRequest.
             // For exists, we set the watch on both node exist and nonexist case.
             // For other case like getData, we only set the watch when node exists.
             if (request_info.request->getOpNum() == 3)

src/DataStreams/SquashingTransform.cpp

@@ -27,7 +27,7 @@ Block SquashingTransform::add(const Block & input_block)
 
 /*
  * To minimize copying, accept two types of argument: const reference for output
- * stream, and rvalue reference for input stream, and decide whether to copy 
+ * stream, and rvalue reference for input stream, and decide whether to copy
  * inside this function. This allows us not to copy Block unless we absolutely
  * have to.
  */

src/Functions/lgamma.cpp

@@ -4,7 +4,6 @@
 #if defined(OS_DARWIN)
 extern "C"
 {
-    /// Is defined in libglibc-compatibility.a
     double lgamma_r(double x, int * signgamp);
 }
 #endif

src/Interpreters/InterpreterShowTablesQuery.cpp

@@ -50,7 +50,7 @@ String InterpreterShowTablesQuery::getRewrittenQuery()
         return rewritten_query.str();
     }
 
-    /// SHOW CLUSTER/CLUSTERS 
+    /// SHOW CLUSTER/CLUSTERS
     if (query.clusters)
     {
         std::stringstream rewritten_query;

src/Interpreters/MySQL/InterpretersMySQLDDLQuery.cpp

@@ -56,7 +56,7 @@ static inline String resolveDatabase(
         }
     }
 
-    /// When USE other_database_name; CREATE TABLE table_name; 
+    /// When USE other_database_name; CREATE TABLE table_name;
     /// context.getCurrentDatabase() is always return `default database`
     /// When USE replica_mysql_database; CREATE TABLE table_name;
     /// context.getCurrentDatabase() is always return replica_clickhouse_database

src/Interpreters/PreparedSets.h

@@ -6,6 +6,7 @@
 #include <unordered_map>
 #include <DataTypes/DataTypeLowCardinality.h>
 
+
 namespace DB
 {
 
@@ -16,7 +17,7 @@ struct PreparedSetKey
     /// if left hand sides of the IN operators have different types).
     static PreparedSetKey forLiteral(const IAST & ast, DataTypes types_)
     {
-        /// Remove LowCardinality types from type list because Set doesn't support LowCardinality keys now, 
+        /// Remove LowCardinality types from type list because Set doesn't support LowCardinality keys now,
         ///   just converts LowCardinality to ordinary types.
         for (auto & type : types_)
             type = recursiveRemoveLowCardinality(type);

src/Processors/Transforms/ConvertingTransform.h

@@ -1,7 +1,9 @@
 #pragma once
+
 #include <Processors/ISimpleTransform.h>
 #include <Core/ColumnNumbers.h>
 
+
 namespace DB
 {
 
@@ -46,7 +48,7 @@ private:
     /// How to construct result block. Position in source block, where to get each column.
     ColumnNumbers conversion;
     /// Do not check that constants are same. Use value from result_header.
-    /// This is needed in case run functions which are constant in query scope, 
+    /// This is needed in case run functions which are constant in query scope,
     /// but may return different result being executed remotely, like `now64()` or `randConstant()`.
     /// In this case we replace constants from remote source to constatns from initiator.
     bool ignore_constant_values;

utils/check-style/check-style

@@ -93,12 +93,15 @@ git status -uno | grep ya.make && echo "ya.make files should be generated with u
 find $ROOT_PATH/{src,programs,utils} -name '*.h' | while read file; do [[ $(head -n1 $file) != '#pragma once' ]] && echo "File $file must have '#pragma once' in first line"; done
 
 # Check for executable bit on non-executable files
-find $ROOT_PATH/{src,base,programs,utils,tests,docs,website,cmake} '(' -name '*.cpp' -or -name '*.h' -or -name '*.sql' -or -name '*.xml' -or -name '*.reference' -or -name '*.txt' -or -name '*.md' ')' -and -executable | grep -q '.' && echo "These files should not be executable."
+find $ROOT_PATH/{src,base,programs,utils,tests,docs,website,cmake} '(' -name '*.cpp' -or -name '*.h' -or -name '*.sql' -or -name '*.xml' -or -name '*.reference' -or -name '*.txt' -or -name '*.md' ')' -and -executable | grep -P '.' && echo "These files should not be executable."
 
 # Check for BOM
-find $ROOT_PATH/{src,base,programs,utils,tests,docs,website,cmake} -name '*.md' -or -name '*.cpp' -or -name '*.h' | xargs grep -l -F $'\xEF\xBB\xBF' && echo "Files should not have UTF-8 BOM"
-find $ROOT_PATH/{src,base,programs,utils,tests,docs,website,cmake} -name '*.md' -or -name '*.cpp' -or -name '*.h' | xargs grep -l -F $'\xFF\xFE' && echo "Files should not have UTF-16LE BOM"
-find $ROOT_PATH/{src,base,programs,utils,tests,docs,website,cmake} -name '*.md' -or -name '*.cpp' -or -name '*.h' | xargs grep -l -F $'\xFE\xFF' && echo "Files should not have UTF-16BE BOM"
+find $ROOT_PATH/{src,base,programs,utils,tests,docs,website,cmake} -name '*.md' -or -name '*.cpp' -or -name '*.h' | xargs grep -l -F $'\xEF\xBB\xBF' | grep -P '.' && echo "Files should not have UTF-8 BOM"
+find $ROOT_PATH/{src,base,programs,utils,tests,docs,website,cmake} -name '*.md' -or -name '*.cpp' -or -name '*.h' | xargs grep -l -F $'\xFF\xFE' | grep -P '.' && echo "Files should not have UTF-16LE BOM"
+find $ROOT_PATH/{src,base,programs,utils,tests,docs,website,cmake} -name '*.md' -or -name '*.cpp' -or -name '*.h' | xargs grep -l -F $'\xFE\xFF' | grep -P '.' && echo "Files should not have UTF-16BE BOM"
 
 # Too many exclamation marks
-find $ROOT_PATH/{src,base,programs,utils} -name '*.h' -or -name '*.cpp' | xargs grep -F '!!!' && echo "Too many exclamation marks (looks dirty, unconfident)."
+find $ROOT_PATH/{src,base,programs,utils} -name '*.h' -or -name '*.cpp' | xargs grep -F '!!!' | grep -P '.' && echo "Too many exclamation marks (looks dirty, unconfident)."
+
+# Trailing whitespaces
+find $ROOT_PATH/{src,base,programs,utils} -name '*.h' -or -name '*.cpp' | xargs grep -P ' $' | grep -P '.' && echo "^ Trailing whitespaces."