From 1fdbb9509f8dbdfe1a4eb3961a332cfc3ad52a99 Mon Sep 17 00:00:00 2001 From: vdimir Date: Mon, 13 Sep 2021 16:35:17 +0300 Subject: [PATCH] Add additional checkTypesOfKeys to JoiningTransform::transformHeader --- src/Interpreters/HashJoin.cpp | 5 +++++ src/Interpreters/HashJoin.h | 2 ++ src/Interpreters/IJoin.h | 2 ++ src/Interpreters/JoinSwitcher.h | 5 +++++ src/Interpreters/MergeJoin.cpp | 8 ++++++++ src/Interpreters/MergeJoin.h | 1 + src/Interpreters/join_common.cpp | 2 +- src/Processors/Transforms/JoiningTransform.cpp | 3 ++- .../0_stateless/01010_pm_join_all_join_bug.reference | 1 + tests/queries/0_stateless/01010_pm_join_all_join_bug.sql | 3 +++ tests/queries/0_stateless/01881_join_on_conditions.sql | 8 ++++---- 11 files changed, 34 insertions(+), 6 deletions(-) diff --git a/src/Interpreters/HashJoin.cpp b/src/Interpreters/HashJoin.cpp index 1c450d531d6..2d7015bdb92 100644 --- a/src/Interpreters/HashJoin.cpp +++ b/src/Interpreters/HashJoin.cpp @@ -1387,6 +1387,11 @@ ColumnWithTypeAndName HashJoin::joinGet(const Block & block, const Block & block return keys.getByPosition(keys.columns() - 1); } +void HashJoin::checkTypesOfKeys(const Block & block) const +{ + JoinCommon::checkTypesOfKeys(block, table_join->keyNamesLeft(), right_table_keys, key_names_right); +} + void HashJoin::joinBlock(Block & block, ExtraBlockPtr & not_processed) { const Names & key_names_left = table_join->keyNamesLeft(); diff --git a/src/Interpreters/HashJoin.h b/src/Interpreters/HashJoin.h index 2e691f189c4..a36393278ff 100644 --- a/src/Interpreters/HashJoin.h +++ b/src/Interpreters/HashJoin.h @@ -140,6 +140,8 @@ public: */ bool addJoinedBlock(const Block & block, bool check_limits) override; + void checkTypesOfKeys(const Block & block) const override; + /** Join data from the map (that was previously built by calls to addJoinedBlock) to the block with data from "left" table. * Could be called from different threads in parallel. */ diff --git a/src/Interpreters/IJoin.h b/src/Interpreters/IJoin.h index 2215402e1d4..c48524e2031 100644 --- a/src/Interpreters/IJoin.h +++ b/src/Interpreters/IJoin.h @@ -27,6 +27,8 @@ public: /// @returns false, if some limit was exceeded and you should not insert more data. virtual bool addJoinedBlock(const Block & block, bool check_limits = true) = 0; + virtual void checkTypesOfKeys(const Block & block) const = 0; + /// Join the block with data from left hand of JOIN to the right hand data (that was previously built by calls to addJoinedBlock). /// Could be called from different threads in parallel. virtual void joinBlock(Block & block, std::shared_ptr & not_processed) = 0; diff --git a/src/Interpreters/JoinSwitcher.h b/src/Interpreters/JoinSwitcher.h index e750bc5eed0..83ac70044a8 100644 --- a/src/Interpreters/JoinSwitcher.h +++ b/src/Interpreters/JoinSwitcher.h @@ -26,6 +26,11 @@ public: /// @returns false, if join-on-disk disk limit exceeded bool addJoinedBlock(const Block & block, bool check_limits) override; + void checkTypesOfKeys(const Block & block) const override + { + join->checkTypesOfKeys(block); + } + void joinBlock(Block & block, std::shared_ptr & not_processed) override { join->joinBlock(block, not_processed); diff --git a/src/Interpreters/MergeJoin.cpp b/src/Interpreters/MergeJoin.cpp index 0150bbe1d93..eb806caa34f 100644 --- a/src/Interpreters/MergeJoin.cpp +++ b/src/Interpreters/MergeJoin.cpp @@ -666,16 +666,24 @@ bool MergeJoin::addJoinedBlock(const Block & src_block, bool) return saveRightBlock(std::move(block)); } +void MergeJoin::checkTypesOfKeys(const Block & block) const +{ + /// Do not check auxailary column for extra conditions, use original key names + JoinCommon::checkTypesOfKeys(block, table_join->keyNamesLeft(), right_table_keys, table_join->keyNamesRight()); +} + void MergeJoin::joinBlock(Block & block, ExtraBlockPtr & not_processed) { Names lowcard_keys = lowcard_right_keys; if (block) { + /// We need to check type of masks before `addConditionJoinColumn`, because it assumes that types is correct JoinCommon::checkTypesOfMasks(block, mask_column_name_left, right_sample_block, mask_column_name_right); /// Add auxiliary column, will be removed after joining addConditionJoinColumn(block, JoinTableSide::Left); + /// Types of keys can be checked only after `checkTypesOfKeys` JoinCommon::checkTypesOfKeys(block, key_names_left, right_table_keys, key_names_right); materializeBlockInplace(block); diff --git a/src/Interpreters/MergeJoin.h b/src/Interpreters/MergeJoin.h index 9e765041846..53cde2bff7c 100644 --- a/src/Interpreters/MergeJoin.h +++ b/src/Interpreters/MergeJoin.h @@ -25,6 +25,7 @@ public: const TableJoin & getTableJoin() const override { return *table_join; } bool addJoinedBlock(const Block & block, bool check_limits) override; + void checkTypesOfKeys(const Block & block) const override; void joinBlock(Block &, ExtraBlockPtr & not_processed) override; void setTotals(const Block &) override; diff --git a/src/Interpreters/join_common.cpp b/src/Interpreters/join_common.cpp index b097a8f6192..9a4cdd97c13 100644 --- a/src/Interpreters/join_common.cpp +++ b/src/Interpreters/join_common.cpp @@ -399,7 +399,7 @@ void checkTypesOfKeys(const Block & block_left, const Names & key_names_left, void checkTypesOfKeys(const Block & block_left, const Names & key_names_left, const String & condition_name_left, const Block & block_right, const Names & key_names_right, const String & condition_name_right) { - checkTypesOfKeys(block_left, key_names_left,block_right,key_names_right); + checkTypesOfKeys(block_left, key_names_left, block_right, key_names_right); checkTypesOfMasks(block_left, condition_name_left, block_right, condition_name_right); } diff --git a/src/Processors/Transforms/JoiningTransform.cpp b/src/Processors/Transforms/JoiningTransform.cpp index f099f41f472..aceb04d7994 100644 --- a/src/Processors/Transforms/JoiningTransform.cpp +++ b/src/Processors/Transforms/JoiningTransform.cpp @@ -14,8 +14,9 @@ namespace ErrorCodes Block JoiningTransform::transformHeader(Block header, const JoinPtr & join) { - ExtraBlockPtr tmp; LOG_DEBUG(&Poco::Logger::get("JoiningTransform"), "Before join block: '{}'", header.dumpStructure()); + join->checkTypesOfKeys(header); + ExtraBlockPtr tmp; join->joinBlock(header, tmp); LOG_DEBUG(&Poco::Logger::get("JoiningTransform"), "After join block: '{}'", header.dumpStructure()); return header; diff --git a/tests/queries/0_stateless/01010_pm_join_all_join_bug.reference b/tests/queries/0_stateless/01010_pm_join_all_join_bug.reference index 5f51e52afa8..6648a96cbc3 100644 --- a/tests/queries/0_stateless/01010_pm_join_all_join_bug.reference +++ b/tests/queries/0_stateless/01010_pm_join_all_join_bug.reference @@ -7,3 +7,4 @@ 1 0 1 1 1 0 1 1 1 +- diff --git a/tests/queries/0_stateless/01010_pm_join_all_join_bug.sql b/tests/queries/0_stateless/01010_pm_join_all_join_bug.sql index ef406108514..18a67f41194 100644 --- a/tests/queries/0_stateless/01010_pm_join_all_join_bug.sql +++ b/tests/queries/0_stateless/01010_pm_join_all_join_bug.sql @@ -9,4 +9,7 @@ SELECT * FROM ints l LEFT JOIN ints r USING i64 ORDER BY l.i32, r.i32; SELECT '-'; SELECT * FROM ints l INNER JOIN ints r USING i64 ORDER BY l.i32, r.i32; +SELECT '-'; +SELECT count() FROM ( SELECT [1], count(1) ) AS t1 ALL RIGHT JOIN ( SELECT number AS s FROM numbers(2) ) AS t2 USING (s); -- { serverError NOT_FOUND_COLUMN_IN_BLOCK } + DROP TABLE ints; diff --git a/tests/queries/0_stateless/01881_join_on_conditions.sql b/tests/queries/0_stateless/01881_join_on_conditions.sql index a34c413845b..e074397ff5e 100644 --- a/tests/queries/0_stateless/01881_join_on_conditions.sql +++ b/tests/queries/0_stateless/01881_join_on_conditions.sql @@ -48,10 +48,10 @@ SELECT DISTINCT t1.id FROM t1 INNER ANY JOIN t2 ON t1.id == t2.id AND toLowCardi SELECT DISTINCT t1.id FROM t1 INNER ANY JOIN t2 ON t1.id == t2.id AND toNullable(toLowCardinality(t1.key2 != '')); SELECT '--'; -SELECT DISTINCT t1.key, toUInt8(t1.id) as e FROM t1 INNER ANY JOIN t2 ON t1.id == t2.id AND e; +SELECT DISTINCT t1.key, toUInt8(t1.id) as e FROM t1 INNER ANY JOIN t2 ON t1.id == t2.id AND e; -- `e + 1` is UInt16 SELECT DISTINCT t1.key, toUInt8(t1.id) as e FROM t1 INNER ANY JOIN t2 ON t1.id == t2.id AND e + 1; -- { serverError 403 } -SELECT DISTINCT t1.key, toUInt8(t1.id) as e FROM t1 INNER ANY JOIN t2 ON t1.id == t2.id AND toUInt8(e + 1); +SELECT DISTINCT t1.key, toUInt8(t1.id) as e FROM t1 INNER ANY JOIN t2 ON t1.id == t2.id AND toUInt8(e + 1); SELECT '--'; SELECT t1.id, t1.key, t1.key2, t2.id, t2.key, t2.key2 FROM t1 FULL JOIN t2 ON t1.id == t2.id AND t2.key == t2.key2 AND t1.key == t1.key2 ORDER BY t1.id, t2.id; @@ -110,10 +110,10 @@ SELECT DISTINCT t1.id FROM t1 INNER ANY JOIN t2 ON t1.id == t2.id AND toLowCardi SELECT DISTINCT t1.id FROM t1 INNER ANY JOIN t2 ON t1.id == t2.id AND toNullable(toLowCardinality(t1.key2 != '')); SELECT '--'; -SELECT DISTINCT t1.key, toUInt8(t1.id) as e FROM t1 INNER ANY JOIN t2 ON t1.id == t2.id AND e; +SELECT DISTINCT t1.key, toUInt8(t1.id) as e FROM t1 INNER ANY JOIN t2 ON t1.id == t2.id AND e; -- `e + 1` is UInt16 SELECT DISTINCT t1.key, toUInt8(t1.id) as e FROM t1 INNER ANY JOIN t2 ON t1.id == t2.id AND e + 1; -- { serverError 403 } -SELECT DISTINCT t1.key, toUInt8(t1.id) as e FROM t1 INNER ANY JOIN t2 ON t1.id == t2.id AND toUInt8(e + 1); +SELECT DISTINCT t1.key, toUInt8(t1.id) as e FROM t1 INNER ANY JOIN t2 ON t1.id == t2.id AND toUInt8(e + 1); SELECT '--'; SELECT t1.id, t1.key, t1.key2, t2.id, t2.key, t2.key2 FROM t1 FULL JOIN t2 ON t1.id == t2.id AND t2.key == t2.key2 AND t1.key == t1.key2 ORDER BY t1.id, t2.id;