From f29e5c60a5739cb809fc962041f2f911d72bf763 Mon Sep 17 00:00:00 2001
From: Azat Khuzhin <a3at.mail@gmail.com>
Date: Fri, 17 Apr 2020 01:23:31 +0300
Subject: [PATCH 1/3] Add const qualifier for IFunction::isInjective()

---
 src/Functions/FunctionStringToString.h        |  2 +-
 src/Functions/FunctionUnaryArithmetic.h       |  2 +-
 src/Functions/FunctionsCoding.h               | 20 +++++++++----------
 src/Functions/FunctionsConversion.h           |  4 ++--
 src/Functions/FunctionsEmbeddedDictionaries.h |  2 +-
 src/Functions/FunctionsExternalDictionaries.h | 10 +++++-----
 src/Functions/FunctionsFormatting.h           |  2 +-
 src/Functions/IFunction.h                     |  2 +-
 src/Functions/IFunctionAdaptors.h             |  4 ++--
 src/Functions/IFunctionImpl.h                 |  4 ++--
 src/Functions/concat.cpp                      |  2 +-
 src/Functions/reverse.cpp                     |  2 +-
 src/Functions/tuple.cpp                       |  2 +-
 src/Interpreters/ExpressionJIT.cpp            |  2 +-
 src/Interpreters/ExpressionJIT.h              |  2 +-
 15 files changed, 31 insertions(+), 31 deletions(-)

diff --git a/src/Functions/FunctionStringToString.h b/src/Functions/FunctionStringToString.h
index 1f712a7a785..8bc8ad8575d 100644
--- a/src/Functions/FunctionStringToString.h
+++ b/src/Functions/FunctionStringToString.h
@@ -35,7 +35,7 @@ public:
         return 1;
     }
 
-    bool isInjective(const Block &) override
+    bool isInjective(const Block &) const override
     {
         return is_injective;
     }
diff --git a/src/Functions/FunctionUnaryArithmetic.h b/src/Functions/FunctionUnaryArithmetic.h
index 89687b5b23e..cafbfcf6641 100644
--- a/src/Functions/FunctionUnaryArithmetic.h
+++ b/src/Functions/FunctionUnaryArithmetic.h
@@ -112,7 +112,7 @@ public:
     }
 
     size_t getNumberOfArguments() const override { return 1; }
-    bool isInjective(const Block &) override { return is_injective; }
+    bool isInjective(const Block &) const override { return is_injective; }
 
     bool useDefaultImplementationForConstants() const override { return true; }
 
diff --git a/src/Functions/FunctionsCoding.h b/src/Functions/FunctionsCoding.h
index 597caec91d2..6a3d6db546c 100644
--- a/src/Functions/FunctionsCoding.h
+++ b/src/Functions/FunctionsCoding.h
@@ -72,7 +72,7 @@ public:
     String getName() const override { return name; }
 
     size_t getNumberOfArguments() const override { return 1; }
-    bool isInjective(const Block &) override { return true; }
+    bool isInjective(const Block &) const override { return true; }
 
     DataTypePtr getReturnTypeImpl(const DataTypes & arguments) const override
     {
@@ -326,7 +326,7 @@ public:
     }
 
     size_t getNumberOfArguments() const override { return 1; }
