From 333ec2e496f4fe4addd22d5fa7c5c2a499099c57 Mon Sep 17 00:00:00 2001 From: Amos Bird Date: Thu, 20 Aug 2020 10:01:40 +0800 Subject: [PATCH] fix scalar subquery hash conflicts --- src/Parsers/ASTColumnsMatcher.cpp | 8 ++++ src/Parsers/ASTColumnsMatcher.h | 1 + src/Parsers/ASTEnumElement.h | 39 ------------------- src/Parsers/ASTFunction.cpp | 9 +++++ src/Parsers/ASTFunction.h | 2 + .../ASTFunctionWithKeyValueArguments.cpp | 20 ++++++++++ .../ASTFunctionWithKeyValueArguments.h | 4 ++ src/Parsers/ASTIdentifier.cpp | 6 +++ src/Parsers/ASTIdentifier.h | 2 + src/Parsers/ASTOrderByElement.cpp | 10 +++++ src/Parsers/ASTOrderByElement.h | 2 + src/Parsers/ASTSelectQuery.cpp | 11 ++++++ src/Parsers/ASTSelectQuery.h | 1 + src/Parsers/ASTSetQuery.cpp | 12 ++++++ src/Parsers/ASTSetQuery.h | 2 + src/Parsers/ASTTablesInSelectQuery.cpp | 22 +++++++++++ src/Parsers/ASTTablesInSelectQuery.h | 3 ++ .../01051_scalar_optimization.reference | 2 + .../0_stateless/01051_scalar_optimization.sql | 6 +++ 19 files changed, 123 insertions(+), 39 deletions(-) delete mode 100644 src/Parsers/ASTEnumElement.h create mode 100644 tests/queries/0_stateless/01051_scalar_optimization.reference create mode 100644 tests/queries/0_stateless/01051_scalar_optimization.sql diff --git a/src/Parsers/ASTColumnsMatcher.cpp b/src/Parsers/ASTColumnsMatcher.cpp index a5ec694daee..b6eb4889a09 100644 --- a/src/Parsers/ASTColumnsMatcher.cpp +++ b/src/Parsers/ASTColumnsMatcher.cpp @@ -2,6 +2,7 @@ #include #include #include +#include namespace DB @@ -20,6 +21,13 @@ ASTPtr ASTColumnsMatcher::clone() const void ASTColumnsMatcher::appendColumnName(WriteBuffer & ostr) const { writeString(original_pattern, ostr); } +void ASTColumnsMatcher::updateTreeHashImpl(SipHash & hash_state) const +{ + hash_state.update(original_pattern.size()); + hash_state.update(original_pattern); + IAST::updateTreeHashImpl(hash_state); +} + void ASTColumnsMatcher::formatImpl(const FormatSettings & settings, FormatState &, FormatStateStacked) const { settings.ostr << (settings.hilite ? hilite_keyword : "") << "COLUMNS" << (settings.hilite ? hilite_none : "") << "(" diff --git a/src/Parsers/ASTColumnsMatcher.h b/src/Parsers/ASTColumnsMatcher.h index 91c8ed68edc..3fa85769712 100644 --- a/src/Parsers/ASTColumnsMatcher.h +++ b/src/Parsers/ASTColumnsMatcher.h @@ -33,6 +33,7 @@ public: void appendColumnName(WriteBuffer & ostr) const override; void setPattern(String pattern); bool isColumnMatching(const String & column_name) const; + void updateTreeHashImpl(SipHash & hash_state) const override; protected: void formatImpl(const FormatSettings & settings, FormatState &, FormatStateStacked) const override; diff --git a/src/Parsers/ASTEnumElement.h b/src/Parsers/ASTEnumElement.h deleted file mode 100644 index c603f5086de..00000000000 --- a/src/Parsers/ASTEnumElement.h +++ /dev/null @@ -1,39 +0,0 @@ -#pragma once - -#include "IAST.h" -#include -#include - - -namespace DB -{ - - -class ASTEnumElement : public IAST -{ -public: - String name; - Field value; - - ASTEnumElement(const String & name, const Field & value) - : name{name}, value {value} {} - - String getID(char) const override { return "EnumElement"; } - - ASTPtr clone() const override - { - return std::make_shared(name, value); - } - -protected: - void formatImpl(const FormatSettings & settings, FormatState & state, FormatStateStacked frame) const override - { - frame.need_parens = false; - - const std::string indent_str = settings.one_line ? "" : std::string(4 * frame.indent, ' '); - settings.ostr << settings.nl_or_ws << indent_str << '\'' << name << "' = " << applyVisitor(FieldVisitorToString{}, value); - } -}; - - -} diff --git a/src/Parsers/ASTFunction.cpp b/src/Parsers/ASTFunction.cpp index ec46eb4ac37..e8e6efc7fd9 100644 --- a/src/Parsers/ASTFunction.cpp +++ b/src/Parsers/ASTFunction.cpp @@ -5,6 +5,7 @@ #include #include #include +#include namespace DB @@ -54,6 +55,14 @@ ASTPtr ASTFunction::clone() const } +void ASTFunction::updateTreeHashImpl(SipHash & hash_state) const +{ + hash_state.update(name.size()); + hash_state.update(name); + IAST::updateTreeHashImpl(hash_state); +} + + /** A special hack. If it's [I]LIKE or NOT [I]LIKE expression and the right hand side is a string literal, * we will highlight unescaped metacharacters % and _ in string literal for convenience. * Motivation: most people are unaware that _ is a metacharacter and forgot to properly escape it with two backslashes. diff --git a/src/Parsers/ASTFunction.h b/src/Parsers/ASTFunction.h index f44eba30ee3..127f50ee586 100644 --- a/src/Parsers/ASTFunction.h +++ b/src/Parsers/ASTFunction.h @@ -23,6 +23,8 @@ public: ASTPtr clone() const override; + void updateTreeHashImpl(SipHash & hash_state) const override; + protected: void formatImplWithoutAlias(const FormatSettings & settings, FormatState & state, FormatStateStacked frame) const override; void appendColumnNameImpl(WriteBuffer & ostr) const override; diff --git a/src/Parsers/ASTFunctionWithKeyValueArguments.cpp b/src/Parsers/ASTFunctionWithKeyValueArguments.cpp index c101353be45..5f1e78b61da 100644 --- a/src/Parsers/ASTFunctionWithKeyValueArguments.cpp +++ b/src/Parsers/ASTFunctionWithKeyValueArguments.cpp @@ -1,6 +1,7 @@ #include #include +#include namespace DB { @@ -35,6 +36,16 @@ void ASTPair::formatImpl(const FormatSettings & settings, FormatState & state, F settings.ostr << (settings.hilite ? hilite_none : ""); } + +void ASTPair::updateTreeHashImpl(SipHash & hash_state) const +{ + hash_state.update(first.size()); + hash_state.update(first); + hash_state.update(second_with_brackets); + IAST::updateTreeHashImpl(hash_state); +} + + String ASTFunctionWithKeyValueArguments::getID(char delim) const { return "FunctionWithKeyValueArguments " + (delim + name); @@ -64,4 +75,13 @@ void ASTFunctionWithKeyValueArguments::formatImpl(const FormatSettings & setting settings.ostr << (settings.hilite ? hilite_none : ""); } + +void ASTFunctionWithKeyValueArguments::updateTreeHashImpl(SipHash & hash_state) const +{ + hash_state.update(name.size()); + hash_state.update(name); + hash_state.update(has_brackets); + IAST::updateTreeHashImpl(hash_state); +} + } diff --git a/src/Parsers/ASTFunctionWithKeyValueArguments.h b/src/Parsers/ASTFunctionWithKeyValueArguments.h index 0e0c31b14e4..786d31d9e35 100644 --- a/src/Parsers/ASTFunctionWithKeyValueArguments.h +++ b/src/Parsers/ASTFunctionWithKeyValueArguments.h @@ -30,6 +30,8 @@ public: ASTPtr clone() const override; void formatImpl(const FormatSettings & settings, FormatState & state, FormatStateStacked frame) const override; + + void updateTreeHashImpl(SipHash & hash_state) const override; }; @@ -58,6 +60,8 @@ public: ASTPtr clone() const override; void formatImpl(const FormatSettings & settings, FormatState & state, FormatStateStacked frame) const override; + + void updateTreeHashImpl(SipHash & hash_state) const override; }; } diff --git a/src/Parsers/ASTIdentifier.cpp b/src/Parsers/ASTIdentifier.cpp index 22c650ede92..9117be46e51 100644 --- a/src/Parsers/ASTIdentifier.cpp +++ b/src/Parsers/ASTIdentifier.cpp @@ -111,6 +111,12 @@ void ASTIdentifier::resetTable(const String & database_name, const String & tabl uuid = ident.uuid; } +void ASTIdentifier::updateTreeHashImpl(SipHash & hash_state) const +{ + hash_state.update(uuid); + IAST::updateTreeHashImpl(hash_state); +} + ASTPtr createTableIdentifier(const String & database_name, const String & table_name) { assert(database_name != "_temporary_and_external_tables"); diff --git a/src/Parsers/ASTIdentifier.h b/src/Parsers/ASTIdentifier.h index 570df953acf..5c06fa7fa38 100644 --- a/src/Parsers/ASTIdentifier.h +++ b/src/Parsers/ASTIdentifier.h @@ -53,6 +53,8 @@ public: void resetTable(const String & database_name, const String & table_name); + void updateTreeHashImpl(SipHash & hash_state) const override; + protected: void formatImplWithoutAlias(const FormatSettings & settings, FormatState & state, FormatStateStacked frame) const override; void appendColumnNameImpl(WriteBuffer & ostr) const override; diff --git a/src/Parsers/ASTOrderByElement.cpp b/src/Parsers/ASTOrderByElement.cpp index b597c98eabe..48290b2669a 100644 --- a/src/Parsers/ASTOrderByElement.cpp +++ b/src/Parsers/ASTOrderByElement.cpp @@ -1,10 +1,20 @@ #include #include +#include namespace DB { +void ASTOrderByElement::updateTreeHashImpl(SipHash & hash_state) const +{ + hash_state.update(direction); + hash_state.update(nulls_direction); + hash_state.update(nulls_direction_was_explicitly_specified); + hash_state.update(with_fill); + IAST::updateTreeHashImpl(hash_state); +} + void ASTOrderByElement::formatImpl(const FormatSettings & settings, FormatState & state, FormatStateStacked frame) const { children.front()->formatImpl(settings, state, frame); diff --git a/src/Parsers/ASTOrderByElement.h b/src/Parsers/ASTOrderByElement.h index 30da8172f52..afeff90741a 100644 --- a/src/Parsers/ASTOrderByElement.h +++ b/src/Parsers/ASTOrderByElement.h @@ -46,6 +46,8 @@ public: return clone; } + void updateTreeHashImpl(SipHash & hash_state) const override; + protected: void formatImpl(const FormatSettings & settings, FormatState & state, FormatStateStacked frame) const override; }; diff --git a/src/Parsers/ASTSelectQuery.cpp b/src/Parsers/ASTSelectQuery.cpp index 9e65543babe..fdc7bd47e4d 100644 --- a/src/Parsers/ASTSelectQuery.cpp +++ b/src/Parsers/ASTSelectQuery.cpp @@ -57,6 +57,17 @@ ASTPtr ASTSelectQuery::clone() const } +void ASTSelectQuery::updateTreeHashImpl(SipHash & hash_state) const +{ + hash_state.update(distinct); + hash_state.update(group_by_with_totals); + hash_state.update(group_by_with_rollup); + hash_state.update(group_by_with_cube); + hash_state.update(limit_with_ties); + IAST::updateTreeHashImpl(hash_state); +} + + void ASTSelectQuery::formatImpl(const FormatSettings & s, FormatState & state, FormatStateStacked frame) const { frame.current_select = this; diff --git a/src/Parsers/ASTSelectQuery.h b/src/Parsers/ASTSelectQuery.h index 480e6bd63e2..9690b51ff2f 100644 --- a/src/Parsers/ASTSelectQuery.h +++ b/src/Parsers/ASTSelectQuery.h @@ -88,6 +88,7 @@ public: void replaceDatabaseAndTable(const String & database_name, const String & table_name); void replaceDatabaseAndTable(const StorageID & table_id); void addTableFunction(ASTPtr & table_function_ptr); + void updateTreeHashImpl(SipHash & hash_state) const override; protected: void formatImpl(const FormatSettings & settings, FormatState & state, FormatStateStacked frame) const override; diff --git a/src/Parsers/ASTSetQuery.cpp b/src/Parsers/ASTSetQuery.cpp index fd5409e4bdd..8835d1dc7da 100644 --- a/src/Parsers/ASTSetQuery.cpp +++ b/src/Parsers/ASTSetQuery.cpp @@ -1,9 +1,21 @@ #include #include +#include namespace DB { + +void ASTSetQuery::updateTreeHashImpl(SipHash & hash_state) const +{ + for (const auto & change : changes) + { + hash_state.update(change.name.size()); + hash_state.update(change.name); + applyVisitor(FieldVisitorHash(hash_state), change.value); + } +} + void ASTSetQuery::formatImpl(const FormatSettings & format, FormatState &, FormatStateStacked) const { if (is_standalone) diff --git a/src/Parsers/ASTSetQuery.h b/src/Parsers/ASTSetQuery.h index e5c3e31b3c2..a91584910bb 100644 --- a/src/Parsers/ASTSetQuery.h +++ b/src/Parsers/ASTSetQuery.h @@ -23,6 +23,8 @@ public: ASTPtr clone() const override { return std::make_shared(*this); } void formatImpl(const FormatSettings & format, FormatState &, FormatStateStacked) const override; + + void updateTreeHashImpl(SipHash & hash_state) const override; }; } diff --git a/src/Parsers/ASTTablesInSelectQuery.cpp b/src/Parsers/ASTTablesInSelectQuery.cpp index 60cb0475be7..0fd93bbd04d 100644 --- a/src/Parsers/ASTTablesInSelectQuery.cpp +++ b/src/Parsers/ASTTablesInSelectQuery.cpp @@ -1,6 +1,7 @@ #include #include #include +#include namespace DB @@ -18,6 +19,13 @@ do \ while (false) +void ASTTableExpression::updateTreeHashImpl(SipHash & hash_state) const +{ + hash_state.update(final); + IAST::updateTreeHashImpl(hash_state); +} + + ASTPtr ASTTableExpression::clone() const { auto res = std::make_shared(*this); @@ -32,6 +40,14 @@ ASTPtr ASTTableExpression::clone() const return res; } +void ASTTableJoin::updateTreeHashImpl(SipHash & hash_state) const +{ + hash_state.update(locality); + hash_state.update(strictness); + hash_state.update(kind); + IAST::updateTreeHashImpl(hash_state); +} + ASTPtr ASTTableJoin::clone() const { auto res = std::make_shared(*this); @@ -43,6 +59,12 @@ ASTPtr ASTTableJoin::clone() const return res; } +void ASTArrayJoin::updateTreeHashImpl(SipHash & hash_state) const +{ + hash_state.update(kind); + IAST::updateTreeHashImpl(hash_state); +} + ASTPtr ASTArrayJoin::clone() const { auto res = std::make_shared(*this); diff --git a/src/Parsers/ASTTablesInSelectQuery.h b/src/Parsers/ASTTablesInSelectQuery.h index 01c6914b46c..792bef747ff 100644 --- a/src/Parsers/ASTTablesInSelectQuery.h +++ b/src/Parsers/ASTTablesInSelectQuery.h @@ -56,6 +56,7 @@ struct ASTTableExpression : public IAST String getID(char) const override { return "TableExpression"; } ASTPtr clone() const override; void formatImpl(const FormatSettings & settings, FormatState & state, FormatStateStacked frame) const override; + void updateTreeHashImpl(SipHash & hash_state) const override; }; @@ -108,6 +109,7 @@ struct ASTTableJoin : public IAST void formatImplBeforeTable(const FormatSettings & settings, FormatState & state, FormatStateStacked frame) const; void formatImplAfterTable(const FormatSettings & settings, FormatState & state, FormatStateStacked frame) const; void formatImpl(const FormatSettings & settings, FormatState & state, FormatStateStacked frame) const override; + void updateTreeHashImpl(SipHash & hash_state) const override; }; inline bool isLeft(ASTTableJoin::Kind kind) { return kind == ASTTableJoin::Kind::Left; } @@ -139,6 +141,7 @@ struct ASTArrayJoin : public IAST String getID(char) const override { return "ArrayJoin"; } ASTPtr clone() const override; void formatImpl(const FormatSettings & settings, FormatState & state, FormatStateStacked frame) const override; + void updateTreeHashImpl(SipHash & hash_state) const override; }; diff --git a/tests/queries/0_stateless/01051_scalar_optimization.reference b/tests/queries/0_stateless/01051_scalar_optimization.reference new file mode 100644 index 00000000000..2d979cdc089 --- /dev/null +++ b/tests/queries/0_stateless/01051_scalar_optimization.reference @@ -0,0 +1,2 @@ +0 99 +1 diff --git a/tests/queries/0_stateless/01051_scalar_optimization.sql b/tests/queries/0_stateless/01051_scalar_optimization.sql new file mode 100644 index 00000000000..3aa218d228b --- /dev/null +++ b/tests/queries/0_stateless/01051_scalar_optimization.sql @@ -0,0 +1,6 @@ +SELECT (SELECT number FROM numbers(100) ORDER BY number LIMIT 1), + (SELECT number FROM numbers(100) ORDER BY number DESC LIMIT 1); + +SELECT 1 + WHERE 0=(SELECT number FROM numbers(2) ORDER BY number LIMIT 1) + AND 1=(SELECT number FROM numbers(2) ORDER BY number DESC LIMIT 1);