Merge pull request #45184 from ClickHouse/vdimir/storage_join_null_44940

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: [{}] != [{}]",
+                throw DB::Exception(ErrorCodes::LOGICAL_ERROR, "Block structure mismatch: [{}] != [{}] ({} != {})",
-                    lhs_block.dumpStructure(), rhs_block.dumpStructure());
+                    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: [{}] != [{}]",
+                throw DB::Exception(ErrorCodes::LOGICAL_ERROR, "Block structure mismatch: [{}] != [{}] ({} != {})",
-                    lhs_block.dumpStructure(), rhs_block.dumpStructure());
+                    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: [{}] != [{}]",
+                throw DB::Exception(ErrorCodes::LOGICAL_ERROR, "Block structure mismatch: [{}] != [{}] ({} != {})",
-                    lhs_block.dumpStructure(), rhs_block.dumpStructure());
+                    lhs_block.dumpStructure(), rhs_block.dumpStructure(), lcol->getDataType(), rcol->getDataType());
         }
     }
 

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<std::string> 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);

src/Storages/StorageJoin.cpp

@@ -229,11 +229,13 @@ HashJoinPtr StorageJoin::getJoinLocked(std::shared_ptr<TableJoin> 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)

src/Storages/StorageJoin.h

@@ -77,9 +77,7 @@ public:
     {
         auto metadata_snapshot = getInMemoryMetadataPtr();
         Block block = metadata_snapshot->getSampleBlock();
-        if (use_nulls && isLeftOrFull(kind))
+        convertRightBlock(block);
-            for (auto & col : block)
-                JoinCommon::convertColumnToNullable(col);
         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;
 };
 
 }

tests/queries/0_stateless/02531_storage_join_null_44940.reference

@@ -0,0 +1,3 @@
+3	\N	3
+2	2	2
+1	1	1

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;