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

[fix](sleep) sleep with character const make be crash #37681

Merged
merged 2 commits into from
Jul 15, 2024
Merged

[fix](sleep) sleep with character const make be crash #37681

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
659fa65
fix sleep make be crash
cambyzju Jul 11, 2024
62dc29d
add some comments
cambyzju Jul 12, 2024
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
19 changes: 7 additions & 12 deletions be/src/vec/functions/function_utility.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,31 +62,26 @@ class FunctionSleep : public IFunction {
size_t get_number_of_arguments() const override { return 1; }

DataTypePtr get_return_type_impl(const DataTypes& arguments) const override {
if (arguments[0].get()->is_nullable()) {
if (arguments[0]->is_nullable()) {
return make_nullable(std::make_shared<DataTypeUInt8>());
}
return std::make_shared<DataTypeUInt8>();
}

bool use_default_implementation_for_nulls() const override { return false; }

// Sleep function should not be executed during open stage, this will makes fragment prepare
// waiting too long, so we do not use default impl.
bool use_default_implementation_for_constants() const override { return false; }

Copy link
Contributor

@zhiqiang-hhhh zhiqiang-hhhh Jul 12, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please add some comment to explain why we not using default_impl for const?
"Sleep function should not be executed during open stage, this will makes fragment prepare waiting too long, so we do not use default impl."

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Status execute_impl(FunctionContext* context, Block& block, const ColumnNumbers& arguments,
size_t result, size_t input_rows_count) const override {
ColumnPtr& argument_column = block.get_by_position(arguments[0]).column;
const auto& argument_column =
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use unpack_if_const may be better

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there are already lots of convert_to_full_column_if_const used, maybe next time, we refractor all these used is better.

block.get_by_position(arguments[0]).column->convert_to_full_column_if_const();

auto res_column = ColumnUInt8::create();

if (is_column_const(*argument_column)) {
Int64 seconds = argument_column->get_int(0);
for (int i = 0; i < input_rows_count; i++) {
std::this_thread::sleep_for(std::chrono::seconds(seconds));
res_column->insert(1);
}

block.replace_by_position(result, std::move(res_column));
} else if (auto* nullable_column = check_and_get_column<ColumnNullable>(*argument_column)) {
if (auto* nullable_column = check_and_get_column<ColumnNullable>(*argument_column)) {
auto null_map_column = ColumnUInt8::create();

auto nested_column = nullable_column->get_nested_column_ptr();
Expand Down Expand Up @@ -154,4 +149,4 @@ void register_function_utility(SimpleFunctionFactory& factory) {
factory.register_function<FunctionVersion>();
}

} // namespace doris::vectorized
} // namespace doris::vectorized
1 change: 1 addition & 0 deletions regression-test/suites/nereids_p0/system/test_query_sys.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ suite("test_query_sys", "query,p0") {
sql "select pi();"
sql "select e();"
sql "select sleep(2);"
sql "select sleep('1.1');"

// INFORMATION_SCHEMA
sql "SELECT table_name FROM INFORMATION_SCHEMA.TABLES where table_schema=\"nereids_test_query_db\" and TABLE_TYPE = \"BASE TABLE\" order by table_name"
Expand Down
Loading