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.

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

[GLUTEN-6989][CH] Support RTrim with const source column #6992

Merged
merged 5 commits into from
Aug 29, 2024

Conversation

lwz9103
Copy link
Contributor

@lwz9103 lwz9103 commented Aug 23, 2024

What changes were proposed in this pull request?

(Please fill in changes proposed in this fix)

(Fixes: #6989)

How was this patch tested?

unit tests

Copy link

#6989

Copy link

Run Gluten Clickhouse CI

Copy link

Run Gluten Clickhouse CI

Copy link

Run Gluten Clickhouse CI

cpp-ch/local-engine/Functions/SparkFunctionTrim.cpp Outdated Show resolved Hide resolved
cpp-ch/local-engine/Functions/SparkFunctionTrim.cpp Outdated Show resolved Hide resolved
cpp-ch/local-engine/Functions/SparkFunctionTrim.cpp Outdated Show resolved Hide resolved
cpp-ch/local-engine/Functions/SparkFunctionTrim.cpp Outdated Show resolved Hide resolved
{
const char * char_data = reinterpret_cast<const char *>(data);
const char * char_end = char_data + size;
if (!trim_set || trim_set->none())
Copy link
Contributor

Choose a reason for hiding this comment

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

remove if. impossible to happend.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It may occurs when trim col is not constant and trim col contains null value.

Copy link
Contributor

Choose a reason for hiding this comment

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

we don't need to consider null cases as long as defaultimplementationfornulls is true.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed

cpp-ch/local-engine/Functions/SparkFunctionTrim.cpp Outdated Show resolved Hide resolved
Copy link

Run Gluten Clickhouse CI

1 similar comment
@lwz9103
Copy link
Contributor Author

lwz9103 commented Aug 27, 2024

Run Gluten Clickhouse CI

Copy link

Run Gluten Clickhouse CI

Copy link

Run Gluten Clickhouse CI

Copy link

Run Gluten Clickhouse CI

Copy link
Contributor

@taiyang-li taiyang-li left a comment

Choose a reason for hiding this comment

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

lgtm

@baibaichen baibaichen merged commit 6671dd0 into apache:main Aug 29, 2024
8 checks passed
sharkdtu pushed a commit to sharkdtu/gluten that referenced this pull request Nov 11, 2024
* [GLUTEN-6989][CH] Support RTrim with const source column
* [GLUTEN-6989][CH] Use bitmap256 to trim
* [GLUTEN-6989][CH] Fix comments
* [GLUTEN-6989][CH] Fix core dump
* [GLUTEN-6989][CH] Remove condition when trim col contains null
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[CH] RTrim not support source column const
3 participants