Merge pull request #10257 from 4ertus2/strange

Some tech debt

src/IO/readDecimalText.h

@@ -10,7 +10,10 @@ namespace ErrorCodes
     extern const int ARGUMENT_OUT_OF_BOUND;
 }
 
-
+/// Try to read Decimal into underlying type T from ReadBuffer. Throws if 'digits_only' is set and there's unexpected symbol in input.
+/// Returns integer 'exponent' factor that x should be muntiplyed by to get correct Decimal value: result = x * 10^exponent.
+/// Use 'digits' input as max allowed meaning decimal digits in result. Place actual meanin digits in 'digits' output.
+/// Do not care about decimal scale, only about meaning digits in decimal text representation.
 template <bool _throw_on_error, typename T>
 inline bool readDigits(ReadBuffer & buf, T & x, unsigned int & digits, int & exponent, bool digits_only = false)
 {

src/Interpreters/DatabaseAndTableWithAlias.h

@@ -49,61 +49,51 @@ struct TableWithColumnNames
 {
     DatabaseAndTableWithAlias table;
     Names columns;
-    Names hidden_columns;
+    Names hidden_columns; /// Not general columns like MATERIALIZED and ALIAS. They are omitted in * and t.* results.
 
     TableWithColumnNames(const DatabaseAndTableWithAlias & table_, const Names & columns_)
         : table(table_)
         , columns(columns_)
-    {}
+    {
+        columns_set.insert(columns.begin(), columns.end());
+    }
 
     TableWithColumnNames(const DatabaseAndTableWithAlias table_, Names && columns_, Names && hidden_columns_)
         : table(table_)
         , columns(columns_)
         , hidden_columns(hidden_columns_)
-    {}
-
-    bool hasColumn(const String & name) const
     {
-        if (columns_set.empty())
-        {
-            columns_set.insert(columns.begin(), columns.end());
-            columns_set.insert(hidden_columns.begin(), hidden_columns.end());
-        }
-
-        return columns_set.count(name);
+        columns_set.insert(columns.begin(), columns.end());
+        columns_set.insert(hidden_columns.begin(), hidden_columns.end());
     }
 
+    bool hasColumn(const String & name) const { return columns_set.count(name); }
+
 private:
-    mutable NameSet columns_set;
+    NameSet columns_set;
 };
 
 struct TableWithColumnNamesAndTypes
 {
     DatabaseAndTableWithAlias table;
     NamesAndTypesList columns;
-    NamesAndTypesList hidden_columns;
+    NamesAndTypesList hidden_columns; /// Not general columns like MATERIALIZED and ALIAS. They are omitted in * and t.* results.
 
     TableWithColumnNamesAndTypes(const DatabaseAndTableWithAlias & table_, const NamesAndTypesList & columns_)
         : table(table_)
         , columns(columns_)
-    {}
-
-    bool hasColumn(const String & name) const
     {
-        if (names.empty())
-        {
-            for (auto & col : columns)
-                names.insert(col.name);
-            for (auto & col : hidden_columns)
-                names.insert(col.name);
-        }
-
-        return names.count(name);
+        for (auto & col : columns)
+            names.insert(col.name);
     }
 
+    bool hasColumn(const String & name) const { return names.count(name); }
+
     void addHiddenColumns(const NamesAndTypesList & addition)
     {
         hidden_columns.insert(hidden_columns.end(), addition.begin(), addition.end());
+        for (auto & col : addition)
+            names.insert(col.name);
     }
 
     TableWithColumnNames removeTypes() const
@@ -122,7 +112,7 @@ struct TableWithColumnNamesAndTypes
     }
 
 private:
-    mutable NameSet names;
+    NameSet names;
 };
 
 std::vector<DatabaseAndTableWithAlias> getDatabaseAndTables(const ASTSelectQuery & select_query, const String & current_database);

src/Interpreters/JoinedTables.h

@@ -27,6 +27,8 @@ public:
 
     StoragePtr getLeftTableStorage();
     bool resolveTables();
+
+    /// Make fake tables_with_columns[0] in case we have predefined input in InterpreterSelectQuery
     void makeFakeTable(StoragePtr storage, const Block & source_header);
 
     const std::vector<TableWithColumnNamesAndTypes> & tablesWithColumns() const { return tables_with_columns; }

src/Interpreters/ColumnNamesContext.cpp → src/Interpreters/RequiredSourceColumnsData.cpp

-#include <Interpreters/ColumnNamesContext.h>
+#include <Common/typeid_cast.h>
+#include <Interpreters/RequiredSourceColumnsData.h>
 #include <Interpreters/IdentifierSemantic.h>
 #include <DataTypes/NestedUtils.h>