-    bool isInjective(const Block &) override { return mask_tail_octets == 0; }
+    bool isInjective(const Block &) const override { return mask_tail_octets == 0; }
 
     DataTypePtr getReturnTypeImpl(const DataTypes & arguments) const override
     {
@@ -447,7 +447,7 @@ public:
     String getName() const override { return name; }
 
     size_t getNumberOfArguments() const override { return 1; }
-    bool isInjective(const Block &) override { return true; }
+    bool isInjective(const Block &) const override { return true; }
 
     DataTypePtr getReturnTypeImpl(const DataTypes & arguments) const override
     {
@@ -546,7 +546,7 @@ public:
     }
 
     size_t getNumberOfArguments() const override { return 1; }
-    bool isInjective(const Block &) override { return true; }
+    bool isInjective(const Block &) const override { return true; }
 
     DataTypePtr getReturnTypeImpl(const DataTypes & arguments) const override
     {
@@ -739,7 +739,7 @@ public:
     }
 
     size_t getNumberOfArguments() const override { return 1; }
-    bool isInjective(const Block &) override { return true; }
+    bool isInjective(const Block &) const override { return true; }
 
     DataTypePtr getReturnTypeImpl(const DataTypes & arguments) const override
     {
@@ -837,7 +837,7 @@ public:
     }
 
     size_t getNumberOfArguments() const override { return 1; }
-    bool isInjective(const Block &) override { return true; }
+    bool isInjective(const Block &) const override { return true; }
 
     DataTypePtr getReturnTypeImpl(const DataTypes & arguments) const override
     {
@@ -941,7 +941,7 @@ public:
     }
 
     size_t getNumberOfArguments() const override { return 1; }
-    bool isInjective(const Block &) override { return true; }
+    bool isInjective(const Block &) const override { return true; }
 
     DataTypePtr getReturnTypeImpl(const DataTypes & arguments) const override
     {
@@ -1224,7 +1224,7 @@ public:
     }
 
     size_t getNumberOfArguments() const override { return 1; }
-    bool isInjective(const Block &) override { return true; }
+    bool isInjective(const Block &) const override { return true; }
 
     DataTypePtr getReturnTypeImpl(const DataTypes & arguments) const override
     {
@@ -1313,7 +1313,7 @@ public:
     }
 
     bool isVariadic() const override { return true; }
-    bool isInjective(const Block &) override { return true; }
+    bool isInjective(const Block &) const override { return true; }
     size_t getNumberOfArguments() const override { return 0; }
 
     DataTypePtr getReturnTypeImpl(const DataTypes & arguments) const override
@@ -1408,7 +1408,7 @@ public:
     }
 
     size_t getNumberOfArguments() const override { return 1; }
