You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by da...@apache.org on 2022/07/29 13:34:03 UTC
[doris] branch master updated: [fix] the nullable info is lost in ifnull expr (#11212)
This is an automated email from the ASF dual-hosted git repository.
dataroaring pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/master by this push:
new c1fbee7fe1 [fix] the nullable info is lost in ifnull expr (#11212)
c1fbee7fe1 is described below
commit c1fbee7fe12b86a2cc6f5f38e8833b588e65ceb0
Author: starocean999 <40...@users.noreply.github.com>
AuthorDate: Fri Jul 29 21:33:58 2022 +0800
[fix] the nullable info is lost in ifnull expr (#11212)
ifnull function has defect when processing nullable column or const column in some case
---
be/src/vec/functions/function_ifnull.h | 24 +++++++++++++---
be/src/vec/functions/if.cpp | 3 ++
.../test_conditional_function.out | 32 ++++++++++++++++++++++
.../test_conditional_function.groovy | 15 ++++++++--
4 files changed, 68 insertions(+), 6 deletions(-)
diff --git a/be/src/vec/functions/function_ifnull.h b/be/src/vec/functions/function_ifnull.h
index b6627641c0..1a83785c48 100644
--- a/be/src/vec/functions/function_ifnull.h
+++ b/be/src/vec/functions/function_ifnull.h
@@ -40,11 +40,21 @@ public:
bool use_default_implementation_for_constants() const override { return false; }
+ // be compatible with fe code
+ /*
+ if (fn.functionName().equalsIgnoreCase("ifnull") || fn.functionName().equalsIgnoreCase("nvl")) {
+ Preconditions.checkState(children.size() == 2);
+ if (children.get(0).isNullable()) {
+ return children.get(1).isNullable();
+ }
+ return false;
+ }
+ */
DataTypePtr get_return_type_impl(const DataTypes& arguments) const override {
- if (!arguments[0]->is_nullable() && arguments[1]->is_nullable()) {
- return reinterpret_cast<const DataTypeNullable*>(arguments[1].get())->get_nested_type();
+ if (arguments[0]->is_nullable()) {
+ return arguments[1];
}
- return arguments[1];
+ return arguments[0];
}
bool use_default_implementation_for_nulls() const override { return false; }
@@ -75,10 +85,16 @@ public:
const ColumnsWithTypeAndName if_columns {
null_column_arg0, block.get_by_position(arguments[1]), nested_column_arg0};
+ // see get_return_type_impl
+ // if result is nullable, means both then and else column are nullable, we use original col_left to keep nullable info
+ // if result is not nullable, means both then and else column are not nullable, we use nested_column_arg0 to remove nullable info
+ bool result_nullable = block.get_by_position(result).type->is_nullable();
Block temporary_block({
null_column_arg0,
block.get_by_position(arguments[1]),
- nested_column_arg0,
+ result_nullable
+ ? col_left
+ : nested_column_arg0, // if result is nullable, we need pass the original col_left else pass nested_column_arg0
block.get_by_position(result),
});
diff --git a/be/src/vec/functions/if.cpp b/be/src/vec/functions/if.cpp
index 0de81cd7c7..0308acc36a 100644
--- a/be/src/vec/functions/if.cpp
+++ b/be/src/vec/functions/if.cpp
@@ -139,6 +139,9 @@ public:
static ColumnPtr get_nested_column(const ColumnPtr& column) {
if (auto* nullable = check_and_get_column<ColumnNullable>(*column))
return nullable->get_nested_column_ptr();
+ else if (const auto* column_const = check_and_get_column<ColumnConst>(*column))
+ return ColumnConst::create(get_nested_column(column_const->get_data_column_ptr()),
+ column->size());
return column;
}
diff --git a/regression-test/data/query/sql_functions/conditional_functions/test_conditional_function.out b/regression-test/data/query/sql_functions/conditional_functions/test_conditional_function.out
index 6f3f475c57..52962b5c54 100644
--- a/regression-test/data/query/sql_functions/conditional_functions/test_conditional_function.out
+++ b/regression-test/data/query/sql_functions/conditional_functions/test_conditional_function.out
@@ -159,3 +159,35 @@ true
-- !sql --
9999-07-31
+-- !sql --
+3
+
+-- !sql --
+9999-07
+
+-- !sql --
+\N
+
+-- !sql --
+9999-07
+
+-- !sql --
+1
+2
+3
+4
+99990101
+99990101
+99990101
+99990101
+
+-- !sql --
+1
+2
+3
+4
+999
+999
+999
+999
+
diff --git a/regression-test/suites/query/sql_functions/conditional_functions/test_conditional_function.groovy b/regression-test/suites/query/sql_functions/conditional_functions/test_conditional_function.groovy
index 7ea565e534..76b32b92ff 100644
--- a/regression-test/suites/query/sql_functions/conditional_functions/test_conditional_function.groovy
+++ b/regression-test/suites/query/sql_functions/conditional_functions/test_conditional_function.groovy
@@ -71,10 +71,21 @@ suite("test_conditional_function", "query") {
qt_sql "select is_null_pred(user_id) from ${tbName} order by user_id"
qt_sql "select is_not_null_pred(user_id) from ${tbName} order by user_id"
- sql "DROP TABLE ${tbName};"
-
qt_sql """select if(date_format(CONCAT_WS('', '9999-07', '-26'), '%Y-%m')= DATE_FORMAT( curdate(), '%Y-%m'),
curdate(),
DATE_FORMAT(DATE_SUB(month_ceil ( CONCAT_WS('', '9999-07', '-26')), 1), '%Y-%m-%d'));"""
+ qt_sql "select ifnull(date_format(CONCAT_WS('', '9999-07', '-00'), '%Y-%m'),3);"
+
+ qt_sql "select ifnull(date_format(CONCAT_WS('', '9999-07', '-01'), '%Y-%m'),3);"
+
+ qt_sql "select ifnull(date_format(CONCAT_WS('', '9999-07', '-00'), '%Y-%m'),date_format(CONCAT_WS('', '9999-07', '-00'), '%Y-%m'));"
+
+ qt_sql "select ifnull(date_format(CONCAT_WS('', '9999-07', '-00'), '%Y-%m'),date_format(CONCAT_WS('', '9999-07', '-26'), '%Y-%m'));"
+
+ qt_sql "select ifnull( user_id, to_date('9999-01-01')) r from ${tbName} order by r"
+
+ qt_sql "select ifnull( user_id, 999) r from ${tbName} order by r"
+
+ sql "DROP TABLE ${tbName};"
}
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org