Skip to content

Conversation

@BiteTheDDDDt
Copy link
Contributor

Proposed changes

fix wrong result of AcceptNullPredicate

@BiteTheDDDDt
Copy link
Contributor Author

run buildall

@github-actions
Copy link
Contributor

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

Copy link
Contributor

@HappenLee HappenLee left a comment

Choose a reason for hiding this comment

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

PLEASE ADD TEST

}
// create selected_flags
uint16_t max_idx = sel[size - 1];
std::vector<uint16_t> old_sel(size);
Copy link
Contributor

Choose a reason for hiding this comment

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

please add some test

@yiguolei yiguolei added usercase Important user case type label dev/2.1.x p0_w labels Aug 17, 2024
uint16_t row_idx = sel[i];
selected[row_idx] = true;
}
for (uint16_t i = 0; i < size; ++i) {
Copy link
Contributor

Choose a reason for hiding this comment

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

add comment here

const auto& nullable_col = assert_cast<const vectorized::ColumnNullable&>(column);
// call nested predicate evaluate
uint16_t new_size = _nested->evaluate(nullable_col.get_nested_column(), sel, size);

Copy link
Contributor

Choose a reason for hiding this comment

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

sel 1,2,3,7,8,9

sel 1,2,3,4,5,6,7,8,9

@BiteTheDDDDt
Copy link
Contributor Author

run buildall

@BiteTheDDDDt
Copy link
Contributor Author

run buildall

Copy link
Contributor

@HappenLee HappenLee left a comment

Choose a reason for hiding this comment

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

LGTM

const auto& nullmap = nullable_col.get_null_map_data();
for (uint16_t i = 0; i < size; ++i) {
flags[i] |= nullmap[sel[i]];
flags[i] |= (original_flags[i] && nullmap[sel[i]]);
Copy link
Contributor

Choose a reason for hiding this comment

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

我觉得这里不对吧
是不是应该是 flags[sel[i]] |= (original_flags[sel[i]] && nullmap[sel[i]]);

@github-actions
Copy link
Contributor

PR approved by at least one committer and no changes requested.

@github-actions github-actions bot added the approved Indicates a PR has been approved by one committer. label Aug 19, 2024
@github-actions
Copy link
Contributor

PR approved by anyone and no changes requested.

@BiteTheDDDDt
Copy link
Contributor Author

run buildall

Copy link
Contributor

@xiaokang xiaokang left a comment

Choose a reason for hiding this comment

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

LGTM

@BiteTheDDDDt BiteTheDDDDt merged commit f1c8426 into apache:master Aug 20, 2024
BiteTheDDDDt added a commit to BiteTheDDDDt/incubator-doris that referenced this pull request Aug 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by one committer. dev/2.1.6-merged dev/3.0.2-merged p0_w reviewed usercase Important user case type label

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants