diff --git a/src/Interpreters/HashJoin.cpp b/src/Interpreters/HashJoin.cpp index dc041094381..5ff4f9beb05 100644 --- a/src/Interpreters/HashJoin.cpp +++ b/src/Interpreters/HashJoin.cpp @@ -886,20 +886,20 @@ public: const auto & lhs = lhs_block.getByPosition(i); const auto & rhs = rhs_block.getByPosition(i); if (lhs.name != rhs.name) - throw DB::Exception(ErrorCodes::LOGICAL_ERROR, "Block structure mismatch: [{}] != [{}]", - lhs_block.dumpStructure(), rhs_block.dumpStructure()); + throw DB::Exception(ErrorCodes::LOGICAL_ERROR, "Block structure mismatch: [{}] != [{}] ({} != {})", + lhs_block.dumpStructure(), rhs_block.dumpStructure(), lhs.name, rhs.name); const auto & ltype = recursiveRemoveLowCardinality(lhs.type); const auto & rtype = recursiveRemoveLowCardinality(rhs.type); if (!ltype->equals(*rtype)) - throw DB::Exception(ErrorCodes::LOGICAL_ERROR, "Block structure mismatch: [{}] != [{}]", - lhs_block.dumpStructure(), rhs_block.dumpStructure()); + throw DB::Exception(ErrorCodes::LOGICAL_ERROR, "Block structure mismatch: [{}] != [{}] ({} != {})", + lhs_block.dumpStructure(), rhs_block.dumpStructure(), ltype->getName(), rtype->getName()); const auto & lcol = recursiveRemoveLowCardinality(lhs.column); const auto & rcol = recursiveRemoveLowCardinality(rhs.column); if (lcol->getDataType() != rcol->getDataType()) - throw DB::Exception(ErrorCodes::LOGICAL_ERROR, "Block structure mismatch: [{}] != [{}]", - lhs_block.dumpStructure(), rhs_block.dumpStructure()); + throw DB::Exception(ErrorCodes::LOGICAL_ERROR, "Block structure mismatch: [{}] != [{}] ({} != {})", + lhs_block.dumpStructure(), rhs_block.dumpStructure(), lcol->getDataType(), rcol->getDataType()); } } diff --git a/src/Interpreters/TableJoin.cpp b/src/Interpreters/TableJoin.cpp index aa4f821657f..78218ac59a5 100644 --- a/src/Interpreters/TableJoin.cpp +++ b/src/Interpreters/TableJoin.cpp @@ -458,16 +458,6 @@ TableJoin::createConvertingActions( LOG_DEBUG(&Poco::Logger::get("TableJoin"), "{} JOIN converting actions: empty", side); return; } - auto format_cols = [](const auto & cols) -> std::string - { - std::vector str_cols; - str_cols.reserve(cols.size()); - for (const auto & col : cols) - str_cols.push_back(fmt::format("'{}': {}", col.name, col.type->getName())); - return fmt::format("[{}]", fmt::join(str_cols, ", ")); - }; - LOG_DEBUG(&Poco::Logger::get("TableJoin"), "{} JOIN converting actions: {} -> {}", - side, format_cols(dag->getRequiredColumns()), format_cols(dag->getResultColumns())); }; log_actions("Left", left_converting_actions); log_actions("Right", right_converting_actions); diff --git a/src/Storages/StorageJoin.cpp b/src/Storages/StorageJoin.cpp index 55f3b889f22..320f05e038f 100644 --- a/src/Storages/StorageJoin.cpp +++ b/src/Storages/StorageJoin.cpp @@ -229,11 +229,13 @@ HashJoinPtr StorageJoin::getJoinLocked(std::shared_ptr analyzed_join, return join_clone; } - void StorageJoin::insertBlock(const Block & block, ContextPtr context) { + Block block_to_insert = block; + convertRightBlock(block_to_insert); + TableLockHolder holder = tryLockTimedWithContext(rwlock, RWLockImpl::Write, context); - join->addJoinedBlock(block, true); + join->addJoinedBlock(block_to_insert, true); } size_t StorageJoin::getSize(ContextPtr context) const @@ -265,6 +267,16 @@ ColumnWithTypeAndName StorageJoin::joinGet(const Block & block, const Block & bl return join->joinGet(block, block_with_columns_to_add); } +void StorageJoin::convertRightBlock(Block & block) const +{ + bool need_covert = use_nulls && isLeftOrFull(kind); + if (!need_covert) + return; + + for (auto & col : block) + JoinCommon::convertColumnToNullable(col); +} + void registerStorageJoin(StorageFactory & factory) { auto creator_fn = [](const StorageFactory::Arguments & args) diff --git a/src/Storages/StorageJoin.h b/src/Storages/StorageJoin.h index 3d7a9d9b5ec..96afd442c72 100644 --- a/src/Storages/StorageJoin.h +++ b/src/Storages/StorageJoin.h @@ -77,9 +77,7 @@ public: { auto metadata_snapshot = getInMemoryMetadataPtr(); Block block = metadata_snapshot->getSampleBlock(); - if (use_nulls && isLeftOrFull(kind)) - for (auto & col : block) - JoinCommon::convertColumnToNullable(col); + convertRightBlock(block); return block; } @@ -108,6 +106,8 @@ private: void finishInsert() override {} size_t getSize(ContextPtr context) const override; RWLockImpl::LockHolder tryLockTimedWithContext(const RWLock & lock, RWLockImpl::Type type, ContextPtr context) const; + + void convertRightBlock(Block & block) const; }; } diff --git a/tests/queries/0_stateless/02531_storage_join_null_44940.reference b/tests/queries/0_stateless/02531_storage_join_null_44940.reference new file mode 100644 index 00000000000..b7e40c360c0 --- /dev/null +++ b/tests/queries/0_stateless/02531_storage_join_null_44940.reference @@ -0,0 +1,3 @@ +3 \N 3 +2 2 2 +1 1 1 diff --git a/tests/queries/0_stateless/02531_storage_join_null_44940.sql b/tests/queries/0_stateless/02531_storage_join_null_44940.sql new file mode 100644 index 00000000000..136fc8bbef1 --- /dev/null +++ b/tests/queries/0_stateless/02531_storage_join_null_44940.sql @@ -0,0 +1,18 @@ + +SET allow_suspicious_low_cardinality_types = 1; + +DROP TABLE IF EXISTS t1__fuzz_8; +DROP TABLE IF EXISTS full_join__fuzz_4; + +CREATE TABLE t1__fuzz_8 (`x` LowCardinality(UInt32), `str` Nullable(Int16)) ENGINE = Memory; +INSERT INTO t1__fuzz_8 VALUES (1, 1), (2, 2); + +CREATE TABLE full_join__fuzz_4 (`x` LowCardinality(UInt32), `s` LowCardinality(String)) ENGINE = Join(`ALL`, FULL, x) SETTINGS join_use_nulls = 1; +INSERT INTO full_join__fuzz_4 VALUES (1, '1'), (2, '2'), (3, '3'); + +SET join_use_nulls = 1; + +SELECT * FROM t1__fuzz_8 FULL OUTER JOIN full_join__fuzz_4 USING (x) ORDER BY x DESC, str ASC, s ASC NULLS LAST; + +DROP TABLE IF EXISTS t1__fuzz_8; +DROP TABLE IF EXISTS full_join__fuzz_4;