Skip to content

Commit ab15c8d

Browse files
alexey-milovidovalexey-milovidov
authored
Merge pull request #12021 from ClickHouse/fix-if-fixed-string
Fix function if with FixedString arguments of different sizes
2 parents 0f1bfa5 + 918e979 commit ab15c8d

File tree

4 files changed

+66
-7
lines changed

4 files changed

+66
-7
lines changed

‎src/Functions/if.cpp‎

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -432,8 +432,7 @@ class FunctionIf : public FunctionIfBase</*null_is_false=*/false>
432432
const PaddedPODArray<UInt8> & cond_data = cond_col->getData();
433433
size_t rows = cond_data.size();
434434

435-
if ((col_then_fixed || col_then_const_fixed)
436-
&& (col_else_fixed || col_else_const_fixed))
435+
if (isFixedString(block.getByPosition(result).type))
437436
{
438437
/// The result is FixedString.
439438

@@ -448,16 +447,19 @@ class FunctionIf : public FunctionIfBase</*null_is_false=*/false>
448447
else if (col_then_const_fixed && col_else_fixed)
449448
conditional(ConstSource<FixedStringSource>(*col_then_const_fixed), FixedStringSource(*col_else_fixed), sink, cond_data);
450449
else if (col_then_const_fixed && col_else_const_fixed)
451-
conditional(ConstSource<FixedStringSource>(*col_then_const_fixed), ConstSource<FixedStringSource>(*col_else_const_fixed), sink, cond_data);
450+
conditional(ConstSource<FixedStringSource>(*col_then_const_fixed),
451+
ConstSource<FixedStringSource>(*col_else_const_fixed), sink, cond_data);
452+
else
453+
return false;
452454

453455
block.getByPosition(result).column = std::move(col_res_untyped);
454456
return true;
455457
}
456458

457-
if ((col_then || col_then_const || col_then_fixed || col_then_const_fixed)
458-
&& (col_else || col_else_const || col_else_fixed || col_else_const_fixed))
459+
if (isString(block.getByPosition(result).type))
459460
{
460461
/// The result is String.
462+
461463
auto col_res = ColumnString::create();
462464
auto sink = StringSink(*col_res, rows);
463465

@@ -485,6 +487,17 @@ class FunctionIf : public FunctionIfBase</*null_is_false=*/false>
485487
conditional(ConstSource<StringSource>(*col_then_const), ConstSource<FixedStringSource>(*col_else_const_fixed), sink, cond_data);
486488
else if (col_then_const_fixed && col_else_const)
487489
conditional(ConstSource<FixedStringSource>(*col_then_const_fixed), ConstSource<StringSource>(*col_else_const), sink, cond_data);
490+
else if (col_then_fixed && col_else_fixed)
491+
conditional(FixedStringSource(*col_then_fixed), FixedStringSource(*col_else_fixed), sink, cond_data);
492+
else if (col_then_fixed && col_else_const_fixed)
493+
conditional(FixedStringSource(*col_then_fixed), ConstSource<FixedStringSource>(*col_else_const_fixed), sink, cond_data);
494+
else if (col_then_const_fixed && col_else_fixed)
495+
conditional(ConstSource<FixedStringSource>(*col_then_const_fixed), FixedStringSource(*col_else_fixed), sink, cond_data);
496+
else if (col_then_const_fixed && col_else_const_fixed)
497+
conditional(ConstSource<FixedStringSource>(*col_then_const_fixed),
498+
ConstSource<FixedStringSource>(*col_else_const_fixed), sink, cond_data);
499+
else
500+
return false;
488501

489502
block.getByPosition(result).column = std::move(col_res);
490503
return true;
@@ -590,7 +603,8 @@ class FunctionIf : public FunctionIfBase</*null_is_false=*/false>
590603
return true;
591604
}
592605

593-
static void executeGeneric(const ColumnUInt8 * cond_col, Block & block, const ColumnNumbers & arguments, size_t result, size_t input_rows_count)
606+
static void executeGeneric(
607+
const ColumnUInt8 * cond_col, Block & block, const ColumnNumbers & arguments, size_t result, size_t input_rows_count)
594608
{
595609
/// Convert both columns to the common type (if needed).
596610

‎src/Functions/multiIf.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ class FunctionMultiIf final : public FunctionIfBase</*null_is_false=*/true>
3939
bool isVariadic() const override { return true; }
4040
size_t getNumberOfArguments() const override { return 0; }
4141
bool useDefaultImplementationForNulls() const override { return false; }
42+
4243
ColumnNumbers getArgumentsThatDontImplyNullableReturnType(size_t number_of_arguments) const override
4344
{
4445
ColumnNumbers args;
@@ -70,7 +71,6 @@ class FunctionMultiIf final : public FunctionIfBase</*null_is_false=*/true>
7071
throw Exception{"Invalid number of arguments for function " + getName(),
7172
ErrorCodes::NUMBER_OF_ARGUMENTS_DOESNT_MATCH};
7273

73-
7474
for_conditions([&](const DataTypePtr & arg)
7575
{
7676
const IDataType * nested_type;

‎tests/queries/0_stateless/01355_if_fixed_string.reference‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
0\0\0\0\0 String
2+
1\0 String
3+
-2\0\0\0 String
4+
3\0 String
5+
-4\0\0\0 String
6+
5\0 String
7+
-6\0\0\0 String
8+
7\0 String
9+
-8\0\0\0 String
10+
9\0 String
11+
0\0 FixedString(2)
12+
1\0 FixedString(2)
13+
-2 FixedString(2)
14+
3\0 FixedString(2)
15+
-4 FixedString(2)
16+
5\0 FixedString(2)
17+
-6 FixedString(2)
18+
7\0 FixedString(2)
19+
-8 FixedString(2)
20+
9\0 FixedString(2)
21+
0 String
22+
1 String
23+
-2 String
24+
3 String
25+
-4 String
26+
5 String
27+
-6 String
28+
7 String
29+
-8 String
30+
9 String
31+
0\0 FixedString(2)
32+
1\0 FixedString(2)
33+
-2 FixedString(2)
34+
3\0 FixedString(2)
35+
-4 FixedString(2)
36+
5\0 FixedString(2)
37+
-6 FixedString(2)
38+
7\0 FixedString(2)
39+
-8 FixedString(2)
40+
9\0 FixedString(2)

‎tests/queries/0_stateless/01355_if_fixed_string.sql‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
SELECT if(number % 2, toFixedString(toString(number), 2), toFixedString(toString(-number), 5)) AS x, toTypeName(x) FROM system.numbers LIMIT 10;
2+
SELECT if(number % 2, toFixedString(toString(number), 2), toFixedString(toString(-number), 2)) AS x, toTypeName(x) FROM system.numbers LIMIT 10;
3+
4+
SELECT multiIf(number % 2, toFixedString(toString(number), 2), toFixedString(toString(-number), 5)) AS x, toTypeName(x) FROM system.numbers LIMIT 10;
5+
SELECT multiIf(number % 2, toFixedString(toString(number), 2), toFixedString(toString(-number), 2)) AS x, toTypeName(x) FROM system.numbers LIMIT 10;

0 commit comments

Comments
 (0)