[Enhancement] support simple sql function for factorial(from Hive)#57144
[Enhancement] support simple sql function for factorial(from Hive)#57144zclllyybb merged 3 commits intoapache:masterfrom
Conversation
- BE: Custom IFunction in math.cpp, UT in function_math_test.cpp. - FE: Update BuiltinScalarFunctions, signature in Factorial.java, constant folding in NumericArithmetic.java, visitor in ScalarFunctionVisitor.java. - Tests: Regression in test_math_unary_always_nullable.groovy & fold_constant_numeric_arithmatic.groovy, updated .out; additional updates for math.cpp, NumericArithmetic.java, test_math_unary_always_nullable.groovy. Closes apache#48203
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
be/src/vec/functions/math.cpp
Outdated
|
|
||
| Status execute_impl(FunctionContext* context, Block& block, const ColumnNumbers& arguments, | ||
| uint32_t result, size_t input_rows_count) const override { | ||
| ColumnPtr int_col = block.get_by_position(arguments[0]).column->convert_to_full_column_if_const(); |
There was a problem hiding this comment.
no need to do this, we have framework to do for const
be/src/vec/functions/math.cpp
Outdated
| ColumnPtr int_col = block.get_by_position(arguments[0]).column->convert_to_full_column_if_const(); | ||
|
|
||
| const ColumnUInt8* input_null_map = nullptr; | ||
| if (int_col->is_nullable()) { |
There was a problem hiding this comment.
ditto. we have framework for nullable input
be/src/vec/functions/math.cpp
Outdated
| const auto* data_col = check_and_get_column<ColumnInt64>(int_col.get()); | ||
|
|
||
| if (!data_col) { | ||
| return Status::InvalidArgument("Illegal column {} of argument of function {}", |
There was a problem hiding this comment.
should be InternalError for wrong type.
zclllyybb
left a comment
There was a problem hiding this comment.
basically good! please also add some case:
- factorial(column) which column is nullable and non-nullable
- factorial(nullable(literal))
done. not sure if its ok. looking forward to ur review |
|
PR approved by at least one committer and no changes requested. |
|
PR approved by anyone and no changes requested. |
|
run buildall |
TPC-H: Total hot run time: 34195 ms |
TPC-DS: Total hot run time: 188233 ms |
ClickBench: Total hot run time: 27.51 s |
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
FE Regression Coverage ReportIncrement line coverage |
…57144) Problem Summary: support simple sql function for factorial(from Hive)
…pache#57144) Problem Summary: support simple sql function for factorial(from Hive)
…pache#57144) Problem Summary: support simple sql function for factorial(from Hive)
What problem does this PR solve?
Related PR: #48203
Problem Summary: support simple sql function for factorial(from Hive)
Release note
None
Check List (For Author)
Test
Behavior changed:
Does this need documentation?
Check List (For Reviewer who merge this PR)