+#include <Parsers/ASTIdentifier.h>
 
 namespace DB
 {
 
-bool ColumnNamesContext::addTableAliasIfAny(const IAST & ast)
-{
-    String alias = ast.tryGetAlias();
-    if (alias.empty())
-        return false;
-
-    table_aliases.insert(alias);
-    return true;
-}
-
-bool ColumnNamesContext::addColumnAliasIfAny(const IAST & ast)
+bool RequiredSourceColumnsData::addColumnAliasIfAny(const IAST & ast)
 {
     String alias = ast.tryGetAlias();
     if (alias.empty())
@@ -28,7 +20,7 @@ bool ColumnNamesContext::addColumnAliasIfAny(const IAST & ast)
     return true;
 }
 
-void ColumnNamesContext::addColumnIdentifier(const ASTIdentifier & node)
+void RequiredSourceColumnsData::addColumnIdentifier(const ASTIdentifier & node)
 {
     if (!IdentifierSemantic::getColumnName(node))
         return;
@@ -38,7 +30,7 @@ void ColumnNamesContext::addColumnIdentifier(const ASTIdentifier & node)
     required_names[node.name].addInclusion(alias);
 }
 
-bool ColumnNamesContext::addArrayJoinAliasIfAny(const IAST & ast)
+bool RequiredSourceColumnsData::addArrayJoinAliasIfAny(const IAST & ast)
 {
     String alias = ast.tryGetAlias();
     if (alias.empty())
@@ -48,12 +40,12 @@ bool ColumnNamesContext::addArrayJoinAliasIfAny(const IAST & ast)
     return true;
 }
 
-void ColumnNamesContext::addArrayJoinIdentifier(const ASTIdentifier & node)
+void RequiredSourceColumnsData::addArrayJoinIdentifier(const ASTIdentifier & node)
 {
     array_join_columns.insert(node.name);
 }
 
-size_t ColumnNamesContext::nameInclusion(const String & name) const
+size_t RequiredSourceColumnsData::nameInclusion(const String & name) const
 {
     auto it = required_names.find(name);
     if (it != required_names.end())
@@ -61,7 +53,7 @@ size_t ColumnNamesContext::nameInclusion(const String & name) const
     return 0;
 }
 
-NameSet ColumnNamesContext::requiredColumns() const
+NameSet RequiredSourceColumnsData::requiredColumns() const
 {
     NameSet required;
     for (const auto & pr : required_names)
@@ -79,7 +71,7 @@ NameSet ColumnNamesContext::requiredColumns() const
     return required;
 }
 
-std::ostream & operator << (std::ostream & os, const ColumnNamesContext & cols)
+std::ostream & operator << (std::ostream & os, const RequiredSourceColumnsData & cols)
 {
     os << "required_names: ";
     for (const auto & pr : cols.required_names)
@@ -89,21 +81,6 @@ std::ostream & operator << (std::ostream & os, const ColumnNamesContext & cols)
             os << "/'" << alias << "'";
         os << ", ";
     }
-    os << "source_tables: ";
-    for (const auto & x : cols.tables)
-    {
-        auto alias = x.alias();
-        auto name = x.name();
-        if (alias && name)
-            os << "'" << *alias << "'/'" << *name << "', ";
-        else if (alias)
-            os << "'" << *alias << "', ";
-        else if (name)
-            os << "'" << *name << "', ";
-    }
-    os << "table_aliases: ";
-    for (const auto & x : cols.table_aliases)
-        os << "'" << x << "', ";
     os << "complex_aliases: ";
     for (const auto & x : cols.complex_aliases)
         os << "'" << x << "', ";

src/Interpreters/ColumnNamesContext.h → src/Interpreters/RequiredSourceColumnsData.h

@@ -3,54 +3,18 @@
 #include <ostream>
 #include <optional>
 
-#include <Common/typeid_cast.h>
 #include <Core/Names.h>
-#include <Parsers/ASTIdentifier.h>
-#include <Parsers/ASTTablesInSelectQuery.h>
+#include <Core/Types.h>
+#include <Parsers/IAST_fwd.h>
 
 namespace DB
 {
 
+class ASTIdentifier;
+
 /// Information about table and column names extracted from ASTSelectQuery block. Do not include info from subselects.
-struct ColumnNamesContext
+struct RequiredSourceColumnsData
 {
-    struct JoinedTable
-    {
-        const ASTTableExpression * expr = nullptr;
-        const ASTTableJoin * join = nullptr;
-
-        std::optional<String> alias() const
-        {
-            String alias;
-            if (expr)
-            {
-                if (expr->database_and_table_name)
-                    alias = expr->database_and_table_name->tryGetAlias();
-                else if (expr->table_function)
-                    alias = expr->table_function->tryGetAlias();
-                else if (expr->subquery)
-                    alias = expr->subquery->tryGetAlias();
-            }
-            if (!alias.empty())
-                return alias;
-            return {};
-        }
-
-        std::optional<String> name() const
-        {
-            if (expr)
-                return tryGetIdentifierName(expr->database_and_table_name);
-            return {};
-        }
-
-        std::optional<ASTTableJoin::Kind> joinKind() const
-        {
-            if (join)
-                return join->kind;
-            return {};
-        }
-    };
-
     struct NameInfo
     {
         std::set<String> aliases;
@@ -65,16 +29,14 @@ struct ColumnNamesContext
     };
 
     std::unordered_map<String, NameInfo> required_names;
-    NameSet table_aliases;
-    NameSet private_aliases;
-    NameSet complex_aliases;
-    NameSet masked_columns;
-    NameSet array_join_columns;
-    std::vector<JoinedTable> tables; /// ordered list of visited tables in FROM section with joins
+    NameSet private_aliases; /// lambda aliases that should not be interpreted as required columns
+    NameSet complex_aliases; /// aliases to functions results: they are not required cause calculated by query itself
+    NameSet masked_columns;  /// columns names masked by function aliases: we still need them in required columns
+    NameSet array_join_columns; /// Tech debt: we exclude ArrayJoin columns from general logic cause they have own logic outside
+
     bool has_table_join = false;
     bool has_array_join = false;
 
-    bool addTableAliasIfAny(const IAST & ast);
     bool addColumnAliasIfAny(const IAST & ast);
     void addColumnIdentifier(const ASTIdentifier & node);
     bool addArrayJoinAliasIfAny(const IAST & ast);
@@ -84,6 +46,6 @@ struct ColumnNamesContext
     size_t nameInclusion(const String & name) const;
 };
 
-std::ostream & operator << (std::ostream & os, const ColumnNamesContext & cols);
+std::ostream & operator << (std::ostream & os, const RequiredSourceColumnsData & cols);
 
 }

src/Interpreters/RequiredSourceColumnsVisitor.cpp

@@ -91,14 +91,12 @@ void RequiredSourceColumnsMatcher::visit(const ASTPtr & ast, Data & data)
 
     if (auto * t = ast->as<ASTSelectQuery>())
     {
-        data.addTableAliasIfAny(*ast);
         visit(*t, ast, data);
         return;
     }
 
     if (ast->as<ASTSubquery>())
     {
-        data.addTableAliasIfAny(*ast);
         return;
     }
 
@@ -161,33 +159,14 @@ void RequiredSourceColumnsMatcher::visit(const ASTFunction & node, const ASTPtr
 
 void RequiredSourceColumnsMatcher::visit(const ASTTablesInSelectQueryElement & node, const ASTPtr &, Data & data)
 {
-    ASTTableExpression * expr = nullptr;
-    ASTTableJoin * join = nullptr;
-
     for (auto & child : node.children)
-    {
-        if (auto * e = child->as<ASTTableExpression>())
-            expr = e;
-        if (auto * j = child->as<ASTTableJoin>())
-            join = j;
-    }
-
-    if (join)
-        data.has_table_join = true;
-    data.tables.emplace_back(ColumnNamesContext::JoinedTable{expr, join});
+        if (child->as<ASTTableJoin>())
+            data.has_table_join = true;
 }
 
 /// ASTIdentifiers here are tables. Do not visit them as generic ones.
-void RequiredSourceColumnsMatcher::visit(const ASTTableExpression & node, const ASTPtr &, Data & data)
+void RequiredSourceColumnsMatcher::visit(const ASTTableExpression &, const ASTPtr &, Data &)
 {
-    if (node.database_and_table_name)
-        data.addTableAliasIfAny(*node.database_and_table_name);
-
-    if (node.table_function)
-        data.addTableAliasIfAny(*node.table_function);
-
-    if (node.subquery)
-        data.addTableAliasIfAny(*node.subquery);
 }
 
 void RequiredSourceColumnsMatcher::visit(const ASTArrayJoin & node, const ASTPtr &, Data & data)

src/Interpreters/RequiredSourceColumnsVisitor.h

 #pragma once
 
-#include <Interpreters/ColumnNamesContext.h>
+#include <Interpreters/RequiredSourceColumnsData.h>
 #include <Interpreters/InDepthNodeVisitor.h>
 
 namespace DB
@@ -21,7 +21,7 @@ class RequiredSourceColumnsMatcher
 {
 public:
     using Visitor = ConstInDepthNodeVisitor<RequiredSourceColumnsMatcher, false>;
-    using Data = ColumnNamesContext;
+    using Data = RequiredSourceColumnsData;
 
     static bool needChildVisit(const ASTPtr & node, const ASTPtr & child);
     static void visit(const ASTPtr & ast, Data & data);
@@ -35,7 +35,7 @@ private:
     static void visit(const ASTSelectQuery & select, const ASTPtr &, Data & data);
 };
 
-/// Extracts all the information about columns and tables from ASTSelectQuery block into ColumnNamesContext object.
+/// Extracts all the information about columns and tables from ASTSelectQuery block into Data object.
 /// It doesn't use anything but AST. It visits nodes from bottom to top except ASTFunction content to get aliases in right manner.
 /// @note There's some ambiguousness with nested columns names that can't be solved without schema.
 using RequiredSourceColumnsVisitor = RequiredSourceColumnsMatcher::Visitor;

src/Parsers/ASTIdentifier.h

@@ -40,6 +40,8 @@ public:
     bool isShort() const { return name_parts.empty() || name == name_parts.back(); }
 
     void setShortName(const String & new_name);
+
+    /// Restore name field from name_parts in case it was cropped by analyzer but we need a full form for future (re)analyze.
     void restoreCompoundName();
 
     const String & shortName() const