-    bool isInjective(const Block &) override { return true; }
+    bool isInjective(const Block &) const override { return true; }
 
     DataTypePtr getReturnTypeImpl(const DataTypes & arguments) const override
     {
diff --git a/src/Functions/FunctionsConversion.h b/src/Functions/FunctionsConversion.h
index b493aef4cac..64708f45598 100644
--- a/src/Functions/FunctionsConversion.h
+++ b/src/Functions/FunctionsConversion.h
@@ -913,7 +913,7 @@ public:
 
     bool isVariadic() const override { return true; }
     size_t getNumberOfArguments() const override { return 0; }
-    bool isInjective(const Block &) override { return std::is_same_v<Name, NameToString>; }
+    bool isInjective(const Block &) const override { return std::is_same_v<Name, NameToString>; }
 
     DataTypePtr getReturnTypeImpl(const ColumnsWithTypeAndName & arguments) const override
     {
@@ -1268,7 +1268,7 @@ public:
     }
 
     size_t getNumberOfArguments() const override { return 2; }
-    bool isInjective(const Block &) override { return true; }
+    bool isInjective(const Block &) const override { return true; }
 
     DataTypePtr getReturnTypeImpl(const ColumnsWithTypeAndName & arguments) const override
     {
diff --git a/src/Functions/FunctionsEmbeddedDictionaries.h b/src/Functions/FunctionsEmbeddedDictionaries.h
index 12b478a26b6..86e8a8016f4 100644
--- a/src/Functions/FunctionsEmbeddedDictionaries.h
+++ b/src/Functions/FunctionsEmbeddedDictionaries.h
@@ -589,7 +589,7 @@ public:
 
     /// For the purpose of query optimization, we assume this function to be injective
     ///  even in face of fact that there are many different cities named Moscow.
-    bool isInjective(const Block &) override { return true; }
+    bool isInjective(const Block &) const override { return true; }
 
     DataTypePtr getReturnTypeImpl(const DataTypes & arguments) const override
     {
diff --git a/src/Functions/FunctionsExternalDictionaries.h b/src/Functions/FunctionsExternalDictionaries.h
index fc3c2c583a9..dcb3572b874 100644
--- a/src/Functions/FunctionsExternalDictionaries.h
+++ b/src/Functions/FunctionsExternalDictionaries.h
@@ -241,7 +241,7 @@ private:
     bool useDefaultImplementationForConstants() const final { return true; }
     ColumnNumbers getArgumentsThatAreAlwaysConstant() const final { return {0, 1}; }
 
-    bool isInjective(const Block & sample_block) override
+    bool isInjective(const Block & sample_block) const override
     {
         return isDictGetFunctionInjective(dictionaries_loader, sample_block);
     }
@@ -763,7 +763,7 @@ private:
     bool useDefaultImplementationForConstants() const final { return true; }
     ColumnNumbers getArgumentsThatAreAlwaysConstant() const final { return {0, 1}; }
 
-    bool isInjective(const Block & sample_block) override
+    bool isInjective(const Block & sample_block) const override
     {
         return isDictGetFunctionInjective(dictionaries_loader, sample_block);
     }
@@ -1328,7 +1328,7 @@ private:
     bool useDefaultImplementationForConstants() const final { return true; }
     ColumnNumbers getArgumentsThatAreAlwaysConstant() const final { return {0, 1}; }
 
-    bool isInjective(const Block & sample_block) override
+    bool isInjective(const Block & sample_block) const override
     {
         return isDictGetFunctionInjective(dictionaries_loader, sample_block);
     }
@@ -1476,7 +1476,7 @@ private:
     bool useDefaultImplementationForConstants() const final { return true; }
     ColumnNumbers getArgumentsThatAreAlwaysConstant() const final { return {0, 1}; }
 
-    bool isInjective(const Block & sample_block) override
+    bool isInjective(const Block & sample_block) const override
     {
         return isDictGetFunctionInjective(dictionaries_loader, sample_block);
     }
@@ -1617,7 +1617,7 @@ public:
 
 private:
     size_t getNumberOfArguments() const override { return 2; }
-    bool isInjective(const Block & /*sample_block*/) override { return true; }
+    bool isInjective(const Block & /*sample_block*/) const override { return true; }
 
     bool useDefaultImplementationForConstants() const final { return true; }
     ColumnNumbers getArgumentsThatAreAlwaysConstant() const final { return {0}; }
diff --git a/src/Functions/FunctionsFormatting.h b/src/Functions/FunctionsFormatting.h
index 0a789b1223a..a5c5635e0ad 100644
--- a/src/Functions/FunctionsFormatting.h
+++ b/src/Functions/FunctionsFormatting.h
@@ -42,7 +42,7 @@ public:
     }
 
     size_t getNumberOfArguments() const override { return 1; }
-    bool isInjective(const Block &) override { return true; }
+    bool isInjective(const Block &) const override { return true; }
 
     DataTypePtr getReturnTypeImpl(const DataTypes & arguments) const override
     {
diff --git a/src/Functions/IFunction.h b/src/Functions/IFunction.h
index 3887f3f1669..b2dcf0ad98c 100644
--- a/src/Functions/IFunction.h
+++ b/src/Functions/IFunction.h
@@ -131,7 +131,7 @@ public:
       *
       * sample_block should contain data types of arguments and values of constants, if relevant.
       */
-    virtual bool isInjective(const Block & /*sample_block*/) { return false; }
+    virtual bool isInjective(const Block & /*sample_block*/) const { return false; }
 
     /** Function is called "deterministic", if it returns same result for same values of arguments.
       * Most of functions are deterministic. Notable counterexample is rand().
diff --git a/src/Functions/IFunctionAdaptors.h b/src/Functions/IFunctionAdaptors.h
index 123faa859e9..9291e721a91 100644
--- a/src/Functions/IFunctionAdaptors.h
+++ b/src/Functions/IFunctionAdaptors.h
@@ -68,7 +68,7 @@ public:
         return impl->getResultIfAlwaysReturnsConstantAndHasArguments(block, arguments);
     }
 
-    bool isInjective(const Block & sample_block) final { return impl->isInjective(sample_block); }
+    bool isInjective(const Block & sample_block) const final { return impl->isInjective(sample_block); }
     bool isDeterministic() const final { return impl->isDeterministic(); }
     bool isDeterministicInScopeOfQuery() const final { return impl->isDeterministicInScopeOfQuery(); }
     bool hasInformationAboutMonotonicity() const final { return impl->hasInformationAboutMonotonicity(); }
@@ -195,7 +195,7 @@ public:
 
     bool isStateful() const override { return function->isStateful(); }
 
-    bool isInjective(const Block & sample_block) override { return function->isInjective(sample_block); }
+    bool isInjective(const Block & sample_block) const override { return function->isInjective(sample_block); }
 
     bool isDeterministic() const override { return function->isDeterministic(); }
 
diff --git a/src/Functions/IFunctionImpl.h b/src/Functions/IFunctionImpl.h
index 66196070afe..12461bbf402 100644
--- a/src/Functions/IFunctionImpl.h
+++ b/src/Functions/IFunctionImpl.h
@@ -107,7 +107,7 @@ public:
     virtual bool isSuitableForConstantFolding() const { return true; }
     virtual ColumnPtr getResultIfAlwaysReturnsConstantAndHasArguments(const Block & /*block*/, const ColumnNumbers & /*arguments*/) const { return nullptr; }
 
-    virtual bool isInjective(const Block & /*sample_block*/) { return false; }
+    virtual bool isInjective(const Block & /*sample_block*/) const { return false; }
     virtual bool isDeterministic() const { return true; }
     virtual bool isDeterministicInScopeOfQuery() const { return true; }
     virtual bool hasInformationAboutMonotonicity() const { return false; }
@@ -256,7 +256,7 @@ public:
     /// Properties from IFunctionBase (see IFunction.h)
     virtual bool isSuitableForConstantFolding() const { return true; }
     virtual ColumnPtr getResultIfAlwaysReturnsConstantAndHasArguments(const Block & /*block*/, const ColumnNumbers & /*arguments*/) const { return nullptr; }
-    virtual bool isInjective(const Block & /*sample_block*/) { return false; }
+    virtual bool isInjective(const Block & /*sample_block*/) const { return false; }
     virtual bool isDeterministic() const { return true; }
     virtual bool isDeterministicInScopeOfQuery() const { return true; }
     virtual bool isStateful() const { return false; }
diff --git a/src/Functions/concat.cpp b/src/Functions/concat.cpp
index 7cf0b2ce891..3c204f098c1 100644
--- a/src/Functions/concat.cpp
+++ b/src/Functions/concat.cpp
@@ -41,7 +41,7 @@ public:
 
     size_t getNumberOfArguments() const override { return 0; }
 
-    bool isInjective(const Block &) override { return is_injective; }
+    bool isInjective(const Block &) const override { return is_injective; }
 
     bool useDefaultImplementationForConstants() const override { return true; }
 
diff --git a/src/Functions/reverse.cpp b/src/Functions/reverse.cpp
index 2c135cf3d7d..bc1237fc457 100644
--- a/src/Functions/reverse.cpp
+++ b/src/Functions/reverse.cpp
@@ -71,7 +71,7 @@ public:
         return 1;
     }
 
-    bool isInjective(const Block &) override
+    bool isInjective(const Block &) const override
     {
         return true;
     }
diff --git a/src/Functions/tuple.cpp b/src/Functions/tuple.cpp
index 451f732c869..772cb4e3c07 100644
--- a/src/Functions/tuple.cpp
+++ b/src/Functions/tuple.cpp
@@ -43,7 +43,7 @@ public:
         return 0;
     }
 
