Skip to content

Conversation

@HappenLee
Copy link
Contributor

@HappenLee HappenLee commented Apr 10, 2024

Proposed changes

Before:
join right table column string over uint32
string column Length is too Large

After:
get the right result, but may exec slowly

Further comments

If this is a relatively large or complex change, kick off the discussion at dev@doris.apache.org by explaining why you chose the solution you did and what alternatives you considered, etc...

@doris-robot
Copy link

Thank you for your contribution to Apache Doris.
Don't know what should be done next? See How to process your PR

Since 2024-03-18, the Document has been moved to doris-website.
See Doris Document.

@HappenLee
Copy link
Contributor Author

run buildall

@github-actions
Copy link
Contributor

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

@doris-robot
Copy link

TeamCity be ut coverage result:
Function Coverage: 35.63% (8911/25012)
Line Coverage: 27.33% (73142/267624)
Region Coverage: 26.46% (37825/142958)
Branch Coverage: 23.21% (19274/83044)
Coverage Report: http://coverage.selectdb-in.cc/coverage/2d0c934b491fad724a2b4e41272ad6f2a1290366_2d0c934b491fad724a2b4e41272ad6f2a1290366/report/index.html

@HappenLee
Copy link
Contributor Author

run buildall

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

Comment on lines +226 to +227
void sort_column(const ColumnString64& column, EqualFlags& flags, IColumn::Permutation& perms,
EqualRange& range, bool last_column) const {
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: method 'sort_column' can be made static [readability-convert-member-functions-to-static]

Suggested change
void sort_column(const ColumnString64& column, EqualFlags& flags, IColumn::Permutation& perms,
EqualRange& range, bool last_column) const {
static void sort_column(const ColumnString64& column, EqualFlags& flags, IColumn::Permutation& perms,
EqualRange& range, bool last_column) {

@HappenLee
Copy link
Contributor Author

run buildall

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

Comment on lines +546 to +547
for (size_t i = 0; i < length; ++i)
cur_offsets[old_size + i] = src_offsets[start + i] - nested_offset + prev_max_offset;
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: statement should be inside braces [readability-braces-around-statements]

Suggested change
for (size_t i = 0; i < length; ++i)
cur_offsets[old_size + i] = src_offsets[start + i] - nested_offset + prev_max_offset;
for (size_t i = 0; i < length; ++i) {
cur_offsets[old_size + i] = src_offsets[start + i] - nested_offset + prev_max_offset;
}

}
}

void ColumnMap::insert_range_from_ignore_overflow(const IColumn& src, size_t start, size_t length) {
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: method 'insert_range_from_ignore_overflow' can be made static [readability-convert-member-functions-to-static]

be/src/vec/columns/column_map.h:104:

-     void insert_range_from_ignore_overflow(const IColumn& src, size_t start,
+     static void insert_range_from_ignore_overflow(const IColumn& src, size_t start,

}
}

void ColumnStruct::insert_range_from_ignore_overflow(const IColumn& src, size_t start,
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: method 'insert_range_from_ignore_overflow' can be made static [readability-convert-member-functions-to-static]

be/src/vec/columns/column_struct.h:147:

-     void insert_range_from_ignore_overflow(const IColumn& src, size_t start,
+     static void insert_range_from_ignore_overflow(const IColumn& src, size_t start,

@HappenLee
Copy link
Contributor Author

run buildall

@doris-robot
Copy link

TeamCity be ut coverage result:
Function Coverage: 35.48% (8907/25107)
Line Coverage: 27.20% (73142/268889)
Region Coverage: 26.33% (37817/143637)
Branch Coverage: 23.11% (19271/83376)
Coverage Report: http://coverage.selectdb-in.cc/coverage/719a6d1be5a331bfd2794ef12f00cb805f97b86f_719a6d1be5a331bfd2794ef12f00cb805f97b86f/report/index.html

@HappenLee
Copy link
Contributor Author

run buildall

@HappenLee
Copy link
Contributor Author

run buildall

@doris-robot
Copy link

TeamCity be ut coverage result:
Function Coverage: 35.49% (8910/25109)
Line Coverage: 27.21% (73184/268926)
Region Coverage: 26.33% (37826/143642)
Branch Coverage: 23.12% (19276/83378)
Coverage Report: http://coverage.selectdb-in.cc/coverage/26833e0cd1968d5ad7d4efec47d689acfe55b94e_26833e0cd1968d5ad7d4efec47d689acfe55b94e/report/index.html

@HappenLee
Copy link
Contributor Author

run buildall

@doris-robot
Copy link

TeamCity be ut coverage result:
Function Coverage: 35.47% (8908/25114)
Line Coverage: 27.20% (73160/268988)
Region Coverage: 26.32% (37811/143675)
Branch Coverage: 23.10% (19267/83400)
Coverage Report: http://coverage.selectdb-in.cc/coverage/eadfde7b23619fc03ea6aefcb14d5f41ed942087_eadfde7b23619fc03ea6aefcb14d5f41ed942087/report/index.html

@HappenLee HappenLee changed the title save column append data [Exec](join) Support column string64 to avoid join failed in string size overflow the uint32 Apr 17, 2024
@HappenLee HappenLee marked this pull request as ready for review April 17, 2024 05:34
@HappenLee
Copy link
Contributor Author

run buildall

@HappenLee
Copy link
Contributor Author

run buildall

@HappenLee
Copy link
Contributor Author

run buildall

@doris-robot
Copy link

TeamCity be ut coverage result:
Function Coverage: 35.44% (8910/25143)
Line Coverage: 27.15% (73178/269566)
Region Coverage: 26.27% (37821/143989)
Branch Coverage: 23.07% (19270/83546)
Coverage Report: http://coverage.selectdb-in.cc/coverage/14a1b54cfa5dd0f9d1161f055a574791d4b359cf_14a1b54cfa5dd0f9d1161f055a574791d4b359cf/report/index.html

@HappenLee
Copy link
Contributor Author

run buildall

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

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

@github-actions
Copy link
Contributor

PR approved by anyone and no changes requested.

@doris-robot
Copy link

TeamCity be ut coverage result:
Function Coverage: 35.43% (8910/25149)
Line Coverage: 27.13% (73191/269828)
Region Coverage: 26.26% (37836/144062)
Branch Coverage: 23.06% (19277/83584)
Coverage Report: http://coverage.selectdb-in.cc/coverage/34f195c557c5e64b662cc597e112b50b18fec600_34f195c557c5e64b662cc597e112b50b18fec600/report/index.html

@HappenLee HappenLee merged commit 7258692 into apache:master Apr 18, 2024
HappenLee added a commit to HappenLee/incubator-doris that referenced this pull request Apr 18, 2024
yiguolei pushed a commit that referenced this pull request Apr 18, 2024
BiteTheDDDDt pushed a commit that referenced this pull request Apr 19, 2024
This is imported by #33511. wrongly used

ColumnStr<T> ();

which violate C++20 standard(see https://wg21.cmeerw.net/cwg/issue2237) but still supported by clang up until now(see llvm/llvm-project#58112)
yiguolei pushed a commit that referenced this pull request Apr 19, 2024
This is imported by #33511. wrongly used

ColumnStr<T> ();

which violate C++20 standard(see https://wg21.cmeerw.net/cwg/issue2237) but still supported by clang up until now(see llvm/llvm-project#58112)
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. reviewed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants