Skip to content

Commit 8e5d672

Browse files
alexey-milovidovalexey-milovidov
authored
Merge pull request ClickHouse#11340 from 4ertus2/bugs
Fix crash in select from StorageJoin
2 parents 4b06d09 + b7958d4 commit 8e5d672

File tree

3 files changed

+76
-41
lines changed

3 files changed

+76
-41
lines changed

‎src/Storages/StorageJoin.cpp‎

Lines changed: 26 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -251,22 +251,26 @@ class JoinSource : public SourceWithProgress
251251
, max_block_size(max_block_size_)
252252
, sample_block(std::move(sample_block_))
253253
{
254-
columns.resize(sample_block.columns());
255254
column_indices.resize(sample_block.columns());
256-
column_with_null.resize(sample_block.columns());
255+
256+
auto & saved_block = parent.getJoinedData()->sample_block;
257+
257258
for (size_t i = 0; i < sample_block.columns(); ++i)
258259
{
259260
auto & [_, type, name] = sample_block.getByPosition(i);
260261
if (parent.right_table_keys.has(name))
261262
{
262263
key_pos = i;
263-
column_with_null[i] = parent.right_table_keys.getByName(name).type->isNullable();
264+
const auto & column = parent.right_table_keys.getByName(name);
265+
restored_block.insert(column);
264266
}
265267
else
266268
{
267-
auto pos = parent.sample_block_with_columns_to_add.getPositionByName(name);
269+
size_t pos = saved_block.getPositionByName(name);
268270
column_indices[i] = pos;
269-
column_with_null[i] = !parent.sample_block_with_columns_to_add.getByPosition(pos).type->equals(*type);
271+
272+
const auto & column = saved_block.getByPosition(pos);
273+
restored_block.insert(column);
270274
}
271275
}
272276
}
@@ -291,43 +295,26 @@ class JoinSource : public SourceWithProgress
291295
std::shared_lock<std::shared_mutex> lock;
292296
UInt64 max_block_size;
293297
Block sample_block;
298+
Block restored_block; /// sample_block with parent column types
294299

295300
ColumnNumbers column_indices;
296-
std::vector<bool> column_with_null;
297301
std::optional<size_t> key_pos;
298-
MutableColumns columns;
299302

300303
std::unique_ptr<void, std::function<void(void *)>> position; /// type erasure
301304

302305

303306
template <ASTTableJoin::Kind KIND, ASTTableJoin::Strictness STRICTNESS, typename Maps>
304307
Chunk createChunk(const Maps & maps)
305308
{
306-
for (size_t i = 0; i < sample_block.columns(); ++i)
307-
{
308-
const auto & src_col = sample_block.safeGetByPosition(i);
309-
columns[i] = src_col.type->createColumn();
310-
if (column_with_null[i])
311-
{
312-
if (key_pos == i)
313-
{
314-
// unwrap null key column
315-
auto & nullable_col = assert_cast<ColumnNullable &>(*columns[i]);
316-
columns[i] = nullable_col.getNestedColumnPtr()->assumeMutable();
317-
}
318-
else
319-
// wrap non key column with null
320-
columns[i] = makeNullable(std::move(columns[i]))->assumeMutable();
321-
}
322-
}
309+
MutableColumns columns = restored_block.cloneEmpty().mutateColumns();
323310

324311
size_t rows_added = 0;
325312

326313
switch (parent.data->type)
327314
{
328315
#define M(TYPE) \
329316
case HashJoin::Type::TYPE: \
330-
rows_added = fillColumns<KIND, STRICTNESS>(*maps.TYPE); \
317+
rows_added = fillColumns<KIND, STRICTNESS>(*maps.TYPE, columns); \
331318
break;
332319
APPLY_FOR_JOIN_VARIANTS_LIMITED(M)
333320
#undef M
@@ -340,29 +327,27 @@ class JoinSource : public SourceWithProgress
340327
if (!rows_added)
341328
return {};
342329

343-
Columns res_columns;
344-
res_columns.reserve(columns.size());
345-
330+
/// Correct nullability
346331
for (size_t i = 0; i < columns.size(); ++i)
347-
if (column_with_null[i])
332+
{
333+
bool src_nullable = restored_block.getByPosition(i).type->isNullable();
334+
bool dst_nullable = sample_block.getByPosition(i).type->isNullable();
335+
336+
if (src_nullable && !dst_nullable)
348337
{
349-
if (key_pos == i)
350-
res_columns.emplace_back(makeNullable(std::move(columns[i])));
351-
else
352-
{
353-
const auto & nullable_col = assert_cast<const ColumnNullable &>(*columns[i]);
354-
res_columns.emplace_back(makeNullable(nullable_col.getNestedColumnPtr()));
355-
}
338+
auto & nullable_column = assert_cast<ColumnNullable &>(*columns[i]);
339+
columns[i] = nullable_column.getNestedColumnPtr()->assumeMutable();
356340
}
357-
else
358-
res_columns.emplace_back(std::move(columns[i]));
341+
else if (!src_nullable && dst_nullable)
342+
columns[i] = makeNullable(std::move(columns[i]))->assumeMutable();
343+
}
359344

360-
UInt64 num_rows = res_columns.at(0)->size();
361-
return Chunk(std::move(res_columns), num_rows);
345+
UInt64 num_rows = columns.at(0)->size();
346+
return Chunk(std::move(columns), num_rows);
362347
}
363348

364349
template <ASTTableJoin::Kind KIND, ASTTableJoin::Strictness STRICTNESS, typename Map>
365-
size_t fillColumns(const Map & map)
350+
size_t fillColumns(const Map & map, MutableColumns & columns)
366351
{
367352
size_t rows_added = 0;
368353

‎tests/queries/0_stateless/01140_select_from_storage_join_fix.reference‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
1 s 1 String String
2+
2 s 2 String String
3+
3 s 3 Nullable(String) String
4+
4 s 4 String Nullable(String)
5+
1 s 1 String String
6+
2 s 2 String String
7+
3 s 3 Nullable(String) String
8+
4 s 4 String Nullable(String)

‎tests/queries/0_stateless/01140_select_from_storage_join_fix.sql‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
DROP TABLE IF EXISTS t1;
2+
DROP TABLE IF EXISTS t2;
3+
DROP TABLE IF EXISTS t3;
4+
DROP TABLE IF EXISTS t4;
5+
6+
CREATE TABLE t1 (id String, name String, value UInt32)
7+
ENGINE = Join(ANY, LEFT, id)
8+
SETTINGS join_use_nulls = 1;
9+
10+
CREATE TABLE t2 (id String, name String, value UInt32)
11+
ENGINE = Join(ANY, LEFT, id)
12+
SETTINGS join_use_nulls = 0;
13+
14+
CREATE TABLE t3 (id Nullable(String), name String, value UInt32)
15+
ENGINE = Join(ANY, LEFT, id)
16+
SETTINGS join_use_nulls = 1;
17+
18+
CREATE TABLE t4 (id String, name Nullable(String), value UInt32)
19+
ENGINE = Join(ANY, LEFT, id)
20+
SETTINGS join_use_nulls = 0;
21+
22+
insert into t1 values('1', 's', 1);
23+
insert into t2 values('2', 's', 2);
24+
insert into t3 values('3', 's', 3);
25+
insert into t4 values('4', 's', 4);
26+
27+
select *, toTypeName(id), toTypeName(name) from t1;
28+
select *, toTypeName(id), toTypeName(name) from t2;
29+
select *, toTypeName(id), toTypeName(name) from t3;
30+
select *, toTypeName(id), toTypeName(name) from t4;
31+
32+
SET join_use_nulls = 1;
33+
34+
select *, toTypeName(id), toTypeName(name) from t1;
35+
select *, toTypeName(id), toTypeName(name) from t2;
36+
select *, toTypeName(id), toTypeName(name) from t3;
37+
select *, toTypeName(id), toTypeName(name) from t4;
38+
39+
DROP TABLE t1;
40+
DROP TABLE t2;
41+
DROP TABLE t3;
42+
DROP TABLE t4;

0 commit comments

Comments
 (0)