-    bool isInjective(const Block &) override
+    bool isInjective(const Block &) const override
     {
         return true;
     }
diff --git a/src/Interpreters/ExpressionJIT.cpp b/src/Interpreters/ExpressionJIT.cpp
index 1f6ac0a9926..d47335eb1ee 100644
--- a/src/Interpreters/ExpressionJIT.cpp
+++ b/src/Interpreters/ExpressionJIT.cpp
@@ -510,7 +510,7 @@ bool LLVMFunction::isSuitableForConstantFolding() const
     return true;
 }
 
-bool LLVMFunction::isInjective(const Block & sample_block)
+bool LLVMFunction::isInjective(const Block & sample_block) const
 {
     for (const auto & f : originals)
         if (!f->isInjective(sample_block))
diff --git a/src/Interpreters/ExpressionJIT.h b/src/Interpreters/ExpressionJIT.h
index 995fb35e52c..dd486c04c5d 100644
--- a/src/Interpreters/ExpressionJIT.h
+++ b/src/Interpreters/ExpressionJIT.h
@@ -51,7 +51,7 @@ public:
 
     bool isSuitableForConstantFolding() const override;
 
-    bool isInjective(const Block & sample_block) override;
+    bool isInjective(const Block & sample_block) const override;
 
     bool hasInformationAboutMonotonicity() const override;
 

From 322681eb37f84364ae3410052fc5ac48b8675465 Mon Sep 17 00:00:00 2001
From: Azat Khuzhin <a3at.mail@gmail.com>
Date: Fri, 17 Apr 2020 01:24:51 +0300
Subject: [PATCH 2/3] Passthrough isInjective via IFunctionOverloadResolver

---
 src/Functions/IFunction.h         | 1 +
 src/Functions/IFunctionAdaptors.h | 3 +++
 src/Functions/IFunctionImpl.h     | 1 +
 3 files changed, 5 insertions(+)

diff --git a/src/Functions/IFunction.h b/src/Functions/IFunction.h
index b2dcf0ad98c..0bd52e835cc 100644
--- a/src/Functions/IFunction.h
+++ b/src/Functions/IFunction.h
@@ -186,6 +186,7 @@ public:
     /// See the comment for the same method in IFunctionBase
     virtual bool isDeterministic() const = 0;
     virtual bool isDeterministicInScopeOfQuery() const = 0;
+    virtual bool isInjective(const Block &) const = 0;
 
     /// Override and return true if function needs to depend on the state of the data.
     virtual bool isStateful() const = 0;
diff --git a/src/Functions/IFunctionAdaptors.h b/src/Functions/IFunctionAdaptors.h
index 9291e721a91..82afaad4c27 100644
--- a/src/Functions/IFunctionAdaptors.h
+++ b/src/Functions/IFunctionAdaptors.h
@@ -96,6 +96,8 @@ public:
 
     bool isDeterministicInScopeOfQuery() const final { return impl->isDeterministicInScopeOfQuery(); }
 
+    bool isInjective(const Block & block) const final { return impl->isInjective(block); }
+
     bool isStateful() const final { return impl->isStateful(); }
 
     bool isVariadic() const final { return impl->isVariadic(); }
@@ -226,6 +228,7 @@ public:
 
     bool isDeterministic() const override { return function->isDeterministic(); }
     bool isDeterministicInScopeOfQuery() const override { return function->isDeterministicInScopeOfQuery(); }
+    bool isInjective(const Block &block) const override { return function->isInjective(block); }
 
     String getName() const override { return function->getName(); }
     bool isStateful() const override { return function->isStateful(); }
diff --git a/src/Functions/IFunctionImpl.h b/src/Functions/IFunctionImpl.h
index 12461bbf402..116363705de 100644
--- a/src/Functions/IFunctionImpl.h
+++ b/src/Functions/IFunctionImpl.h
@@ -152,6 +152,7 @@ public:
     /// Properties from IFunctionOverloadResolver. See comments in IFunction.h
     virtual bool isDeterministic() const { return true; }
     virtual bool isDeterministicInScopeOfQuery() const { return true; }
+    virtual bool isInjective(const Block &) const { return false; }
     virtual bool isStateful() const { return false; }
     virtual bool isVariadic() const { return false; }
 

From 548399725fa5a014f26d8cc34d51554d91584045 Mon Sep 17 00:00:00 2001
From: Azat Khuzhin <a3at.mail@gmail.com>
Date: Sat, 18 Apr 2020 02:25:14 +0300
Subject: [PATCH 3/3] Use IFunction::isInjective() over manual list of such
 functions for GROUP BY optimization

---
 src/Interpreters/SyntaxAnalyzer.cpp | 28 +++++-----------------------
 1 file changed, 5 insertions(+), 23 deletions(-)

diff --git a/src/Interpreters/SyntaxAnalyzer.cpp b/src/Interpreters/SyntaxAnalyzer.cpp
index e19961e7a7c..223b7b459c1 100644
--- a/src/Interpreters/SyntaxAnalyzer.cpp
+++ b/src/Interpreters/SyntaxAnalyzer.cpp
@@ -33,6 +33,8 @@
 #include <Parsers/parseQuery.h>
 #include <Parsers/queryToString.h>
 
+#include <Functions/FunctionFactory.h>
+
 #include <DataTypes/NestedUtils.h>
 #include <DataTypes/DataTypeNullable.h>
 
@@ -216,28 +218,6 @@ void executeScalarSubqueries(ASTPtr & query, const Context & context, size_t sub
     ExecuteScalarSubqueriesVisitor(visitor_data, log.stream()).visit(query);
 }
 
-/** Calls to these functions in the GROUP BY statement would be
-  * replaced by their immediate argument.
-  */
-const std::unordered_set<String> injective_function_names
-{
-        "negate",
-        "bitNot",
-        "reverse",
-        "reverseUTF8",
-        "toString",
-        "toFixedString",
-        "IPv4NumToString",
-        "IPv4StringToNum",
-        "hex",
-        "unhex",
-        "bitmaskToList",
-        "bitmaskToArray",
-        "tuple",
-        "regionToName",
-        "concatAssumeInjective",
-};
-
 const std::unordered_set<String> possibly_injective_function_names
 {
         "dictGetString",
@@ -278,6 +258,8 @@ void appendUnusedGroupByColumn(ASTSelectQuery * select_query, const NameSet & so
 /// Eliminates injective function calls and constant expressions from group by statement.
 void optimizeGroupBy(ASTSelectQuery * select_query, const NameSet & source_columns, const Context & context)
 {
+    const FunctionFactory & function_factory = FunctionFactory::instance();
+
     if (!select_query->groupBy())
     {
         // If there is a HAVING clause without GROUP BY, make sure we have some aggregation happen.
@@ -327,7 +309,7 @@ void optimizeGroupBy(ASTSelectQuery * select_query, const NameSet & source_colum
                     continue;
                 }
             }
-            else if (!injective_function_names.count(function->name))
+            else if (!function_factory.get(function->name, context)->isInjective(Block{}))
             {
                 ++i;
                 continue;