Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix that coalesce mistakenly removed nullable from the result column (#3394) #3399

Merged
merged 11 commits into from
Apr 14, 2022
Merged

Fix that coalesce mistakenly removed nullable from the result column (#3394) #3399

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
0e25e4c
update
fuzhe1989 Nov 5, 2021
70c3bc3
Update coalesce_pushdown.test
fuzhe1989 Nov 5, 2021
f222b4f
Delete gtest_coalesce.cpp
fuzhe1989 Nov 5, 2021
84b80ef
Merge branch 'release-5.2' into cherry-pick-3394-to-release-5.2
fzhedu Apr 14, 2022
1a3fd15
Merge branch 'release-5.2' into cherry-pick-3394-to-release-5.2
ti-chi-bot Apr 14, 2022
2d8f070
Merge branch 'release-5.2' into cherry-pick-3394-to-release-5.2
ti-chi-bot Apr 14, 2022
e7ed54f
Merge branch 'release-5.2' into cherry-pick-3394-to-release-5.2
ti-chi-bot Apr 14, 2022
3c39e1e
Merge branch 'release-5.2' into cherry-pick-3394-to-release-5.2
ti-chi-bot Apr 14, 2022
7f71033
format code
windtalker Apr 14, 2022
501db7b
Merge branch 'cherry-pick-3394-to-release-5.2' of github.com:ti-chi-b…
windtalker Apr 14, 2022
b15fb5f
Merge branch 'release-5.2' into cherry-pick-3394-to-release-5.2
ti-chi-bot Apr 14, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
124 changes: 29 additions & 95 deletions dbms/src/Functions/FunctionsNull.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
#include <Functions/FunctionsNull.h>
#include <Functions/FunctionsLogical.h>
#include <Columns/ColumnNullable.h>
#include <DataTypes/DataTypeNothing.h>
#include <DataTypes/DataTypeNullable.h>
#include <DataTypes/DataTypesNumber.h>
#include <Functions/FunctionFactory.h>
#include <Functions/FunctionsComparison.h>
#include <Functions/FunctionsConditional.h>
#include <Functions/FunctionFactory.h>
#include <DataTypes/DataTypesNumber.h>
#include <DataTypes/DataTypeNullable.h>
#include <DataTypes/DataTypeNothing.h>
#include <Columns/ColumnNullable.h>
#include <Functions/FunctionsLogical.h>
#include <Functions/FunctionsNull.h>


namespace DB
Expand All @@ -25,20 +25,11 @@ void registerFunctionsNull(FunctionFactory & factory)

/// Implementation of isNull.

FunctionPtr FunctionIsNull::create(const Context &)
{
return std::make_shared<FunctionIsNull>();
}
FunctionPtr FunctionIsNull::create(const Context &) { return std::make_shared<FunctionIsNull>(); }

std::string FunctionIsNull::getName() const
{
return name;
}
std::string FunctionIsNull::getName() const { return name; }

DataTypePtr FunctionIsNull::getReturnTypeImpl(const DataTypes &) const
{
return std::make_shared<DataTypeUInt8>();
}
DataTypePtr FunctionIsNull::getReturnTypeImpl(const DataTypes &) const { return std::make_shared<DataTypeUInt8>(); }

void FunctionIsNull::executeImpl(Block & block, const ColumnNumbers & arguments, size_t result)
{
Expand All @@ -58,37 +49,16 @@ void FunctionIsNull::executeImpl(Block & block, const ColumnNumbers & arguments,

/// Implementation of isNotNull.

FunctionPtr FunctionIsNotNull::create(const Context &)
{
return std::make_shared<FunctionIsNotNull>();
}
FunctionPtr FunctionIsNotNull::create(const Context &) { return std::make_shared<FunctionIsNotNull>(); }

std::string FunctionIsNotNull::getName() const
{
return name;
}
std::string FunctionIsNotNull::getName() const { return name; }

DataTypePtr FunctionIsNotNull::getReturnTypeImpl(const DataTypes &) const
{
return std::make_shared<DataTypeUInt8>();
}
DataTypePtr FunctionIsNotNull::getReturnTypeImpl(const DataTypes &) const { return std::make_shared<DataTypeUInt8>(); }

void FunctionIsNotNull::executeImpl(Block & block, const ColumnNumbers & arguments, size_t result)
{
Block temp_block
{
block.getByPosition(arguments[0]),
{
nullptr,
std::make_shared<DataTypeUInt8>(),
""
},
{
nullptr,
std::make_shared<DataTypeUInt8>(),
""
}
};
Block temp_block{block.getByPosition(arguments[0]), {nullptr, std::make_shared<DataTypeUInt8>(), ""},
{nullptr, std::make_shared<DataTypeUInt8>(), ""}};

FunctionIsNull{}.execute(temp_block, {0}, 1);
FunctionNot{}.execute(temp_block, {1}, 2);
Expand All @@ -98,15 +68,9 @@ void FunctionIsNotNull::executeImpl(Block & block, const ColumnNumbers & argumen

/// Implementation of coalesce.

FunctionPtr FunctionCoalesce::create(const Context & context)
{
return std::make_shared<FunctionCoalesce>(context);
}
FunctionPtr FunctionCoalesce::create(const Context & context) { return std::make_shared<FunctionCoalesce>(context); }

std::string FunctionCoalesce::getName() const
{
return name;
}
std::string FunctionCoalesce::getName() const { return name; }

DataTypePtr FunctionCoalesce::getReturnTypeImpl(const DataTypes & arguments) const
{
Expand Down Expand Up @@ -220,23 +184,17 @@ void FunctionCoalesce::executeImpl(Block & block, const ColumnNumbers & argument
ColumnPtr res = std::move(temp_block.getByPosition(result).column);

/// if last argument is not nullable, result should be also not nullable
if (!block.getByPosition(multi_if_args.back()).column->isColumnNullable() && res->isColumnNullable())
if (!block.getByPosition(filtered_args.back()).type->isNullable() && res->isColumnNullable())
res = static_cast<const ColumnNullable &>(*res).getNestedColumnPtr();

block.getByPosition(result).column = std::move(res);
}

/// Implementation of ifNull.

FunctionPtr FunctionIfNull::create(const Context &)
{
return std::make_shared<FunctionIfNull>();
}
FunctionPtr FunctionIfNull::create(const Context &) { return std::make_shared<FunctionIfNull>(); }

std::string FunctionIfNull::getName() const
{
return name;
}
std::string FunctionIfNull::getName() const { return name; }

DataTypePtr FunctionIfNull::getReturnTypeImpl(const DataTypes & arguments) const
{
Expand Down Expand Up @@ -284,15 +242,9 @@ void FunctionIfNull::executeImpl(Block & block, const ColumnNumbers & arguments,

/// Implementation of nullIf.

FunctionPtr FunctionNullIf::create(const Context & )
{
return std::make_shared<FunctionNullIf>();
}
FunctionPtr FunctionNullIf::create(const Context &) { return std::make_shared<FunctionNullIf>(); }

std::string FunctionNullIf::getName() const
{
return name;
}
std::string FunctionNullIf::getName() const { return name; }

DataTypePtr FunctionNullIf::getReturnTypeImpl(const DataTypes & arguments) const
{
Expand Down Expand Up @@ -328,20 +280,11 @@ void FunctionNullIf::executeImpl(Block & block, const ColumnNumbers & arguments,

/// Implementation of assumeNotNull.

FunctionPtr FunctionAssumeNotNull::create(const Context &)
{
return std::make_shared<FunctionAssumeNotNull>();
}
FunctionPtr FunctionAssumeNotNull::create(const Context &) { return std::make_shared<FunctionAssumeNotNull>(); }

std::string FunctionAssumeNotNull::getName() const
{
return name;
}
std::string FunctionAssumeNotNull::getName() const { return name; }

DataTypePtr FunctionAssumeNotNull::getReturnTypeImpl(const DataTypes & arguments) const
{
return removeNullable(arguments[0]);
}
DataTypePtr FunctionAssumeNotNull::getReturnTypeImpl(const DataTypes & arguments) const { return removeNullable(arguments[0]); }

void FunctionAssumeNotNull::executeImpl(Block & block, const ColumnNumbers & arguments, size_t result)
{
Expand All @@ -359,24 +302,15 @@ void FunctionAssumeNotNull::executeImpl(Block & block, const ColumnNumbers & arg

/// Implementation of toNullable.

FunctionPtr FunctionToNullable::create(const Context &)
{
return std::make_shared<FunctionToNullable>();
}
FunctionPtr FunctionToNullable::create(const Context &) { return std::make_shared<FunctionToNullable>(); }

std::string FunctionToNullable::getName() const
{
return name;
}
std::string FunctionToNullable::getName() const { return name; }

DataTypePtr FunctionToNullable::getReturnTypeImpl(const DataTypes & arguments) const
{
return makeNullable(arguments[0]);
}
DataTypePtr FunctionToNullable::getReturnTypeImpl(const DataTypes & arguments) const { return makeNullable(arguments[0]); }

void FunctionToNullable::executeImpl(Block & block, const ColumnNumbers & arguments, size_t result)
{
block.getByPosition(result).column = makeNullable(block.getByPosition(arguments[0]).column);
}

}
} // namespace DB
16 changes: 13 additions & 3 deletions tests/fullstack-test/expr/coalesce_pushdown.test
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,11 @@ mysql> INSERT INTO test.test_tb(id,char_,enum_,longtext_,mediumtext_,set_,text_,

mysql> analyze table test.test_tb
mysql> alter table test.test_tb set tiflash replica 1
func> wait_table test test_tb


mysql> drop table if exists test.fix_3388
mysql> create table if not exists test.fix_3388(a varchar(10))
mysql> alter table test.fix_3388 set tiflash replica 1
mysql> insert into test.fix_3388 values ('a')
func> wait_table test test_tb fix_3388

# start checking
mysql> select /*+ read_from_storage(tiflash[test.test_tb]) */ id from test.test_tb where char_ = coalesce(null, char_);
Expand Down Expand Up @@ -335,3 +337,11 @@ mysql> select /*+ read_from_storage(tiflash[test.test_tb]) */ id from test.test_
| 2 |
+----+

# fix 3388
mysql> set tidb_enforce_mpp=1; select count(*) from test.fix_3388 where coalesce(a, null) = "a"
+----------+
| count(*) |
+----------+
| 1 |
+----------+