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

[Feature] Support sort key be able to nullable. #15641

Merged

Conversation

Linkerist
Copy link
Contributor

@Linkerist Linkerist commented Dec 22, 2022

What type of PR is this:

  • BugFix
  • Feature
  • Enhancement
  • Refactor
  • UT
  • Doc
  • Tool

Which issues of this PR fixes :

Fixes #15057
close #14289

Problem Summary(Required) :

Sort key should can be nullable, this PR support it. In primary key encoder, when encode a nullable column, if the datum is null, fill a null marker in the front of it, if the datum is not null, just fill a normal marker in the front of it.

Checklist:

  • I have added test cases for my bug fix or my new feature
  • This pr will affect users' behaviors
  • This pr needs user documentation (for new or modified features or behaviors)
    • I have added documentation for my new feature or new function

Bugfix cherry-pick branch check:

  • I have checked the version labels which the pr will be auto backported to target branch
    • 2.5
    • 2.4
    • 2.3
    • 2.2

@github-actions
Copy link

clang-tidy review says "All clean, LGTM! 👍"

@sonarcloud
Copy link

sonarcloud bot commented Dec 22, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
2.4% 2.4% Duplication

@sevev sevev mentioned this pull request Jan 5, 2023
16 tasks
buff.clear();
for (int j = 0; j < ncol; j++) {
if (cols[j]->is_null(i)) {
buff.push_back(SORT_KEY_NULL_FIRST_MARKER);
Copy link
Contributor

Choose a reason for hiding this comment

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

If the data is also started as 0x00, how to handle it?

Copy link
Contributor

Choose a reason for hiding this comment

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

If the data is also started as 0x00, how to handle it?

If we're a middle component of a composite key, we need to add a \x00 at the end in order to separate this component from the next one. However, if we just did that, we'd have issues where a key that actually has \x00 in it would compare wrong, so we have to instead add \x00\x00, and encode \x00 as \x00\x01.

@@ -238,9 +241,6 @@ inline Status decode_slice(Slice* src, std::string* dest, bool is_last) {
}

bool PrimaryKeyEncoder::is_supported(const VectorizedField& f) {
if (f.is_nullable()) {
return false;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Will this affect Primary Key's restriction? so primary key still don't support nulllable yet?

Copy link
Contributor

Choose a reason for hiding this comment

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

Will this affect Primary Key's restriction? so primary key still don't support nulllable yet?

Yes, the keys of PrimaryKey table should not be nullable.

@wanpengfei-git wanpengfei-git added the Approved Ready to merge label Jan 16, 2023
@wanpengfei-git
Copy link
Collaborator

run starrocks_admit_test

@wanpengfei-git wanpengfei-git merged commit db4ecc5 into StarRocks:main Jan 17, 2023
@github-actions github-actions bot removed Approved Ready to merge be-build labels Jan 17, 2023
@Linkerist Linkerist deleted the feature_support_nullable_sort_key branch March 16, 2023 06:59
sevev pushed a commit to sevev/starrocks that referenced this pull request Aug 15, 2023
Support sort key be able to nullable.

Signed-off-by: zhangqiang <qiangzh95@gmail.com>
wanpengfei-git pushed a commit that referenced this pull request Aug 22, 2023
Support sort key be able to nullable.

Signed-off-by: zhangqiang <qiangzh95@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Enhancement] Support nullable in primary key encoder
6 participants