Skip to content

[Fix](Variant) use uinque id to access column reader (#39841)#40269

Merged
eldenmoon merged 4 commits intoapache:branch-2.1from
eldenmoon:branch-2.1-p3
Sep 9, 2024
Merged

[Fix](Variant) use uinque id to access column reader (#39841)#40269
eldenmoon merged 4 commits intoapache:branch-2.1from
eldenmoon:branch-2.1-p3

Conversation

@eldenmoon
Copy link
Copy Markdown
Member

@eldenmoon eldenmoon commented Sep 2, 2024

@doris-robot
Copy link
Copy Markdown

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.

@eldenmoon
Copy link
Copy Markdown
Member Author

run buildall

@eldenmoon
Copy link
Copy Markdown
Member Author

run buildall

Copy link
Copy Markdown
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 thread be/src/olap/rowset/segment_v2/segment.cpp
Copy link
Copy Markdown
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 thread be/src/olap/rowset/segment_v2/segment.cpp
@eldenmoon
Copy link
Copy Markdown
Member Author

run buildall

Copy link
Copy Markdown
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

@@ -523,22 +549,23 @@ Status Segment::_new_iterator_with_variant_root(const TabletColumn& tablet_colum
Status Segment::new_column_iterator_with_path(const TabletColumn& tablet_column,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

warning: function 'new_column_iterator_with_path' has cognitive complexity of 77 (threshold 50) [readability-function-cognitive-complexity]

Status Segment::new_column_iterator_with_path(const TabletColumn& tablet_column,
                ^
Additional context

be/src/olap/rowset/segment_v2/segment.cpp:552: +1, including nesting penalty of 0, nesting level increased to 1

    int32_t unique_id = tablet_column.unique_id() > 0 ? tablet_column.unique_id()
                                                      ^

be/src/olap/rowset/segment_v2/segment.cpp:554: +1, including nesting penalty of 0, nesting level increased to 1

    if (!_sub_column_tree.contains(unique_id)) {
    ^

be/src/olap/rowset/segment_v2/segment.cpp:556: +2, including nesting penalty of 1, nesting level increased to 2

        RETURN_IF_ERROR(new_default_iterator(tablet_column, iter));
        ^

be/src/common/status.h:619: expanded from macro 'RETURN_IF_ERROR'

    do {                                \
    ^

be/src/olap/rowset/segment_v2/segment.cpp:556: +3, including nesting penalty of 2, nesting level increased to 3

        RETURN_IF_ERROR(new_default_iterator(tablet_column, iter));
        ^

be/src/common/status.h:621: expanded from macro 'RETURN_IF_ERROR'

        if (UNLIKELY(!_status_.ok())) { \
        ^

be/src/olap/rowset/segment_v2/segment.cpp:570: nesting level increased to 1

    auto type_to_read_flat_leaves = [](ReaderType type) {
                                    ^

be/src/olap/rowset/segment_v2/segment.cpp:575: +1

               type == ReaderType::READER_FULL_COMPACTION || type == ReaderType::READER_CHECKSUM;
                                                          ^

be/src/olap/rowset/segment_v2/segment.cpp:578: +1, including nesting penalty of 0, nesting level increased to 1

    if (opt != nullptr && type_to_read_flat_leaves(opt->io_ctx.reader_type)) {
    ^

be/src/olap/rowset/segment_v2/segment.cpp:578: +1

    if (opt != nullptr && type_to_read_flat_leaves(opt->io_ctx.reader_type)) {
                       ^

be/src/olap/rowset/segment_v2/segment.cpp:583: +2, including nesting penalty of 1, nesting level increased to 2

        if (!node) {
        ^

be/src/olap/rowset/segment_v2/segment.cpp:585: +3, including nesting penalty of 2, nesting level increased to 3

            if (sparse_node != nullptr && sparse_node->is_leaf_node()) {
            ^

be/src/olap/rowset/segment_v2/segment.cpp:585: +1

            if (sparse_node != nullptr && sparse_node->is_leaf_node()) {
                                       ^

be/src/olap/rowset/segment_v2/segment.cpp:586: +4, including nesting penalty of 3, nesting level increased to 4

                RETURN_IF_ERROR(_new_iterator_with_variant_root(
                ^

be/src/common/status.h:619: expanded from macro 'RETURN_IF_ERROR'

    do {                                \
    ^

be/src/olap/rowset/segment_v2/segment.cpp:586: +5, including nesting penalty of 4, nesting level increased to 5

                RETURN_IF_ERROR(_new_iterator_with_variant_root(
                ^

be/src/common/status.h:621: expanded from macro 'RETURN_IF_ERROR'

        if (UNLIKELY(!_status_.ok())) { \
        ^

be/src/olap/rowset/segment_v2/segment.cpp:588: +1, nesting level increased to 3

            } else {
              ^

be/src/olap/rowset/segment_v2/segment.cpp:589: +4, including nesting penalty of 3, nesting level increased to 4

                RETURN_IF_ERROR(new_default_iterator(tablet_column, iter));
                ^

be/src/common/status.h:619: expanded from macro 'RETURN_IF_ERROR'

    do {                                \
    ^

be/src/olap/rowset/segment_v2/segment.cpp:589: +5, including nesting penalty of 4, nesting level increased to 5

                RETURN_IF_ERROR(new_default_iterator(tablet_column, iter));
                ^

be/src/common/status.h:621: expanded from macro 'RETURN_IF_ERROR'

        if (UNLIKELY(!_status_.ok())) { \
        ^

be/src/olap/rowset/segment_v2/segment.cpp:594: +2, including nesting penalty of 1, nesting level increased to 2

        RETURN_IF_ERROR(node->data.reader->new_iterator(&it));
        ^

be/src/common/status.h:619: expanded from macro 'RETURN_IF_ERROR'

    do {                                \
    ^

be/src/olap/rowset/segment_v2/segment.cpp:594: +3, including nesting penalty of 2, nesting level increased to 3

        RETURN_IF_ERROR(node->data.reader->new_iterator(&it));
        ^

be/src/common/status.h:621: expanded from macro 'RETURN_IF_ERROR'

        if (UNLIKELY(!_status_.ok())) { \
        ^

be/src/olap/rowset/segment_v2/segment.cpp:599: +1, including nesting penalty of 0, nesting level increased to 1

    if (node != nullptr) {
    ^

be/src/olap/rowset/segment_v2/segment.cpp:600: +2, including nesting penalty of 1, nesting level increased to 2

        if (node->is_leaf_node() && sparse_node == nullptr) {
        ^

be/src/olap/rowset/segment_v2/segment.cpp:600: +1

        if (node->is_leaf_node() && sparse_node == nullptr) {
                                 ^

be/src/olap/rowset/segment_v2/segment.cpp:605: +3, including nesting penalty of 2, nesting level increased to 3

            RETURN_IF_ERROR(node->data.reader->new_iterator(&it));
            ^

be/src/common/status.h:619: expanded from macro 'RETURN_IF_ERROR'

    do {                                \
    ^

be/src/olap/rowset/segment_v2/segment.cpp:605: +4, including nesting penalty of 3, nesting level increased to 4

            RETURN_IF_ERROR(node->data.reader->new_iterator(&it));
            ^

be/src/common/status.h:621: expanded from macro 'RETURN_IF_ERROR'

        if (UNLIKELY(!_status_.ok())) { \
        ^

be/src/olap/rowset/segment_v2/segment.cpp:607: +1, nesting level increased to 2

        } else {
          ^

be/src/olap/rowset/segment_v2/segment.cpp:610: +3, including nesting penalty of 2, nesting level increased to 3

            RETURN_IF_ERROR(HierarchicalDataReader::create(iter, relative_path, node, root));
            ^

be/src/common/status.h:619: expanded from macro 'RETURN_IF_ERROR'

    do {                                \
    ^

be/src/olap/rowset/segment_v2/segment.cpp:610: +4, including nesting penalty of 3, nesting level increased to 4

            RETURN_IF_ERROR(HierarchicalDataReader::create(iter, relative_path, node, root));
            ^

be/src/common/status.h:621: expanded from macro 'RETURN_IF_ERROR'

        if (UNLIKELY(!_status_.ok())) { \
        ^

be/src/olap/rowset/segment_v2/segment.cpp:612: +1, nesting level increased to 1

    } else {
      ^

be/src/olap/rowset/segment_v2/segment.cpp:614: +2, including nesting penalty of 1, nesting level increased to 2

        if (sparse_node != nullptr) {
        ^

be/src/olap/rowset/segment_v2/segment.cpp:616: +3, including nesting penalty of 2, nesting level increased to 3

            RETURN_IF_ERROR(_new_iterator_with_variant_root(tablet_column, iter, root,
            ^

be/src/common/status.h:619: expanded from macro 'RETURN_IF_ERROR'

    do {                                \
    ^

be/src/olap/rowset/segment_v2/segment.cpp:616: +4, including nesting penalty of 3, nesting level increased to 4

            RETURN_IF_ERROR(_new_iterator_with_variant_root(tablet_column, iter, root,
            ^

be/src/common/status.h:621: expanded from macro 'RETURN_IF_ERROR'

        if (UNLIKELY(!_status_.ok())) { \
        ^

be/src/olap/rowset/segment_v2/segment.cpp:618: +1, nesting level increased to 2

        } else {
          ^

be/src/olap/rowset/segment_v2/segment.cpp:620: +3, including nesting penalty of 2, nesting level increased to 3

            RETURN_IF_ERROR(new_default_iterator(tablet_column, iter));
            ^

be/src/common/status.h:619: expanded from macro 'RETURN_IF_ERROR'

    do {                                \
    ^

be/src/olap/rowset/segment_v2/segment.cpp:620: +4, including nesting penalty of 3, nesting level increased to 4

            RETURN_IF_ERROR(new_default_iterator(tablet_column, iter));
            ^

be/src/common/status.h:621: expanded from macro 'RETURN_IF_ERROR'

        if (UNLIKELY(!_status_.ok())) { \
        ^

Currently, the variant type is not supported rename column because its
column reader accesses columns by path rather than by unique ID. If the
name is modified, the column reader may not locate the column
correctly.So we should access by unique id
@eldenmoon
Copy link
Copy Markdown
Member Author

run buildall

@eldenmoon
Copy link
Copy Markdown
Member Author

run buildall

@eldenmoon
Copy link
Copy Markdown
Member Author

run buildall

@eldenmoon eldenmoon merged commit f69063e into apache:branch-2.1 Sep 9, 2024
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.

2 participants