[CORE] Remove an extra argument of make_decimal function #3754
Conversation
rui-mo
changed the title
|
Thanks for opening a pull request! Could you open an issue for this pull request on Github Issues? https://github.com/oap-project/gluten/issues Then could you also rename commit message and pull request title in the following format? See also: |
|
Run Gluten Clickhouse CI |
rui-mo
added
the
pending velox rebase
rui-mo
force-pushed
the
wip_make_decimal
branch
from
ec85d18
to
93260df
Compare
|
Run Gluten Clickhouse CI |
rui-mo
force-pushed
the
wip_make_decimal
branch
from
93260df
to
760497d
Compare
|
Run Gluten Clickhouse CI |
| val expressionNodes = | ||
| Lists.newArrayList(childNode, toTypeNodes, new BooleanLiteralNode(original.nullOnOverflow)) | ||
| Lists.newArrayList(childNode, new BooleanLiteralNode(original.nullOnOverflow)) |
There was a problem hiding this comment.
@zzcclp Is toTypeNodes needed for CH backend?
There was a problem hiding this comment.
@loneylee please help to check this.
There was a problem hiding this comment.
Not need, but the function parameters have been changed, and the ch backend needs to update too.
else if (function_name == "make_decimal")
{
if (args.size() < 3)
throw Exception(ErrorCodes::BAD_ARGUMENTS, "make_decimal function requires at least 3 args.");
ch_function_name = SCALAR_FUNCTIONS.at(function_name);
auto null_on_overflow = args.at(2).value().literal().boolean();
if (null_on_overflow)
ch_function_name = ch_function_name + "OrNull";
}
CI test is passed. Maybe ut is not open. I will commit a pr to fix it.
There was a problem hiding this comment.
@loneylee Modified in this PR. Could you help review? Thanks.
rui-mo
force-pushed
the
wip_make_decimal
branch
from
760497d
to
390dba0
Compare
|
Run Gluten Clickhouse CI |
|
Run Gluten Clickhouse CI |
rui-mo
force-pushed
the
wip_make_decimal
branch
from
ce16aa1
to
86aac8f
Compare
|
Run Gluten Clickhouse CI |
rui-mo
changed the title
|
===== Performance report for TPCH SF2000 with Velox backend, for reference only ====
|
|
This fixed TPC-DS q78's fallback on hash aggregation. Thanks! |
What changes were proposed in this pull request?
Remove the extra argument
toTypeNodesfrom make_decimal function.How was this patch tested?
Verified on Jenkins.